djkoloski / rust_serialization_benchmark

Benchmarks for rust serialization frameworks
514 stars 48 forks source link

Add support for 'Savefile'-serialization framework #47

Closed avl closed 9 months ago

avl commented 9 months ago

Thanks for creating 'rust_serialization_benchmark'. It really is a comprehensive rust serialization benchmark, an excellent resource for everyone looking to choose a rust serialization framework!

I'm the author of the 'Savefile' serialization framework: https://crates.io/crates/savefile . Its selling point is that it supports data format versioning, and has the ability to store a schema to be able to detect incompatible versions, while still being a binary format that is suitable for storing large arrays quickly.

I'm wondering if you'd like to include Savefile-support in the benchmark suite?

I think I've implemented it mostly correct, but just tell me if there's something I should do differently. I haven't updated the README.md - is there perhaps some automated way to do that? :-) Maybe it should be done on the same machine as last time, so all numbers don't suddenly change?

I'm of course biased, and I've implemented Savefile to get the best performance, but hopefully without too much cheating :-) . I've disabled schema checking, since I don't think the other benchmarks do that in the way Savefile does.

Performance wise, at least on nightly, Savefile is basically competitive, but not the fastest. In the mesh benchmark it performs very well though.

djkoloski commented 9 months ago

Thanks for the PR!

I'm wondering if you'd like to include Savefile-support in the benchmark suite?

Absolutely! I also appreciate that you've taken the initiative to add support yourself, that helps out a lot. 🙂

I haven't updated the README.md - is there perhaps some automated way to do that?

Yes, the README will update automatically (about an hour) after the branch is merged.

Maybe it should be done on the same machine as last time, so all numbers don't suddenly change?

We're using the same image in CI every time, although that doesn't guarantee the same resources or performance characteristics. In general, these benchmarks are only intended to be comparative - measuring how crates perform relative to each other.

I've implemented Savefile to get the best performance, but hopefully without too much cheating

It looks pretty good, I did remove the manual repr attributes and unsafe attributes though. My reasoning for this is that all crates have to work from the same starting data, and that data should be what you'd write normally. Changing what you'd write based on the crate you choose is something I'd expect people to do eventually, but not here.

I've disabled schema checking, since I don't think the other benchmarks do that in the way Savefile does.

That's totally fine, though you're also free to add a variant for the deserialize benchmark that does perform schema checking. rkyv does this to benchmark its validated and unvalidated deserialization.

I'm going to go ahead and merge this, feel free to address any feedback in a follow-up PR. Thanks again!

avl commented 9 months ago

Cool!

I totally buy your argument about removing the unsafe attributes. It means savefile performs much slower (factor 10x) in the mesh benchmark, but of course the benchmark should be fair. I'll just have to come up with some way to recover this speed without relying on unsafe attributes :-) .

Thanks for merging!