fission-codes / go-car-mirror

Generic Go implementation of the CAR Mirror protocol
Apache License 2.0
4 stars 0 forks source link

Bloom simplifications / optimizations #81

Open justindotpub opened 1 year ago

justindotpub commented 1 year ago

Notes captured from dialog with @expede and @matheus23.

The current approach to Bloom Filters in go-car-mirror is quite flexible and makes it possible to deal with bloom capacity overflow, but it adds extra complexity for implementors. Brooke's recommendation is to start with something simpler.

Only support XXH3 per the spec.

Take the XXH3 hash, and then save bits of certain sizes around such that you can JIT convert to bitfield indices based on the size of the bitfield chosen.

Also, if you keep around an array of precomputed indices without flattening them to a bitfield (i.e. [99, 42, 5014, 139, ...]), you can take however many of them you need for your size of bloom. There’s a bit more detail, but TL;DR you can do very performant things like query subgraphs of blooms from an in-memory cache and merge a bunch of them together (often in the query directly) and then flatten to a Bloom.

...

More details need to be spelled out here, but this issue is for getting the code back to spec compliance with regard to bloom usage / encoding, for the initial version of batch support.

ETA: 2023-10-31

justindotpub commented 1 year ago

As I've started testing with different sizes for MaxBlocksPerRound (and MaxBlocksPerColdCall) I've discovered that the current code will nest blooms more than 32 deep, leading to a CBOR error.

cbor: exceeded max nested level 32

Looks like this may need to be addressed sooner.