sea-orm icon indicating copy to clipboard operation
sea-orm copied to clipboard

Use serde_json::Value is an intermediate state, and the performance of sea-orm will be lost

Open baoyachi opened this issue 3 years ago • 2 comments
trafficstars

Is it possible to receive sqlx's Row object directly through the object for conversion without intervening serde_json::Value. At present, serde_json::Value is an intermediate state, and the performance of sea-orm will be lost.

The code like this: 未命名文件

baoyachi avatar Aug 25 '22 13:08 baoyachi

Hey @baoyachi, thanks for the digging!

Internally, in SeaORM, we offer an into_json() method to map sqlx::Row directly to serde_json::Value

let cake: Option<serde_json::Value> = Cake::find_by_id(1)
    .into_json()
    .one(db)
    .await?;

assert_eq!(
    cake,
    Some(serde_json::json!({
        "id": 1,
        "name": "Cheese Cake"
    }))
);

https://www.sea-ql.org/SeaORM/docs/basic-crud/json/

The source code responsible for the conversion from sqlx::Row to serde_json::Value: https://github.com/SeaQL/sea-orm/blob/master/src/query/json.rs

billy1624 avatar Sep 01 '22 04:09 billy1624

I'm not sure if I answer your question. Is this a question that asking such conversion is possible or not? Or, this is a request for optimization of existing code? If so, could you point out the source code that needs to be refactor?

billy1624 avatar Sep 01 '22 04:09 billy1624

I'm pretty sure I understand what @baoyachi means here (I found this issue through google).

https://github.com/SeaQL/sea-orm/blob/45840990211373aa799384ba3175c582d51b373f/src/executor/query.rs#L643-L687

The implementation of TryGetableFromJson always gets the value as Option<serde_json::Value> first, and then invokes serde_json::from_value. This is not optimal. Instead, it would be better to directly get Option<sqlx::types::Json<Self>> from the database and just unpack it. The temporary serde_json::Value is unnecessary.

For example, if I deserialize struct Foo { a: i32, b: i32 }, then serde-json could directly produce this as a plain 8-bytes value. However, deserializing it as serde_json::Value first causes String allocations for "a" and "b" as well as a BTreeMap or IndexMap to store the object. This is clearly not ideal for performance.

main-- avatar Oct 05 '22 17:10 main--

@main-- you are right

baoyachi avatar Oct 08 '22 01:10 baoyachi

Hey @baoyachi, thanks for the clarification! Now I see what @baoyachi want to say. Please check https://github.com/SeaQL/sea-orm/pull/1249

billy1624 avatar Nov 24 '22 06:11 billy1624

Hey @baoyachi, thanks for the clarification! Now I see what @baoyachi want to say. Please check #1249

@billy1624 How much performance will be improved? Is there a numerical result comparison?

baoyachi avatar Nov 28 '22 09:11 baoyachi