cmd,core,tests: introduce new BAL execution flags, log BAL slow blocks, surface prefetch reader time as a metric#34892
Merged
Conversation
Adds the StateCounts type that the BAL slow-block work depends on: - core/state/state_counts.go: 10-field plain-int snapshot type with Add merge primitive; isolates the live atomic mutation surface from the value-typed aggregation pipeline. - core/state/statedb.go: SnapshotCounts() method that converts the StateDB's atomic counters to a plain StateCounts at the boundary. - core/blockchain_stats.go: ExecuteStats embeds state.StateCounts; adds ExecWall/PostProcess/Prefetch BAL extension fields, the slowBlockBAL JSON struct + BAL field on slowBlockLog, and extracts buildSlowBlockLog as a pure helper for direct testing. Without this commit the bal-devnet-3 branch as committed in subsequent commits would not build for a fresh clone (state.StateCounts undefined).
Without this, the inline interface assertion in processBlockWithAccessList silently fell through (the prefetchReader returned by ReaderEIP7928 is a *reader wrapper, not the inner *prefetchStateReader), causing the prefetcher contribution to state_read_ms to drop to zero in production. Mirrors the existing GetStateStats forwarding pattern. Adds a regression test that asserts *reader exposes PrefetchReadTimes via the BAL chain, plus a fallback test for non-prefetch readers.
Mirrors the nil-check already used in buildSlowBlockLog. The previous unguarded access was safe today only because parallel_state_processor short-circuits on error before the metrics path is reached, but the API contract was fragile — a future caller could reach reportBALMetrics without an established balTransitionStats and panic.
- Add a comment at the code-mutation gate explaining the deliberate len(code) > 0 (vs code != nil) match against non-BAL semantics; in devnet-3 BAL access lists, an empty []byte is non-nil but encodes "no code install". - Remove BALStateTransitionMetrics.OriginStorageLoadTime: declared but never assigned anywhere in the tree. The actual state-transition read time is captured by AccountReadTime/StorageReadTime added in the prior commit.
The struct is 80 bytes (10 ints) — value semantics matches the type's "snapshot, safe to pass by value" thesis stated in its doc comment, and removes three unnecessary &-takings at call sites. No behavior change.
The first Metrics() call inside calcAndVerifyRoot snapshots accountReadNS and storageReadNS into the cached AccountReadTime/StorageReadTime fields. But commitAccount (called from writeBlockWithState's CommitWithUpdate path) increments storageReadNS *after* that snapshot, so reading m.StorageReadTime later would silently drop those reads. Re-call Metrics() before reading the read-time fields so the cache reflects the post-commit atomics. Other metric fields (AccountUpdate, AccountCommits, etc.) are written directly to s.metrics elsewhere and remain untouched by Metrics(). Found via the metric-correctness audit.
Forward prefetchStateReader.Wait() through *reader.WaitPrefetch and call it before reading the read-time atomics. Eliminates the edge-case where prefetcher goroutines outlast block execution + commit. For slow blocks (the metric's target audience) this is a no-op; for fast blocks it ensures the metric is complete rather than slightly under.
Replace the cached AccountReadTime/StorageReadTime fields (which had a snapshot-staleness bug fixed in 16e98f5 by re-calling Metrics()) with a live ReadTimes() accessor. Metrics() now only returns commit/hash-phase timings — it no longer touches atomics. blockchain.go reads atomics directly via stateTransition.ReadTimes(), eliminating the refresh hack. Also resolves the I1 fragility: Metrics() returning &s.metrics no longer involves any writes inside the function, so concurrent callers can't race on the read-time field updates.
Replace the {Account, Storage, Code} time.Duration scalars threaded through
ProcessResultWithMetrics, txExecResult, processBlockPreTx and resultHandler
with a single ReadDurations struct + Add merge primitive. Same shape as
StateCounts. Adds (*StateDB).SnapshotReads() helper at the boundary.
- Replace reader.go:553 line citation in GetStateStats with the function name; line numbers rot. - Note the BAL sum-of-CPU-time semantics on the read-time field group in ExecuteStats so cross-client consumers don't read total ≤ TotalTime as an invariant.
Per-tx + pre-tx + post-tx StateDBs each have independent stateObjects caches, so summing their AccountLoaded/StorageLoaded counts over-counts addresses/slots touched by multiple phase StateDBs (compared to non-BAL single-StateDB semantics where the cache deduplicates). Override the read counts at the BAL stats wiring site using two new helpers on bal.BlockAccessList. The BAL is the canonical block-level deduplicated access record, so this restores cross-client comparable "unique accounts/slots touched" semantics. CodeLoaded/CodeLoadBytes still sum per-tx — the BAL doesn't track code- fetch events distinctly. Slight over-count remains there, documented.
The previous follow-up note: per-tx + pre-tx + post-tx StateDBs each
have their own stateObjects, so summing CodeLoaded/CodeLoadBytes
over-counts contracts whose code body was fetched by multiple phases.
Fix: snapshot per-StateDB the {address: codeLen} map of contracts whose
s.code is populated, plumb through the existing aggregation pipeline,
dedupe by address in resultHandler/prepareExecResult. The merged map's
size and value-sum become CodeLoaded and CodeLoadBytes respectively,
overriding the per-tx-summed values at the wiring site.
Empirical: a 3-tx block touching the same set of system contracts now
reports code=4, code_bytes=1098 (matches single-tx baseline) instead
of code=12, code_bytes=3294 under the prior over-count.
Field semantics belong on the type so they survive future call sites and show up in godoc; the per-line comments at the constructor in buildSlowBlockLog were redundant with the struct definition.
Moves the snapshot DTOs into statedb.go directly above SnapshotCounts and SnapshotReads, so the types and their sole constructors live in the same file. Avoids a single-purpose state_counts.go.
The BAL block has been verified and committed by the time we reach the read-time accounting block, so the prefetcher (whose workload is bounded by the BAL contents) has no outstanding tasks to wait on.
…ncy when executing with BALs
…ng with a BAL, state loading tasks are completed before tx and state root calculation. can be used with --bal.prefetchworkers
jwasinger
commented
May 6, 2026
| } | ||
| // buildSlowBlockLog builds the slow-block JSON payload. Split out from logSlow | ||
| // so the JSON shape is directly testable. | ||
| func buildSlowBlockLog(s *ExecuteStats, block *types.Block) slowBlockLog { |
Contributor
Author
There was a problem hiding this comment.
We can definitely clean this up and add a dedicated logBadBlockWithBAL method. but I'm going to leave that for the future because the target branch is essentially a staging branch.
CPerezz
added a commit
to CPerezz/go-ethereum
that referenced
this pull request
May 6, 2026
Populates per-block state read/write counts in slow-block JSON for BAL blocks (which ethereum#34892 left as TBD), and adds reader-level read timing. Builds on top of bal-devnet-3 — most of the PR's earlier slow-block log infrastructure was adapted into upstream by that commit, so this change is now scoped to the metric population that the BAL alone can derive. - BAL helpers: BlockAccessList.{UniqueAccountCount, UniqueStorageSlotCount, WrittenCounts}. WrittenCounts walks the BAL once and returns the block-aggregate write counts. - Reader-level read timing: *reader times all synchronous Account/Storage/ Code/CodeSize calls via atomic counters; exposed via ReadTimes() ReadDurations and the new state.ReadTimer interface. Replaces StateDB- level AccountReads/StorageReads/CodeReads tracking (the StateDB shouldn't time its dependencies — the reader is where the I/O happens). - Reader-level code-load dedup: *reader.codeLoaded sync.Map records the first-seen byte length per address; CodeLoads() returns (count, bytes). Exposed via state.CodeLoadTracker. Replaces StateDB CodeLoaded/ CodeLoadBytes tracking and the SnapshotCodeLoads aggregation pattern. - BALStateTransition: caches BlockAccessList.WrittenCounts() once at construction; tracks accountDeleted/storageDeleted atomics for the parallel root-pass (the BAL alone can't distinguish a selfdestruct from a balance/nonce reset). Exposes Deletions() DeletionCounts. Drops the older accountUpdated/storageUpdated/codeUpdated/codeUpdateBytes counters (now derived from WrittenCounts). - BAL block stats path (blockchain.go): populates StateCounts directly — AccountUpdated = WrittenCounts.Accounts - Deletions.Accounts (same for storage). AccountLoaded/StorageLoaded come from BAL. CodeLoaded/ CodeLoadBytes come from the shared *reader (deduplicated across phase StateDBs naturally because they share one reader instance). - Non-BAL block stats path: read durations come from the reader; counts from StateDB fields. StorageUpdated/StorageDeleted unified to int width. - Hard type assertions: state.ReadTimer / state.CodeLoadTracker / state.ReaderStater consumers use direct casts (no silent zero fallback) — every Reader chain in production satisfies these interfaces. - Meter alignment: account/storage Updated meters subtract Deletions to avoid double-reporting blocks under both Update and Delete dashboards.
jwasinger
pushed a commit
to CPerezz/go-ethereum
that referenced
this pull request
May 12, 2026
Populates per-block state read/write counts in slow-block JSON for BAL blocks (which ethereum#34892 left as TBD), and adds reader-level read timing. Builds on top of bal-devnet-3 — most of the PR's earlier slow-block log infrastructure was adapted into upstream by that commit, so this change is now scoped to the metric population that the BAL alone can derive. - BAL helpers: BlockAccessList.{UniqueAccountCount, UniqueStorageSlotCount, WrittenCounts}. WrittenCounts walks the BAL once and returns the block-aggregate write counts. - Reader-level read timing: *reader times all synchronous Account/Storage/ Code/CodeSize calls via atomic counters; exposed via ReadTimes() ReadDurations and the new state.ReadTimer interface. Replaces StateDB- level AccountReads/StorageReads/CodeReads tracking (the StateDB shouldn't time its dependencies — the reader is where the I/O happens). - Reader-level code-load dedup: *reader.codeLoaded sync.Map records the first-seen byte length per address; CodeLoads() returns (count, bytes). Exposed via state.CodeLoadTracker. Replaces StateDB CodeLoaded/ CodeLoadBytes tracking and the SnapshotCodeLoads aggregation pattern. - BALStateTransition: caches BlockAccessList.WrittenCounts() once at construction; tracks accountDeleted/storageDeleted atomics for the parallel root-pass (the BAL alone can't distinguish a selfdestruct from a balance/nonce reset). Exposes Deletions() DeletionCounts. Drops the older accountUpdated/storageUpdated/codeUpdated/codeUpdateBytes counters (now derived from WrittenCounts). - BAL block stats path (blockchain.go): populates StateCounts directly — AccountUpdated = WrittenCounts.Accounts - Deletions.Accounts (same for storage). AccountLoaded/StorageLoaded come from BAL. CodeLoaded/ CodeLoadBytes come from the shared *reader (deduplicated across phase StateDBs naturally because they share one reader instance). - Non-BAL block stats path: read durations come from the reader; counts from StateDB fields. StorageUpdated/StorageDeleted unified to int width. - Hard type assertions: state.ReadTimer / state.CodeLoadTracker / state.ReaderStater consumers use direct casts (no silent zero fallback) — every Reader chain in production satisfies these interfaces. - Meter alignment: account/storage Updated meters subtract Deletions to avoid double-reporting blocks under both Update and Delete dashboards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adapts some of the changes from #34861 . Some other metrics which are recorded manually during execution in that PR, but can be deduced from the BAL are TBD.
I've added two bal feature flags:
--bal.prefetchworkers <uint>: this tunes the number of concurrent go-routines that will be used to perform state fetching tasks by the BAL prefetcher. Default isruntime.NumCPUs, the current behavior inbal-devnet-3.--bal.blockingprefetch: If set, state prefetching will block the execution of transactions and state root update.