Skip to content

perf(mpt): compare node reference digests as words#677

Open
Qumeric wants to merge 1 commit into
valery/guest-keccak-spongefrom
valery/mpt-digest-eq
Open

perf(mpt): compare node reference digests as words#677
Qumeric wants to merge 1 commit into
valery/guest-keccak-spongefrom
valery/mpt-digest-eq

Conversation

@Qumeric

@Qumeric Qumeric commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Decoding compares every node's reference against the one its parent stated. bytes_eq walks those
byte by byte, which costs about five instructions per byte: two loads, two pointer bumps and a
branch. At 32 bytes that is ~160 instructions per comparison, and the decoder performs one per node.

The comment on bytes_eq explains why it was written that way — slice == becomes a memcmp call
— and that is still true for slices whose length the compiler cannot see. It is not true when the
length is fixed: the guest target enables +unaligned-scalar-mem, so a 32-byte comparison compiles
to eight word loads and an XOR chain, 17 straight-line instructions with no call and no loop.

Three of the four call sites compare a full digest, so they go through a digest_eq helper that
fixes the length. The fourth compares a reference shorter than 32 bytes and keeps the byte loop,
which is still the cheaper form there. digest_eq is correct for any input because slices of
different lengths are never equal, so a reference that is not a full digest is simply inequality.

execute_metered_insns 616,699,072 -> 587,583,276, -4.721% (block 24001988, execute-metered).
Rows -4.577%, main cells -1.601%, interaction cells -2.883%, metered memory -0.836%.

The new test covers the direction that matters for soundness: a corrupted digest must be rejected,
including when only its last byte differs, which is what a comparison that stopped short would miss.

The length is fixed here because NodeRef::Digest is documented as always 32 bytes, but the variant
holds a &[u8] and the invariant is only enforced by a debug_assert that release builds drop.
Changing it to &[u8; 32] would make this helper unnecessary and enforce the invariant in the type;
left as a follow-up so this change stays measurable on its own.

The digest comparison during decoding ran a byte at a time, five instructions per byte, because
a slice comparison of unknown length becomes a `memcmp` call. Fixing the length to 32 at the
three sites where one side is a full digest lets it compile to four word loads per side and an
XOR chain instead. Shorter references keep the byte loop, which is still the cheaper form for
them.
@Qumeric
Qumeric requested a review from shayanh July 25, 2026 11:40
@Qumeric
Qumeric marked this pull request as ready for review July 25, 2026 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant