Serialization benchmarks.

[arsh]

Description of change

1. Using Criterion benchmarks, I want to show how we see a performance gain (~33% faster) by switching from bincode to bincode2 when reading the cached blocks from disk.
2. I want to get feedback on changes made to DiskDataCache to abstract out the serialization library. For example, am I doing the right abstraction? Am I using the trait boundaries in the right way? So, mostly on the Rust-side of things.

Benchmarks results

In summary, using bincode2 gets a better performance than bincode and closer to the base line of reading the contents of a file to memory (read_file in the graphs). From flamegraphs, we noticed that bincode was zeroing the vectors in its fill_buffer implementation. bincode2 has a different implementation which is more efficient.

Reading a file (base line)

Reading cache through bincode

Reading cache through bincode2

Relevant issues: N/A

Does this change impact existing behavior?

No

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

[passaro]

This is great! I'll add a few specific comments, but first:

I want to get feedback on changes made to DiskDataCache to abstract out the serialization library

Do we need to abstract the serialization library? Any reason not to just replace it?

[arsh]

This is great! I'll add a few specific comments, but first:

I want to get feedback on changes made to DiskDataCache to abstract out the serialization library

Do we need to abstract the serialization library? Any reason not to just replace it?

I wanted to test them at the same time and needed ability to configure it and thus this abstraction. We don't necessarily need it. However, we could be in the same situation in the future where we want to test with a new library and it would be nice to have this in place to just plug the new one in.

[jamesbornholt]

This is cool, but bincode2 hasn't been updated for 4 years and looks unmaintained, so not sure we should be switching to it.

[jamesbornholt]

I don't like that we have to take a non-dev-dependency on "all the versions we want to benchmark" here. I'd rather put it behind a feature flag, or just not commit the versions we're not using.

[arsh]

This is cool, but bincode2 hasn't been updated for 4 years and looks unmaintained, so not sure we should be switching to it.

bincode 1.x is in a similar boat. The version we use, 1.3.3, was released 3 years ago. After that, bincode switched to 2.x. These 2.x versions are marked with rc which I presume means release candidate and not production ready? Since 2.x claims to be a re-write, I did the same benchmarks using it and did not see the improvements we got from bincode2.

One way forward is to switch to bincode = 2.x, do perf analysis to determine why its performance isn't as good as bincode2, and potentially submit a PR to fix it.

More importantly, I was hoping this micro-optimization would help with the overall cache performance as described in #719 but when I tested it following what is described there, I did not see an improvement, which looks like something else (to be determined) is the bottleneck.