SeaQL / sea-orm

🐚 An async & dynamic ORM for Rust
https://www.sea-ql.org/SeaORM/
Apache License 2.0
7.31k stars 514 forks source link

Add SeaORM to the Diesel benchmark #204

Closed weiznich closed 1 year ago

weiznich commented 3 years ago

I would like to invent you to add sea-orm (and maybe also sea-ql) to the relational database connection crates benchmark suite located inside of the diesel repository. This would give potential users a good overview over the current state + you would also have a comparison with other crates for free. Additionally to address your worries from reddit about those benchmarks not reflecting the strength of the async implementation side: We are more than happy to extend that benchmark suite so that these strengths could be shown in a better way. What sort of benchmark would you expect to show that? Maybe you would provide an initial implementation of an sea-orm based solution for such an use case?

tyt2y3 commented 3 years ago

Hi Diesel team. Welcome, sorry for the delay and thank you for helping out.

It really painted the picture where all of us are at, and the comparison with SQLx is very useful as it illustrates the overhead SeaORM added on top.

The Diesel benchmark suite is definitely a useful reference.

In my imagination, the more 'real world' benchmark would be a web server, and we'll be counting the number of requests it can handle per second, similar to below:

https://www.guillermocava.com/web-rest-api-benchmark-on-a-real-life-application/ https://github.com/mihaicracan/web-rest-api-benchmark

It would be interesting to compare Rust against node.js too! (actually that's the whole point)

We are still busy working on feature development of SeaORM, and we'd try to reduce the overhead later. Definitely not wanting to create a performance regression at some point, so the Diesel benchmark suite would be very helpful indeed.

Hopefully may be a contributor would come up. It's even better, because he'd be less biased too.

weiznich commented 3 years ago

It really painted the picture where all of us are at, and the comparison with SQLx is very useful as it illustrates the overhead SeaORM added on top.

Feel free to use those benchmarks as baseline, that's exactly the reason I've written them. The readme includes instructions in how to run those benchmarks and how to enable certain implementations. Results are published daily here as soon as the referenced PR is merged. If you want to use a updated SeaORM version in that benchmarks just submit a PR with the update. I'm also fine with using git versions there. The first thing I would likely look into in your position is where exactly the overhead of compared to Sqlx is coming from. Is this some unfortunate implementation detail that can be fixed later on quite easily or is it something more fundamental that may require redesigning certain parts of your crate.

In my imagination, the more 'real world' benchmark would be a web server, and we'll be counting the number of requests it can handle per second, similar to below:

My problem with such real world benchmarks is that they include to many variables to really say something about actual components of the benchmark system. In this case such a benchmark does include much more components than a database layer, which makes it not obvious if something is fast/slow because of the database implementation or the web framework or something else. In my opinion a benchmark should contain only one axis of variability, that one that you want to compare. In my specific case these are database interaction crates, so I prefer to have a benchmark that includes only those and ideally nothing else. That written I can understand your idea here, as it is really hard to show the benefit of async code in such a setup, but I really fear that such a whole stack framework benchmark essentially boils down to something that is more a web framework benchmark than a database layer framework.

Hopefully may be a contributor would come up. It's even better, because he'd be less biased too.

I should probably state that I've tried my best to not be influenced by my involvement in diesel while writing those benchmarks. I've tried to write down the assumptions behind each benchmarks as part of the Readme and I've tried to implement the best version for each crate that I'm aware of. If there was more than one possible solution I've implemented multiple versions of the same benchmark to see the trade offs between different API's. Also I try to gather feedback from the authors of the corresponding crates and I'm generally really open on code improvements on any of the implementations. So if you feel that a certain implementation is just not optimal just reach out and we will try to fix it. If you feel that some of the assumptions described in the benchmarks Readme is putting a disadvantage on a certain implementation, again please point that out and we will see how we can improve the "rules" and the implementation so that the benchmarks become more fair.

tyt2y3 commented 3 years ago

The first thing I would likely look into in your position is where exactly the overhead of compared to Sqlx is coming from. Is this some unfortunate implementation detail that can be fixed later on quite easily or is it something more fundamental that may require redesigning certain parts of your crate.

I think a part of it is unavoidable: the dynamic query building stage. But then, there should be a lot of room to improve, like using smallvec, and refactor internally to reduce memory reallocation etc.

My problem with such real world benchmarks is that they include to many variables to really say something about actual components of the benchmark system.

Okay, sorry for not made myself clear. We can have an actix web server, connecting to a database, doing some CRUD operations. The only variable will be the database driver, and then we will be able to tell whether a top-to-bottom async implementation actually makes a difference in terms of concurrency.

The comparison with node.js is a different game: we aim to compete with the node.js + expressjs + TypeORM (or any ORM) stack, with an entire Rust stack. I think we can draw more developers into the Rust ecosystem, if we can prove ourselves to be more performant while being comparatively productive.

If you feel that some of the assumptions described in the benchmarks Readme is putting a disadvantage on a certain implementation, again please point that out and we will see how we can improve the "rules" and the implementation so that the benchmarks become more fair.

Yes. I have looked through the benchmark, and my only suggestion to improvement would be to have a slightly more complex schema and queries. I will definitely take some time to think about it! But right now our priority is to 'make it work and ship it'.

I'd like to keep this issue open, so a person who cares would be motivated to contribute something.

weiznich commented 3 years ago

I think a part of it is unavoidable: the dynamic query building stage. But then, there should be a lot of room to improve, like using smallvec, and refactor internally to reduce memory reallocation etc.

I would expect a more fundamental issue there. Let me explain why: Diesel has two query building modes, one more or less static variant and a mire dynamic approach. Starting point for both is the QueryFragment trait. The static variant knows the types of every part of ther query ast tree. Therefore the compiler will inline almost anything an the whole query building compiles down to some string that are put together. We do not use any other optimizations there, just the ordinary string type from rusts std lib and just + to concat another string. The dynamic variant uses dyn QueryFragment trait objects, so the whole inlining is not possible there. Additionally prepared statement caching is handled slightly different for those cases. For the static variant we might just use the at compile time known type id as key (because the type represents this query) and skip query building at all as soon as a prepared statement already exists. For the dynamic case that's not possible, so we need to use the sql string as key for the cache and run the query builder every time.

Nevertheless all of this the difference between those two variants are just above noise level for the sqlite simple_query benchmark. The diesel variant corresponds to the static approach which basically skips query building and the boxed variant corresponds to the dynamic variant which builds the query every time. Given those minor differences I would argue that any advanced optimization there is probably not worth the work put into that. Also at leas for me this seems to indicate that there is something else going on for the sea-orm benchmarks. In the end there shouldn't happen that much more than just concating strings + some conditions. That's something at the nano second level, not the tenths of microsecond level.

Okay, sorry for not made myself clear. We can have an actix web server, connecting to a database, doing some CRUD operations. The only variable will be the database driver, and then we will be able to tell whether a top-to-bottom async implementation actually makes a difference in terms of concurrency.

The comparison with node.js is a different game: we aim to compete with the node.js + expressjs + TypeORM (or any ORM) stack, with an entire Rust stack. I think we can draw more developers into the Rust ecosystem, if we can prove ourselves to be more performant while being comparatively productive.

This would remove at least some variability. On the other hand it would introduce an inherent bias due to the usage of an async web framework. That's the reason why I prefer benchmarks without anything added on top.

Yes. I have looked through the benchmark, and my only suggestion to improvement would be to have a slightly more complex schema and queries. I will definitely take some time to think about it! But right now our priority is to 'make it work and ship it'.

Feel free to come back with some concrete proposal here. Adding more a few more complex queries would be definitively welcome.