perf(core): decode aggregated bloom filter rows on demand via raw-blob views - #4022
infrmtcs-agent[bot] wants to merge 1 commit into
Conversation
|
Claude finished @infrmtcs-agent[bot]'s task in 4m 36s —— View job Review: PR #4022 — decode aggregated bloom filter rows on demand
Summary: This is a clean, well-executed optimization. No blockers or important issues found. Nits (informational, not blocking):
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4022 +/- ##
==========================================
- Coverage 79.48% 79.35% -0.13%
==========================================
Files 466 466
Lines 36009 36025 +16
==========================================
- Hits 28620 28587 -33
- Misses 7380 7429 +49
Partials 9 9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@infrmtcs can you add some numbers to this PR |
| // indices are raw bloom locations, not yet reduced modulo EventsBloomLength. | ||
| intersectRows(rawIndices []uint64, innerMatches *bitset.BitSet) error | ||
| // indices are already reduced modulo EventsBloomLength. | ||
| intersectRows(indices, acc []uint64) error |
There was a problem hiding this comment.
Nit: acc -> accumulator or matchWords
| } | ||
|
|
||
| // rowWords validates row index's length prefixes and returns its raw word bytes. | ||
| func (r blobRows) rowWords(index uint64) ([]byte, error) { |
There was a problem hiding this comment.
Nit: may rename to rowWordBytes or rowBytes since it doesn't return words
| offset := filterHeaderSize + int(index)*filterRowSize | ||
| // bitsetLen and blobLen are independent fields, so both are checked. | ||
| if blobLen := int(binary.BigEndian.Uint32(r[offset:])); blobLen != filterRowBlobLen { | ||
| return nil, ErrBloomFilterSizeMismatch |
There was a problem hiding this comment.
Nit: can be wrapped with context to avoid ambiguity with error below
| } | ||
| offset += filterRowLenSize | ||
| if bitsetLen := binary.BigEndian.Uint64(r[offset:]); bitsetLen != NumBlocksPerFilter { | ||
| return nil, ErrBloomFilterSizeMismatch |
There was a problem hiding this comment.
Nit: can be wrapped with context to avoid ambiguity with error above
| } | ||
|
|
||
| // parseHeader validates the blob framing and sets v's block range. | ||
| func (v *filterView[R, PR]) parseHeader(data []byte) error { |
There was a problem hiding this comment.
The four checks here return two bare sentinels between them, worth wrapping with the got/want values so a corrupt blob says which check failed instead of just unexpected EOF or bloom filter len mismatch.
b947d69 to
5d05b4d
Compare
Review in progress
|
3 participants
User description
Summary
PR Type
Enhancement, Tests
Description
Optimize bloom filter decoding performance
AggregatedBloomFilterViewto decode rows on demand from raw-blob views.intersectRowsand bitset manipulations.Refactor cache and core blockchain logic
AggregatedBloomFilterCacheto manage and return raw-blob views.blockKeyFilterinterface to support both filters and views.GetAggregatedBloomFilterViewdatabase accessor.Improve testing and benchmarking
AggregatedBloomFilterand its view.BlocksForKeysIntobenchmarks to evaluate both implementations.File Walkthrough
7 files
Update cache to use raw-blob views instead of decoded filtersUpdate blockchain fallback to fetch aggregated bloom filter viewsIntroduce blockKeyFilter interface for event matching abstractionAdd GetAggregatedBloomFilterView for database retrievalRefactor BlocksForKeys to utilize BlocksForKeysIntoImplement blobRows and optimize row intersection logicDefine AggregatedBloomFilterView and optimize key matching logic3 files
Adapt cache tests to use raw-blob views helperAdd benchmarks for filter and view BlocksForKeysInto methodsParametrize tests to cover both decoded filters and raw-blob views