birkenfeld / serde-pickle

Rust (de)serialization for the Python pickle format.
Apache License 2.0
185 stars 27 forks source link

Use criterion for benchmarks, replace read_bytes for small fixed-size allocations #21

Closed lkolbly closed 2 years ago

lkolbly commented 2 years ago

The criterion change isn't super important by itself, I was just using it because I like it for comparing changes (it will by default show a diff against the last run).

The read_fixed_{2,4,8}_bytes functions could be combined into a single function which uses const generics, but that would increase the MSRV so I opted not to.

This change improves performance typically by double-digit percentages:

biglist.pickle: -43%
manyrefs.pickle: -3%
manystrings.pickle: -18%
tests_py3_proto0.pickle: -16%
tests_py3_proto1.pickle: 0%
tests_py3_proto2.pickle: -13%
tests_py3_proto3.pickle: -13%
tests_py3_proto4.pickle: -30%
tests_py3_proto5.pickle: -9%
tests_py2_proto0.pickle: -26%
tests_py2_proto1.pickle: -34%
tests_py2_proto2.pickle: -10%
unpickle_list: -23%
unpickle_list_no_memo: -39%
unpickle_dict: -17%
unpickle_nested_list: -9%
unpickle_nested_list_no_memo: -2%
unpickle_simple_tuple: -47%
birkenfeld commented 2 years ago

Excellent, thanks! Unfortunately criterion also doesn't seem to support 1.41.1, can we make it somehow optional?

lkolbly commented 2 years ago

I think we can - I made it an optional dependency of serde-pickle itself, so you can run cargo test successfully on Rust 1.41.1. Then for cargo bench, you'll still get an error on Rust 1.41.1, but only because it compiles out the benchmarks so there's nothing to run. I could put in a stub benchmark if we want.

birkenfeld commented 2 years ago

Great, thanks!

birkenfeld commented 2 years ago

Released in v1.1.1.