perf(zkevm): install the hash seed from the payload root - #13166
perf(zkevm): install the hash seed from the payload root#13166LukaszRozmej wants to merge 17 commits into
Conversation
|
Claude finished @LukaszRozmej's task in 5m 26s —— View job Review:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
rubo
left a comment
rubo
left a comment
There was a problem hiding this comment.
Better to address Claude's nits.
The seeded lane arrays were built by a static initializer, which gave the type a class constructor - so every mixer call, the hottest leaf in the guest, paid a class-initialisation check, a fence and a two-level static load. The guest now installs the seed once from StatelessExecutor before anything hashes a key, and the type carries no initializer at all, so no class constructor is emitted. The seed becomes a parameter rather than a literal, which is where it needs to be when the payload starts carrying it.
MemoryMarshal.GetArrayDataReference carries no null check, so an unseeded lane array traps on an unmapped address rather than throwing, and the short-input path never touches the arrays at all - it hashes with seed zero and silently succeeds. A DEBUG-only assertion on the three guest hash entries catches the ordering bug in the ZkEvm suites at no cost to the release guest. The seeding obligation moves from GuestMixerTests to an assembly-scoped SetUpFixture, so a new fixture in the assembly cannot forget it, and a test pins the type's lack of a class constructor - the property the whole optimisation rests on, which a static field initializer in any partial would silently undo. InstanceRandom, now mutable, drops to internal.
e9442de to
55c4d86
Compare
The base branch was changed.
|
@claude re-review |
|
Claude finished @LukaszRozmej's task in 3m 47s —— View job Re-review:
|
| prior finding | state |
|---|---|
Medium — "fails loudly" is wrong; short-input path silently hashes with seed 0 |
Fixed. AssertSeeded() guards MixAddress, Mix32 and FastHashFallback — the third is exactly the path that never touches the lane arrays. The <remarks> on AssertSeeded now records the trap and the seed-0 case as the real failure modes, and the PR body matches. |
| Low — per-fixture seeding, per-process state | Fixed. Namespace-less [SetUpFixture] in SeedGuestHashes.cs wraps the whole assembly; the [OneTimeSetUp] is gone from GuestMixerTests. |
Low — public on a writable static |
Fixed. internal static uint InstanceRandom in the guest partial, host partial keeps public static readonly. The correction about Hash256.cs:73 is right — same assembly, internal reaches it. |
| Low (from @wurdum) — nothing pins the no-cctor property | Fixed. GuestHashSeedTests.Guest_hash_type_has_no_class_constructor. I checked the shared partial: all the PairRoundN members are properties, not fields, and the static lambda at SpanExtensions.cs:123 caches into the generated <>c nested type, not into SpanExtensions — so the assertion holds today and will genuinely fail if a field initializer is re-added. |
Assert coverage is complete: FastHash64For20BytesFallback and FastHash64For32BytesFallback both route through MixAddress/Mix32, and ValueHash256.GetHashCode reaches the mixers via Bytes.FastHash(), so there is no fourth entry point past the guard.
Two Low findings, both about the guard's stated reach rather than the code.
Low
1. The [Conditional("DEBUG")] assert never runs in CI — SpanExtensions.zkevm.cs:86
stateless-tests.yml:32-34 dispatches all three ZkEvm suites with -c release, and Nethermind.Core is a ProjectReference built in the same configuration — so DEBUG is undefined and every AssertSeeded() call site is stripped. It fires only in a local Debug run. That undercuts the PR body's "turns a violation into a failed assert in the ZkEvm suites", and the stated reason for leaving Nethermind.Trie.ZkEvm.Test unseeded. No live hazard — that project is only GuestNibblesTests and Nethermind.Evm.ZkEvm.Test only GuestJumpDestinationTests, neither of which hashes — so this is a wording fix, unless you want the guard to be real in CI, which would take a symbol the ZkEvm test builds define and the guest publish does not.
2. A comment was separated from the field it documents — SpanExtensions.std.cs:15-20
The pre-existing "Ensure that hashes are different for every run of the node…" lines describe InstanceRandom on line 22; the new SeedHashes declaration was inserted between them, so they now read as documentation for the no-op partial. Placing SeedHashes after the field fixes it and shrinks the diff.
Also considered, no finding
- ILC preinitialization now sees
InstanceRandom == 0. This is the inverse of what I noted last round and worth stating correctly: before the change the compile-time value was the real literal, so anything ILC froze was consistent with runtime. Now a cctor that ILC preinitialized and that hashes would bake seed-0hash codes into frozen data, then miss every lookup afterSeedHashesruns — silent, not loud. I do not think it is reachable: the static tables that would matter areFrozenDictionary<AddressAsKey, CodeInfo>(EthereumPrecompileProvider.cs:15), whose construction is far past what ILC's preinit interpreter handles, and a frozen-with-seed-0precompile table would have changed the guest output for block 25532382 rather than leaving it byte-identical. Flagging it as the thing to re-check if a static hash table is ever added to the guest's reachable set. - Re-seeding. Still idempotent (
ExecutepassesDefaultHashSeed), still a no-op on the host. When the seed becomes payload-derived, re-seeding with live static caches orphans every entry; theCall once, before anything hashes a keyline in the XML doc carries that contract. Follow-up, not this PR. - Thread safety of
SeedHashes. Two non-atomic static writes, but the guest is single-threaded and the[SetUpFixture]runs before any fixture. Fine. - Comment quality. The four added comment blocks all explain why (the cctor/fence cost, the null-vs-trap distinction, the assembly-wide scope, the CI blind spot). None restate adjacent code; nothing to flag.
Not verified
Builds still require approval in this environment, so neither partial was compiled and no suite was run. In particular I did not confirm that <inheritdoc cref="SpanExtensions.SeedHashes" /> on the implementing declaration of a partial method resolves the way you'd want — Roslyn's handling of doc comments on partial implementations is worth a glance at the build warnings, since the <remarks> on the host partial is the only place the no-op behaviour is written down.
Nothing here blocks merge.
· branch perf/zkevm-seeded-lanes
|
@claude re-review at head 65e79e1. The latest commit adds CRC collision regressions and removes the ineffective benchmark dependency condition. Please assess the replies to the four latest findings, especially the evidence that passing raw tiny-path data to CRC recreates a seed-independent collision family. The new path hash before host CRC chaining is intentional. Validation: 384 BytesTests, 93 TreePathTests, all five affected cases with AES disabled, and a clean benchmark build. The AES hot-member/base comparison and full payload A/B remain outstanding; please keep that performance finding open rather than treating these tests as performance evidence. |
|
Claude finished @benaadams's task in 5m 56s —— View job Re-review:
|
| latest finding | state |
|---|---|
| Medium — remove the pre-hash before CRC chaining | Withdrawn. The evidence is correct and the substitution I proposed is unsafe. One residual point about the guest side below. |
| Medium — AES hot-member / payload A/B unmeasured | Open, as requested. Nothing in this commit is treated as performance evidence. |
Low — inert EnableZkEvm condition on the benchmark package |
Fixed. Condition dropped; the explicit direct reference is right, since FastHashBenchmarks calls XxHash3 itself and shouldn't rely on Nethermind.State.Flat continuing to expose it. |
| Low — CRC-derived inputs no longer targeted the function under test | Fixed. They now probe the actual host CombineHash, plus a production-level TinyTreePath regression. Two notes on the mechanics below. |
Medium (open, per request)
1. The extra AES round is still unmeasured on the members it hits hardest — SpanExtensions.cs:394. Unchanged by this commit and explicitly left open. FastHashAes appends a round on every host span hash ≥ 16 bytes (so ValueHash256.GetHashCode() goes 2 → 3 rounds), FastHash64For20/32Bytes gained a seed XOR in round 2, and neither ValueHash256.GetHashCode() nor AddressAsKey.GetHashCode() appears in the table with a base comparison. The only EXPB run predates every hunk here and already showed realblocks +2.40 % / fusaka +1.61 %.
Low
2. The guest pays for the pre-hash without needing it — TinyTreePath.cs:52. The guest CombineHash is a seeded 16-byte MixShortBytes — not affine, no kernel — so the host's reason doesn't apply there, and the guest instead pays an extra 8-byte MixShortBytes and loses 32 of _data's 64 bits to the int truncation. Not a change request: the only fix is a #if ZK_EVM branch here, and removing build-condition branches from the chaining sites is a stated goal of this PR. Recorded because guest step count is the number this PR exists to move.
3. The seed loop adds no independent samples — BytesTests.cs:787-795. CombineHash(seed, v) is injective for v < 2³² (every nonzero kernel element has its top bit at ≥ 32), so the new comparison is equivalent to hash0 == hash1 for every seed. equalPairs is therefore exactly 3× the collision count: Is.LessThan(4) now tolerates one collision where it tolerated three, and the count * HashSeeds.Length denominator reports 3,072 samples for 1,024 comparisons. Tightening is good; happening by accident isn't.
4. rawHashes.Count == 1 encodes host-CRC behaviour unguarded — TreePathTests.cs:68. The BytesTests analogue is #if !ZK_EVM; this one isn't. Safe only because Nethermind.Trie.ZkEvm.Test is a separate directory holding one file, so the glob never reaches TreePathTests.cs.
Also considered, no finding
- A cheaper nonlinear step.
CombineHash(seed, (ulong)MumFold((ulong)_data, K))would break affinity for one multiply instead ofLoadShortInput+ 2 AES rounds, andMumFoldis alreadypublic. I'm not proposing it: with a fixed publicKthe map is offline-solvable, so it would trade a keyed hash (hostAesHashSeedis per-process) for an unkeyed one — a different, weaker version of the same problem. Making it keyed needsInstanceRandom, which ispublicon the host butinternalin the guest partial, so a uniform seeded form would mean either new surface or a#if. The retainedFastHashis the defensible choice. Hashing_distinguishes_zero_paths_of_different_lengthsnow covers(tiny, chained)=(false,false),(true,false),(true,true)and asserts exactlymaximum + 1. The non-tiny arm is deterministic, not seed-lucky: nonzero CRC32C kernel elements have degree ≥ 32, so lengths 0…64 cannot collide.- Guest benchmark builds.
FastHashBenchmarks.cscarries#if ZK_EVMblocks, so dropping the csproj condition means a guest-configuration build ofNethermind.Benchmarknow resolves an unusedSystem.IO.Hashing. Harmless — the project isn't inBFLAT_REFSand the guest arms don't call it. - Comment quality. One comment added this round (
// Shifted copies of the reflected CRC32C generator leave the checksum unchanged) and it is exactly the kind that should exist: it states the invariant that makes the magic constant legible, and I verified it is accurate. Nothing to flag.
Not verified
Builds require approval in this environment, so nothing was compiled and no suite was run — the 384 / 93 / AES-disabled results and the clean benchmark build are taken from the PR body, not reproduced. Also unchecked by machine: <inheritdoc /> on the two partial implementing declarations of CombineHash/SeedHashes resolving without a doc warning, and the guest step-count effect, which the PR body correctly lists as outstanding.
Nothing new blocks merge. The one thing I'd still want before merge is finding 1's measurement, kept open as asked.
· branch perf/zkevm-seeded-lanes
|
@claude re-review the two test fixes in 22f4b61: count each collision pair once with an explicit threshold, and guard the host-only CRC regression. All five affected cases pass with AES enabled and disabled. The guest pre-hash cost is recorded as a follow-up, and the AES hot-member/payload performance finding remains open. No production hashing changed. |
|
Claude finished @benaadams's task in 2m 13s —— View job Re-review: test fixes in
|
EXPB Benchmark ComparisonRun: View workflow run superblocksScenario: Client Processing (SSE)
K6 TTFB
realblocksScenario: Client Processing (SSE)
K6 TTFB
fusakaScenario: Client Processing (SSE)
K6 TTFB
|
EVM Opcode Benchmark DiffAggregated runs: base=3, pr=3 Regressions (1)
|
…anes # Conflicts: # src/Nethermind/Nethermind.Evm/TransactionSubstate.cs
`InputDecoder.Decode` installs the process-wide hash seed, and `ZkEvmBlockchainTestFixture` inherits `ParallelScope.All` from `PyspecBlockchainFixtureBase` - its `parallel: false` argument is `ParallelExecutionOverride`, the block-processing knob, not NUnit's scheduling. So a reseed could land while a `WitnessMatchesFixture` case was mid-execution, leaving the containers it had already populated unreachable; the seed API is explicitly not synchronised against concurrent hashing. `StatelessSchemaTests` also decodes but is a plain fixture in an assembly that marks no parallelism, so it already runs on the non-parallel shift and needs nothing.
NodeKey hashed on the keccak's raw leading four bytes, the one guest container whose bucket does not depend on the run seed. Node hashes that share a bucket are then found by one offline grind and hold for every block and payload, which is what SeedHashes exists to prevent, and the store is the largest dictionary in the guest. Nothing rejects witness nodes that no trie walk reaches, so they need not be reachable state. Route the hash code through FastHash64For32Bytes, seeded per run in the guest and per process on the host. The leading-bytes test asserted a precondition that the seeded hash no longer meets, so it searches for a colliding pair instead of assuming one.
|
@claude re-review |
|
Claude finished @LukaszRozmej's task in 9m 33s —— View job Re-review:
|
| /// the guest's target. The hash code goes through the run-seeded mixer rather than the keccak's own | ||
| /// leading bytes: those are uniformly distributed, which answers accidental collisions but not a | ||
| /// chosen witness. Unseeded, one offline grind yields node hashes sharing a bucket for every block | ||
| /// and payload, and nothing here rejects unreachable witness nodes. See | ||
| /// <see cref="SpanExtensions.SeedHashes"/>. |
There was a problem hiding this comment.
Medium — this is the one container where the payload-root seed is not a fixed point, so the protection is weaker than the remark implies.
The change is right and strictly better than the leading-bytes hash. But the threat model recorded here reads like the one that covers the storage-slot maps, and for this store it does not hold, because the seed's preimage excludes the witness:
// InputDecoder.cs:47-51
NewPayloadRequest<TExecutionPayload>.Merkleize(input.NewPayloadRequest, out UInt256 root);
SpanExtensions.SeedHashes(in root);NewPayloadRequest is { ExecutionPayload, VersionedHashes, ParentBeaconBlockRoot, ExecutionRequests } — StatelessInput.Witness is a sibling field and is not merkleized into root. Everywhere else in this PR the circularity is what does the work: change the colliding key set and you change the root that seeds the mixer. Here the witness supplier fixes the payload, reads the seed straight off it, and then grinds node blobs against a known seed. No fixed point to solve.
Cost of that grind, with n witness nodes and B ≈ n buckets: ~B keccaks to land one node in a chosen bucket, so ~n² total. At n = 32k that's ~10⁹ keccaks — hours on commodity hardware, and it lands in the constructor (line 39), before execution. And as the remark itself says, nothing rejects unreachable nodes, so the padding is free.
So the honest statement is that seeding converts a one-time universal grind into a per-payload O(n²) one, rather than removing it. Worth saying, precisely because the rest of the PR's argument is stronger than that and a reader will carry it over. (No change requested to the code — closing the gap properly means bounding the witness or rejecting unreachable nodes, which is out of scope here.)
| public override int GetHashCode() => | ||
| (int)SpanExtensions.FastHash64For32Bytes(ref Unsafe.As<ValueHash256, byte>(ref Unsafe.AsRef(in _hash))); |
There was a problem hiding this comment.
Medium — the cheapest hash in the guest's largest dictionary becomes the most expensive one, and the line this replaced carried a measurement.
Before: Unsafe.As<ValueHash256, int> — one load. After, in the guest (Aes.IsSupported is false on riscv64):
FastHash64For32Bytes → FastHash64For32BytesFallback → Mix32 (4 unaligned 64-bit loads + 4 seed XORs) → MixWords, which is [MethodImpl(NoInlining)] under ZK_EVM → 2 × MultiplyFold + MumFold, and MultiplyFold under ZK_EVM is the 4-way 32-bit decomposition. That's ~12 multiplies plus a non-inlined call, per probe, on Get/Set/KeyExists — i.e. every trie node resolve, plus n insertions in the constructor.
What makes this worth a number rather than a shrug is the sentence the diff deletes: "Reading the leading word and truncating it instead measured 0.15% worse." The team measured a variation far smaller than this one. The PR body says native RISC-V execution was not rerun, and this commit post-dates the body's description entirely.
A guest step count for 25532382.ssz at 24d03d0 vs 22f4b61 would settle it. If it turns out non-trivial, worth measuring a second arm: MumFold(Unsafe.ReadUnaligned<ulong>(ref …), InstanceRandom.u0) — one MultiplyFold instead of three. It keeps the property, because forcing a bucket without knowing the seed then requires an n-way multicollision on the full 64-bit leading word of keccak (~2⁶⁴), rather than the 32-bit prefix agreement the old form needed.
| private static (ValueHash256, ValueHash256) FindHashCodeCollision() | ||
| { | ||
| const int Attempts = 1 << 19; | ||
| Dictionary<int, ValueHash256> seen = new(Attempts); | ||
|
|
||
| for (int i = 0; i < Attempts; i++) | ||
| { | ||
| ValueHash256 candidate = ValueKeccak.Compute(MemoryMarshal.AsBytes(new ReadOnlySpan<int>(in i))); | ||
| int hashCode = NodeKeyHashCode(in candidate); | ||
|
|
||
| if (seen.TryGetValue(hashCode, out ValueHash256 previous)) | ||
| { | ||
| if (previous != candidate) return (previous, candidate); | ||
| } | ||
| else | ||
| { | ||
| seen[hashCode] = candidate; | ||
| } | ||
| } | ||
|
|
||
| Assert.Fail($"No hash-code collision within {Attempts} keys."); | ||
| return default; | ||
| } |
There was a problem hiding this comment.
Medium — the rewrite drops the coverage the old test existed for, and the fix is a two-line change to the search.
The old pair was first = 0…0, second = 0…01: identical in words 0–2, differing only in the last byte. Under the old hash code (leading four bytes) they shared a bucket, so the lookup could only succeed if the hand-rolled Equals compared the fourth word. That is exactly what the class's remarks say is spelled out by hand — Unsafe.ReadUnaligned<ulong> four times — and therefore the thing most likely to be got wrong.
The new pair is two unrelated keccaks that happen to share a 32-bit hash code. They differ in word 0 with probability ~1, so an Equals truncated to words 0–2 still passes this test. Nothing else in the file covers it: every other case uses keys that land in different buckets, where Equals is never consulted.
Both properties are recoverable at the same cost, because Set takes an arbitrary ValueHash256 — the candidates need not be real keccaks. Vary a counter in one word of an otherwise fixed value and birthday-collide on the hash code; the pair then differs only in that word:
[Test]
public void Separates_keys_that_share_a_hash_code([Range(0, 3)] int word)
{
(ValueHash256 first, ValueHash256 second) = FindHashCodeCollision(word);
…
}
private static (ValueHash256, ValueHash256) FindHashCodeCollision(int word)
{
…
ValueHash256 candidate = default;
BinaryPrimitives.WriteInt64LittleEndian(candidate.BytesAsSpan[(word * 8)..], i);
…
}That pins all four comparisons instead of none, and drops the keccak per candidate.
Two smaller notes on the helper while it is being touched:
new Dictionary<int, ValueHash256>(Attempts)pre-sizes to 2¹⁹ entries.ValueHash256wraps aVector256<byte>, so the entry struct aligns to 64 bytes — ~33 MB of entries plus 2 MB of buckets, allocated eagerly on the LOH, for a search that terminates around 2¹⁶ on average.new()peaks near 8 MB.NodeKeyHashCodemirrors a private member. IfNodeKey.GetHashCodeever changes, the search targets the wrong buckets, the pair no longer collides, and the test passes while asserting nothing. The remark is honest about the mirroring; a line saying the test degrades to vacuous rather than failing would make the next reader check it.
| // Decoding installs the process-wide hash seed; this fixture is ParallelScope.All, so the | ||
| // reseed would otherwise land while a WitnessMatchesFixture case is hashing. | ||
| [NonParallelizable] |
There was a problem hiding this comment.
Low — the attribute is right; the reason given is the one that does not apply to this build.
Ethereum.Blockchain.Pyspec.Test builds without EnableZkEvm, so SpanExtensions.SeedHashes here is the std partial with an empty body (SpanExtensions.std.cs:28). InputDecoder.Decode installs nothing in this assembly, and the described reseed-mid-hash race cannot occur.
What is live is the other process-wide static this method touches:
// StatelessExecutor.cs:24, 50, 159
FailureOutput = output;
public static ReadOnlyMemory<byte> FailureOutput { get; private set; }Two StatelessExecutorOutputMatchesFixture cases running concurrently under the inherited ParallelScope.All were racing on it. Not asserted today, so nothing was failing — but it is a genuine shared-mutable static in a parallel fixture, and [NonParallelizable] fixes it.
Everything else in the commit message checks out: parallel: false really is ParallelExecutionOverride (PyspecTestFixture.cs:23), the [Parallelizable(ParallelScope.All)] really is on PyspecBlockchainFixtureBase (line 20) and inherited, and NUnit's shift dispatcher does drain the parallel shift before running a non-parallel work item. Just worth pointing the comment at FailureOutput, or saying the seed part is forward-looking for a guest-configuration build.
5 participants
Changes
Install the full 256-bit
new_payload_request_rootas the guest's hash seed immediately after SSZ Merkleization, before constructing the spec provider, block, witness, or execution state. GuestSpanExtensionshas no static field initializers or class constructor.SpanExtensions.CombineHash(uint, ulong). Its standard partial uses CRC; its guest partial hashes the combined inputs with the run-seeded mixer. Hash, trie, synchronization, and guest RLP callers use this method without build-condition branches. Account hashing reaches it throughValueHash256.GetChainedHashCodeand retains the full nonce.TinyTreePath.GetChainedHashCodecombines the newGetHashCode()result, rather than applying CRC directly to the raw path bytes. Raw-path CRC collisions survive every preceding hash, including an AES-derived one; hashing the path first prevents that fixed collision family from carrying through. The merge base uses CRC directly on the raw tiny path, so this also fixes a pre-existing host weakness. Standard non-AES key hashing still uses the new scalar mixer instead of XxHash3; the AES changes also remain.UInt256Comparer. Share the existing span-based comparer fromPersistentStorageProvider; host BAL containers retain their default comparer selection.SeedHashes(in ValueHash256)overload and the production-dead XxHash3 helpers. XxHash3 remains a benchmark baseline through direct calls in the benchmark project, which now owns its package reference. Core no longer referencesSystem.IO.Hashing; tests exercise the actual scalar fallback.maximum + 1hashes to be distinct, extend them to chained tiny paths, and test every bit of the chaining seed. Add a guest regression case for the preceding-hash value that zeroed the old multiply-based combiner.ulong, checkingIsUint64before readingpanicCode.u0.The seed choice follows the proposed EIP-8025 update and zkevm-standards#41. The payload root is public and identical across provers and retries; it does not provide private entropy per proof attempt. These are in-memory hashes, not cryptographic authentication functions or consensus outputs.
Seeding remains an ordering requirement. Reseeding invalidates populated hash-keyed containers and is not synchronized with hashing. In release guests, unseeded scalar 32-byte and other inputs longer than 16 bytes (except addresses) can hash silently with zero seed; address and short-input paths access uninitialized arrays. Missing seeding is not guaranteed to fail immediately.
The pinned int256 package still lacks runtime seeding. int256 #119 adds
UInt256.SeedHashes(in UInt256); a package upgrade and explicit call remain future integration work. Guest storage-slot maps currently reach the seeded span mixer throughUInt256Comparer.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
The latest follow-up changes tests and benchmark package metadata only: 384 host
BytesTestsand 93TreePathTestspassed, plus all five affected cases with AES disabled. The regression constructs 256 distinct valid tiny paths with identical raw CRCs under each of three chaining seeds and verifies that hashing before chaining breaks that family. The existing CRC-derived byte-input tests now also exerciseCombineHashdirectly. The benchmark dependency is explicit and unconditional; it does not attempt to exclude the transitive package from guest builds. Guest checks below were run for the preceding production-code commit, not repeated for this test-only follow-up.Local Release checks:
BytesTestsandAccountTestsTreePathTestsGuestMixerTests, intrinsics enabledGuestMixerTests,DOTNET_EnableHWIntrinsic=0EnableZkEvm=trueGuest mixer coverage includes every seed bit, seed-specific collisions, independent multiply references, tail/block boundary vectors, dispatch consistency, and the absence of a class constructor. An assembly-scoped fixture installs the seed; seed-mutating tests are nonparallel and restore it.
TinyTreePathHashBenchmarksmeasures the productionHashAndTinyPath.GetHashCodepath, with and without a 32-byte address hash (Hash256).FastHashBenchmarksandFastHash64Benchmarksretain direct scalar and XxHash3 baseline measurements.Native RISC-V execution was not rerun for this follow-up. A completed host payload benchmark at
65e79e1is reported below; the subsequent commit22f4b61changes tests only. Earlier guest step counts and full-suite totals predate the final mixer and chaining changes and are not current-head validation.Documentation
Requires documentation update
Requires explanation in Release Notes
SpanExtensions.InstanceRandomchanges fromuinttoUInt256; its former field signature is not retained. In-memory key hash values change on both AES and non-AES hosts. CRC remains the host chaining operation through the shared combiner.Measurements
Local BenchmarkDotNet 0.15.8 measurements on one Ryzen 9 9950X / Windows 11 host, x64 AES, .NET runtime 10.0.11. The external harness targets net10.0 and was built with SDK 11.0.100-preview.7. Each invocation hashes 1,024 deterministic inputs. Arms ran sequentially; base and pre-combiner runs used three warmup / five measured iterations of 200 ms, and final runs used five warmup / ten measured iterations of 500 ms.
HashAndTinyPath.GetHashCode13a8178aff6The final column measures the production hashing implementation from
1a072ec, unchanged through22f4b61. Both composite-key cases remain about 1 ns/key slower than the merge base. “Address hash present” means aHash256combined with a tiny path; it does not measure the 20-byteAddress.GetHashCode()orAddressAsKey.GetHashCode(). Direct base/head measurements for those members and standaloneValueHash256.GetHashCode()remain outstanding.Final standard deviations are 0.036 and 0.405 ns respectively; the address-present result is noisier. The shared CRC combiner substantially reduces the PR's chaining overhead. It does not restore base timings, because the new standalone key hashing is retained. In particular, the tiny path now hashes through
GetHashCode()before chaining; CRC does not directly hash its raw path bytes.Other final means:
TinyTreePath.GetHashCode1.225 ns,TreePath.GetHashCode2.269 ns, andValueHash256.GetHashCode641.097 ns. The tiny standalone override is not the production chained path and is not used as a proxy for it.These are batched local hash-throughput measurements, not dependent-lookup latency, contention, native guest instruction counts, or block-processing timings. Intermediate experiments that restored standalone CRC hashing or only shortened the span are excluded from the final table.
Host payload results from EXPB run 34082560400, at
65e79e1(same production hashing as22f4b61). The workflow comparison reports client processing times from SSE for the flat layout:These are one run per payload set against cached master measurements, not repeated paired A/B trials. They do not establish a reliable speedup or isolate Address hashing cost. The master comparison here differs from the pinned merge-base comparison in the local microbenchmarks. Values above are the workflow's reported results; this description update does not constitute a separate audit of the run logs.
The guest also currently hashes the tiny path before the shared seeded combiner. That extra pre-hash is not needed for the host CRC collision argument in the guest; its cost remains a follow-up measurement/optimization. All path bits influence the intermediate hash, which is folded to 32 bits before combining. The CRC collision regression is host-only. Its byte-pair collision count uses 1,024 distinct pairs and explicitly allows at most one collision, independent of the number of chaining seeds tested.