Bal devnet 6 benchmarks

[Marchhill]

Changes

• Split BlockAccessList/AccountChanges/SlotChanges into role-specific ReadOnly* (wire), Generated* (locally produced), and *AtIndex (per-tx) types so the hot paths only carry the fields they need.
• Parallelise transaction execution against the suggested BAL via a per-account prestate gate; the loader and pre-execution iteration also run as parallel-loop slots so workers no longer wait on a block-wide barrier.
• Defer per-account sorted-array rebuilds to a single Array.Sort on first read, replacing the O(n²) per-prestate-load InsertSorted.
• Cut allocations on the validation and generation hot paths (zero-alloc dict-lookup BAL validation, pooled WorldStates / TxProcessors / BALs, tighter generated-BAL structures).
• Fix a BAL delegation-OOG bug: defer GetCachedCodeInfo past the CALL cold-access OOG check so a reverted EIP-7702 call doesn't leave the delegation target in the BAL.

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[Marchhill]

@claude review

[claude]

Claude finished @Marchhill's task in 4m 46s —— View job

---

PR Review: Bal devnet 6 benchmarks

Tasks:

• Post initial comment
• Load repo rules
• Read key changed files
• Review new BAL type hierarchy
• Review parallel execution path
• Review serialization/decoding changes
• Review benchmark results
• Post final review

---

Summary

Third review pass. All High/Medium/Low findings from the previous two reviews (May 7, May 8) were addressed in 31f74f2. This pass looks fresh at the code on the current branch.

The type hierarchy split, binary-search hot paths, double-checked sort, prestate gate mechanics, and parallel-loop scheduling are all sound. The gas validation two-pass algorithm, EIP-8037 inclusion checks, preExecutionDoneTcs fault propagation, and _inUse/_perTxBal publication-barrier model are correct.

---

Benchmark Results (from auto-run)

Metric	Superblocks (PR vs master)	Realblocks
AVG	-1.14% ✅	-0.17% ✅
MEDIAN	+2.71% ⚠️	flat
P90	+0.95% ⚠️	flat
P99	-20.81% ✅	-3.42% ✅

The large P99 win on superblocks (tail latency cut by ~21%) is the intended optimization target and a real improvement. The MEDIAN/P90 regressions are small (2-3%) and may be within run-to-run noise, but @kamilchodola flagged this as worse than PR #11436's baseline. This deserves investigation before merge — at a minimum, an explicit statement that the median/P90 regression is accepted in trade for the P99 win.

---

Findings

Medium

1. Per-SLOAD heap allocation in ReadOnlySlotChanges.Get() (inline comment)
return [.. tmp.WithoutLeadingZeros()] allocates a new byte[] on every call. In the parallel path every SLOAD goes through this. For a superblock with thousands of storage reads this is continuous allocation pressure on the GC during parallel execution. The caller chain only needs a ReadOnlySpan<byte>, so the backing store could be a caller-supplied stack buffer or a field-backed buffer per-worker.

2. Per-SLOAD heap allocation in TracedAccessWorldState.GetInternal (parallel mode) (inline comment)
MemoryMarshal.CreateReadOnlySpan(...).ToArray() allocates 32 bytes per intra-tx storage read on the parallel path (line 370). The allocation is forced because value is a local EvmWord. A field-backed 32-byte buffer on the worker's TracedAccessWorldState (safe because each worker owns its instance) would eliminate these allocations entirely.

Low

3. SortedDictionary for per-tx account tracking (inline comment)
BlockAccessListAtIndex._accountChanges is SortedDictionary<Address, AccountChangesAtIndex>. Every AddBalanceChange, AddNonceChange, AddStorageChange, etc. call pays O(log n) for the sort. The sorted order is only consumed at merge time by GeneratedAccountChanges which imposes its own sorted structure; BlockAccessListAtIndex itself never needs to iterate in address order during tx execution. A Dictionary + sort-on-demand at merge time would halve the per-change overhead.

---

Correctness verification (no issues)

• EnsureSorted double-checked lock: correct. Plain _sortedDirty = true → SignalPrestateLoaded() (TrySetResult release fence) → worker GetAwaiter().GetResult() (acquire fence) → Volatile.Read(ref _sortedDirty) guarantees visibility.
• _perTxBal publication ordering: plain array write by worker → gasResults[i].SetResult() (release) → validator GetAwaiter().GetResult() (acquire) → plain array read by validator. No Volatile needed; model is sound.
• preExecutionDoneTcs fault paths: TrySetException + TrySetCanceled safety nets on all error paths; validator never hangs.
• _lastLoadedBal TOCTOU guard: written only from slot 0 before parallel tx workers start; no races.
• ValidateBlockAccessList two-pass correctness: generates→suggested and suggested→generates cover missing and surplus cases; prestate-only entries are invisible to per-index queries.
• EIP-8037 inclusion check (2D gas): CheckPerTxInclusion called before gas accounting in both paths; cumulative worst-case is checked correctly.

[Marchhill]

@claude review

[claude]

Claude finished @Marchhill's task in 7m 12s —— View job

---

PR Review: Bal devnet 6 benchmarks (Pass 4)

Tasks:

• Post initial comment
• Load repo rules
• Read key changed files since last review
• Assess May 11 findings resolution status
• Look for new issues
• Post final review

---

Summary

Fourth review pass. The two new commits since the May 11 review (0852865 perf(bal): cut per-SLOAD allocations and SortedDictionary insert cost, 81c6f3a fix lint) address all three Medium findings from that review. No new Critical, High, or Medium issues found.

---

May 11 Findings — Resolution Status

#	Severity	Finding	Status
1	Medium	Per-SLOAD heap allocation in ReadOnlySlotChanges.Get()	✅ Resolved — Get(uint, Span<byte>) now takes a caller-supplied buffer; per-instance _scratchStorage byte[32] field on both BlockAccessListBasedWorldState and TracedAccessWorldState
2	Medium	32-byte heap allocation per intra-tx SLOAD in TracedAccessWorldState.GetInternal	✅ Resolved — CopyTo(_scratchStorage) replaces .ToArray(); buffer verified safe (each TxProcessorWithWorldState owns its own BlockAccessListBasedWorldState — not shared across workers)
3	Low	SortedDictionary for per-tx account tracking	✅ Resolved — BlockAccessListAtIndex._accountChanges is now Dictionary<Address, AccountChangesAtIndex>; sorted merge delegated to GeneratedAccountChanges._accountChanges at merge time

---

Correctness verification (no issues)

• _scratchStorage thread-safety: Confirmed. ParallelTxProcessorWithWorldStateManager constructs each pooled TxProcessorWithWorldState with its own new BlockAccessListBasedWorldState(...) (line 806 of BlockAccessListManager.cs). Workers rent exclusive instances; no concurrent write to _scratchStorage is possible.
• StorageChange byte-order round-trip: new StorageChange(index, UInt256) calls ByteSwap() to convert host-endian UInt256 to big-endian EvmWord wire form. ReadOnlySlotChanges.Get and TracedAccessWorldState.GetInternal reinterpret EvmWord as bytes and return big-endian. Round-trip is correct.
• Prestate publication ordering: Plain writes to _balanceChanges/_nonceChanges/_codeChanges/Changes on the loader → SignalPrestateLoaded() (TrySetResult() release fence) → workers' WaitForPrestate() (GetAwaiter().GetResult() acquire fence). Updated arrays are visible to workers post-gate. Correct.
• _inUse/_perTxBal publication: Each balIndex slot has exactly one writer; gasResults[i].SetResult (release) → validator GetAwaiter().GetResult() (acquire) → plain array read in MergeAndReturnBal. No Volatile needed. Correct.
• AddStorageChange in sequential mode: _generatingBlockAccessList.AddStorageChange(storageCell, new(oldValue, true), new(newValue, true)) constructs UInt256 from big-endian spans, then StorageChange applies ByteSwap(). Read-back reinterprets as big-endian bytes. Correct.

---

Remaining pre-existing Low findings (out of scope per author)

• ReadOnlyAccountChanges is a misleading name for a class with several mutation methods (LoadPreStateBalance, EnablePrestateGate, etc.). The class-level <summary> now explains the constrained-mutation contract, which partially mitigates this.
• // todo: maybe split into smaller classes in IBlockAccessListManager.cs (line 16).

---

Benchmark note

The May 8 benchmark run (before the allocation-optimization commits) showed MEDIAN +2.71% and P90 +0.95% regressions on superblocks alongside the large P99 −20.81% win. Commit 0852865 targets exactly the per-SLOAD allocation and per-insert SortedDictionary costs that would pressure median/P90 under concurrency. A fresh benchmark run is recommended to confirm those metrics recover before merge.

[Marchhill]

superseded by #11573