perf(l1): add binaryandhash data block index to all rocksdb CFs for faster point lookups#6529
perf(l1): add binaryandhash data block index to all rocksdb CFs for faster point lookups#6529dicethedev wants to merge 2 commits intolambdaclass:mainfrom
Conversation
Greptile SummaryThis PR enables Confidence Score: 4/5Safe to merge — no correctness or data-integrity issues; only P2 style and trade-off concerns. All findings are P2: an inaccurate hash-ratio comment replicated across every arm, and a debatable choice to apply BinaryAndHash to sequentially-read CFs (HEADERS/BODIES) where it provides no point-lookup benefit and adds minor block-cache overhead. The change itself uses the RocksDB default value, is backward-compatible, and is functionally correct. crates/storage/backend/rocksdb.rs — specifically the HEADERS | BODIES arm where BinaryAndHash trades cache space for a benefit that matters less for sequential reads.
|
| Filename | Overview |
|---|---|
| crates/storage/backend/rocksdb.rs | Adds DataBlockIndexType::BinaryAndHash with hash_ratio=0.75 (RocksDB default) to all CF configurations; change is backward-compatible but the inaccurate hash-ratio comments and application to range-scan-dominant CFs are minor concerns. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CF open loop] --> B{match cf_name}
B -->|HEADERS, BODIES| C[32KB block + BinaryAndHash 0.75 + block_cache]
B -->|CANONICAL_BLOCK_HASHES, BLOCK_NUMBERS| D[16KB block + bloom_filter + BinaryAndHash 0.75 + block_cache]
B -->|ACCOUNT_TRIE_NODES, STORAGE_TRIE_NODES| E[16KB block + bloom_filter + BinaryAndHash 0.75 + memtable_prefix_bloom + block_cache]
B -->|ACCOUNT_FLATKEYVALUE, STORAGE_FLATKEYVALUE| F[16KB block + bloom_filter + BinaryAndHash 0.75 + memtable_prefix_bloom + block_cache]
B -->|ACCOUNT_CODES| G[32KB block + blob_files + BinaryAndHash 0.75 + block_cache]
B -->|RECEIPTS| H[32KB block + BinaryAndHash 0.75 + block_cache]
B -->|default| I[16KB block + BinaryAndHash 0.75 + block_cache]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 114
Comment:
**Misleading hash-ratio comment**
The comment "Hash index covers 75% of entries for good performance" is inaccurate. `data_block_hash_ratio` is not a coverage percentage; it is the ratio of hash-table slots to the number of entries in the data block. A value of `0.75` means 0.75 slots are allocated per entry, which is the RocksDB default and is perfectly fine — but the description is misleading. The ratio controls the hash-table's space budget relative to key count, not "which fraction of entries are indexed."
The same misleading comment is repeated in every match arm (including the one at line 127 that has an extra leading space: `// Hash index for faster lookups`).
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 113-116
Comment:
**`BinaryAndHash` interacts poorly with range-scan dominant CFs**
`HEADERS` and `BODIES` are primarily read sequentially (e.g. during initial block download), not via exact-key point lookups. `DataBlockIndexType::BinaryAndHash` only accelerates point lookups; for sequential iteration RocksDB still uses the binary-search restart array, and the extra hash-table bytes within each 32 KB block are loaded into the block cache for no benefit. This inflates block-cache pressure for the heaviest sequential-access CFs. Consider keeping `BinarySearch` (the default) for `HEADERS | BODIES`, and apply `BinaryAndHash` only to the CFs where random point lookups dominate (`ACCOUNT_TRIE_NODES`, `STORAGE_TRIE_NODES`, `ACCOUNT_FLATKEYVALUE`, `STORAGE_FLATKEYVALUE`).
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "add BinaryAndHash data block index to al..." | Re-trigger Greptile
Motivation
Point lookups into RocksDB data blocks currently use binary search. For trie nodes and flat key-value stores, which are heavily read during block execution, this adds unnecessary CPU overhead on each block scan.
Description
Enables
DataBlockIndexType::BinaryAndHashwith a hash ratio of0.75on all column families. This adds a hash index inside each SST data block so point lookups hit O(1) hash probe instead of O(log n) binarysearch, at the cost of ~25% extra space within each data block.
Applied to all CFs:
HEADERS,BODIES,CANONICAL_BLOCK_HASHES,BLOCK_NUMBERS,ACCOUNT_TRIE_NODES,STORAGE_TRIE_NODES,ACCOUNT_FLATKEYVALUE,STORAGE_FLATKEYVALUE,ACCOUNT_CODES,RECEIPTS, and the default arm.Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PRincludes breaking changes to the
Storerequiring a re-sync.Closes #5941