Optimize ReadCacheLookup, 6.7 speed-up

[arvidn]

This is a major hot-spot when serializing with back references. This patch speeds it up by:

• pre-allocating hash maps and hash sets
• using IdentityHash, to avoid hashing the tree-hashes again
• exiting early if the structure we're trying to deduplicate is too small (i.e. trade-off compression in favor of speed)
• use BitVec instead of Vec<u8> to store paths (which only contain 0 and 1 anyway).

Compared to main, the relevant benchmarks indicate a time reduction of between 80-90%, of serializing clvm with back references.

$ cargo bench node_to_bytes_backrefs -- --baseline main
     Running benches/serialize.rs (target/release/deps/serialize-21b0d813d80dbf8a)
Benchmarking serialize/node_to_bytes_backrefs 0: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 41.6s, or reduce sample count to 10.
serialize/node_to_bytes_backrefs 0
                        time:   [417.23 ms 420.23 ms 423.98 ms]
                        change: [-85.682% -85.544% -85.390%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  4 (4.00%) high mild
  7 (7.00%) high severe
Benchmarking serialize/node_to_bytes_backrefs 1: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 48.2s, or reduce sample count to 10.
serialize/node_to_bytes_backrefs 1
                        time:   [478.85 ms 481.87 ms 485.32 ms]
                        change: [-91.144% -91.077% -91.007%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 19 outliers among 100 measurements (19.00%)
  8 (8.00%) high mild
  11 (11.00%) high severe

One concern is that, not mixing in any random state in the hasher, opens up for hash-collision attacks. I'm not sure if it's worth addressing or what the best way would be. Maybe add a random value to the IdentityHash object, and add it to the hash value.

[coveralls-official]

Pull Request Test Coverage Report for Build 12546252319

Details

• 64 of 66 (96.97%) changed or added relevant lines in 2 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.01%) to 93.599%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/serde/identity_hash.rs	17	19	89.47%

Files with Coverage Reduction	New Missed Lines	%
src/serde/read_cache_lookup.rs	1	98.51%

Totals
Change from base Build 12517795391:	-0.01%
Covered Lines:	5878
Relevant Lines:	6280

---

💛 - Coveralls

[richardkiss]

Looks good. I'd be curious to know if the u8 -> bool change does anything. But it seems at worst harmless.

[arvidn]

There are still some worst-case scenarios where this isn't fast enough to serialize a full block. I'm working on a more ambitions PR to speed this up more.

[arvidn]

a material portion of the speed-up in this patch comes from not using a salt in the hasher. Which is a bit surprising, but I suppose we initialize new HashSet and HashMaps quite often. adding back the salt makes the benchmarks take 3x the time I wasn't doing it right. fixed now. I've added two new commits

[richardkiss]

Looks good