execution/commitment: enchance MPT hashing hot-paths to reduce allocations#20796
execution/commitment: enchance MPT hashing hot-paths to reduce allocations#20796Sahil-4555 wants to merge 7 commits intoerigontech:mainfrom
Conversation
ac48acc to
73135f0
Compare
| // getDeferredUpdate handles copying the prefix into its own prefixBuf. | ||
| upd := getDeferredUpdate(prefix, bitmap, touchMap, afterMap, cells, prev) | ||
|
|
||
| be.pendingPrefixes.Set(upd.prefix, struct{}{}) |
There was a problem hiding this comment.
i don't understand this move
There was a problem hiding this comment.
pendingPrefixes map holds a reference to the byte slice that serves as its key. The incoming prefix input specifies a shared, mutable scratch buffer (compactKeyBuf). If we send it directly to be.pendingPrefixes.Set(prefix) causes the map to keep a memory reference that will be overwritten on the next iteration of the folding loop, damaging the map's internal keys.
By invoking getDeferredUpdate first, we transfer the prefix bytes into the job's stable, isolated memory (update.prefixBuf). We then give upd.prefix to .Set(). Moving the call down here allows us to utilize the securely isolated byte slice as the map key, completely avoiding the aliasing problem and not needing to allocate a new copy
|
also check last couple PR's here: https://github.com/erigontech/erigon/pulls/awskii |
There was a problem hiding this comment.
Pull request overview
This PR targets allocation/memory reductions in the execution/commitment Hex Patricia trie hot paths, primarily around key compaction/encoding and cell (de)serialization during fold/unfold and hashing.
Changes:
- Added a caller-buffer-based
HexToCompactBufand switched fold/unfold branch-key encoding to reuse per-trie scratch buffers. - Reduced per-cell/per-branch overhead by inlining
cell.fillFromFieldsparsing and removing array clearing incell.reset. - Reduced deferred-update allocations by embedding a fixed prefix buffer inside pooled
DeferredBranchUpdateobjects.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
execution/commitment/nibbles/nibbles.go |
Adds HexToCompactBuf as a zero-allocation alternative to HexToCompact. |
execution/commitment/hex_patricia_hashed.go |
Reuses scratch buffers for compact keys, inlines deserialization parsing, and avoids some allocation patterns in hashing paths. |
execution/commitment/commitment.go |
Changes deferred branch update prefix handling to avoid per-update allocations by copying into an embedded buffer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
73135f0 to
b600d7a
Compare
b600d7a to
ca2cde0
Compare
This PR includes targeted memory improvements to the
execution/commitmentMPT hashing hot-paths, resulting in considerable reductions in total allocation and execution time. The improvements are achieved by gradually replacing dynamic heap allocations with pre-allocated structural buffers. We specifically developed a zero-allocationHexToCompactBufmethod and added[128]bytescratch buffers to theHexPatriciaHashedobject to entirely minimize memory escapes while encoding keys during branch folding and unfolding. Furthermore, we avoided costly per-jobmake([]byte)allocations by embedding a[64]byteprefix array directly into the pooledDeferredBranchUpdatestruct, ensuring thread-local data separation for concurrent workers.we optimized the
cell.fillFromFieldsdeserializer's extraction logic is inlined to eliminate closure and slice-of-structs overhead. Finally, we eliminated unnecessary array-clearing processes incell.reset()(using existing length-gated reads instead) and changedcomputeCellHashto use stack-allocated buffers for prepending state hashes.