From 51fdf0e0530e73cbc351f59cef096159ea6a60e1 Mon Sep 17 00:00:00 2001 From: CPerezz Date: Mon, 4 May 2026 18:42:03 +0200 Subject: [PATCH] core: drop and tighten comments per PR feedback --- core/blockchain.go | 21 +++-------- core/blockchain_stats.go | 56 +++++++++--------------------- core/parallel_state_processor.go | 39 +++++++-------------- core/state/bal_state_transition.go | 30 +++++----------- core/state/reader.go | 7 ++-- core/state/reader_eip_7928.go | 13 +++---- core/state/reader_eip_7928_test.go | 14 +++----- core/state/statedb.go | 28 +++++---------- 8 files changed, 62 insertions(+), 146 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 9a98ba2896..efe6985001 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -652,10 +652,8 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block * writeTime := time.Since(writeStart) var stats ExecuteStats - // Counts: write counts come from the BAL state transition; read counts - // for accounts/storage come from the BAL access list (deduplicated); - // code-load counts come from a deduplicated address set tracked across - // all phase StateDBs by the parallel processor. + // AccountLoaded/StorageLoaded come from the BAL access list (deduplicated); + // per-StateDB sums would over-count addresses touched by multiple phases. stats.StateCounts = res.Counts stats.StateCounts.Add(stateTransition.WriteCounts()) if al := block.AccessList(); al != nil { @@ -665,17 +663,10 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block * stats.StateCounts.CodeLoaded = res.CodeLoaded stats.StateCounts.CodeLoadBytes = res.CodeLoadBytes - // Time durations under parallel execution use wall-clock semantics. - // Per-tx duration sums (CPU-time) are intentionally not plumbed: they - // would conflict with mgas/sec accounting against TotalTime. - stats.Execution = res.ExecTime // wall-clock parallel execution + stats.Execution = res.ExecTime stats.ExecWall = res.ExecTime stats.PostProcess = res.PostProcessTime - // Map BALStateTransitionMetrics (already wall-clock-correct) onto schema - // fields used by logSlow's StateHashMs computation. The sum - // AccountUpdate+StateUpdate+StateHash is the parallel state-root compute - // time, matching reportBALMetrics's stateRootComputeTimer. if m := res.StateTransitionMetrics; m != nil { stats.AccountHashes = m.AccountUpdate + m.StateUpdate + m.StateHash stats.AccountCommits = m.AccountCommits @@ -683,10 +674,7 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block * stats.DatabaseCommit = m.TrieDBCommits stats.Prefetch = m.StatePrefetch } - // Sum read times across per-tx execution, BAL state-transition, and - // prefetcher async fetches. Sum-of-CPU-time, not wall-clock. No - // WaitPrefetch needed: state is already committed, so the prefetcher - // (bounded by BAL contents) has drained. + // Sum-of-CPU-time across per-tx, BAL state-transition, and prefetcher paths. var prefetchAccountReads, prefetchStorageReads time.Duration if pr, ok := prefetchReader.(interface { PrefetchReadTimes() (time.Duration, time.Duration) @@ -698,7 +686,6 @@ func (bc *BlockChain) processBlockWithAccessList(parentRoot common.Hash, block * stats.StorageReads = res.Reads.Storage + prefetchStorageReads + balStorageReads stats.CodeReads = res.Reads.Code - // Cache stats from the shared prefetch reader (accumulates centrally). if r, ok := prefetchReader.(state.ReaderStater); ok { stats.StateReadCacheStats = r.GetStats() } diff --git a/core/blockchain_stats.go b/core/blockchain_stats.go index 8cda061b94..53834fca9a 100644 --- a/core/blockchain_stats.go +++ b/core/blockchain_stats.go @@ -28,9 +28,7 @@ import ( // ExecuteStats includes all the statistics of a block execution in details. type ExecuteStats struct { - // State read times. For BAL blocks these are sum-of-CPU-time across - // per-tx, pre-tx, post-tx, BAL state-transition and prefetcher paths; - // can exceed TotalTime by design. Sequential blocks: wall-clock. + // State read times AccountReads time.Duration // Time spent on the account reads StorageReads time.Duration // Time spent on the storage reads AccountHashes time.Duration // Time spent on the account trie hash @@ -40,8 +38,7 @@ type ExecuteStats struct { StorageCommits time.Duration // Time spent on the storage trie commit CodeReads time.Duration // Time spent on the contract code read - // Embedded state-mutation counts. Field promotion preserves access as - // s.AccountLoaded etc. Note StorageUpdated/StorageDeleted are int64 here + // State-mutation counts. StorageUpdated/StorageDeleted are int64 // (snapshot from atomic.Int64 on StateDB). state.StateCounts @@ -55,9 +52,7 @@ type ExecuteStats struct { TotalTime time.Duration // The total time spent on block execution MgasPerSecond float64 // The million gas processed per second - // BAL extension durations — set by processBlockWithAccessList for blocks - // processed via the parallel BAL path. Surfaced in the slow-block log's - // optional `bal` block. + // BAL parallel-path durations, surfaced under slowBlockLog.BAL. ExecWall time.Duration // Wall-clock parallel transaction execution PostProcess time.Duration // Post-tx finalization (system contracts, requests) Prefetch time.Duration // BAL state prefetching @@ -123,9 +118,7 @@ type slowBlockLog struct { StateReads slowBlockReads `json:"state_reads"` StateWrites slowBlockWrites `json:"state_writes"` Cache slowBlockCache `json:"cache"` - // BAL is the parallel-execution extension. Present iff the block was - // processed via the BAL parallel path. Cross-client consumers can use its - // presence to distinguish parallel-executed blocks from sequential ones. + // BAL is set only for blocks processed via the parallel BAL path. BAL *slowBlockBAL `json:"bal,omitempty"` } @@ -187,31 +180,18 @@ type slowBlockCodeCacheEntry struct { MissBytes int64 `json:"miss_bytes"` } -// slowBlockBAL is the parallel-execution extension surfaced under the -// optional "bal" field of slowBlockLog. It carries timings that are -// well-defined under parallel execution but don't fit the sequential schema. +// slowBlockBAL holds parallel-execution timings that don't fit the sequential schema. type slowBlockBAL struct { - // ExecWallMs is wall-clock parallel transaction execution. - ExecWallMs float64 `json:"exec_wall_ms"` - // PostProcessMs is post-tx system contracts (withdrawals, consolidations, finalize). - PostProcessMs float64 `json:"post_process_ms"` - // PrefetchMs is the BAL state prefetcher (alias of state_prefetch_ms). - PrefetchMs float64 `json:"prefetch_ms"` - // StatePrefetchMs is async state-load time during state-root computation. - StatePrefetchMs float64 `json:"state_prefetch_ms"` - // AccountUpdateMs is the account trie update phase. - AccountUpdateMs float64 `json:"account_update_ms"` - // StateUpdateMs is the state trie update phase. - StateUpdateMs float64 `json:"state_update_ms"` - // StateHashMs is state-root hash computation. - StateHashMs float64 `json:"state_hash_ms"` - // AccountCommitMs is the account trie commit to disk. - AccountCommitMs float64 `json:"account_commit_ms"` - // StorageCommitMs is the storage trie commit to disk. - StorageCommitMs float64 `json:"storage_commit_ms"` - // TrieDBCommitMs is the trie database commit. - TrieDBCommitMs float64 `json:"triedb_commit_ms"` - // SnapshotCommitMs is the state snapshot commit. + ExecWallMs float64 `json:"exec_wall_ms"` + PostProcessMs float64 `json:"post_process_ms"` + PrefetchMs float64 `json:"prefetch_ms"` + StatePrefetchMs float64 `json:"state_prefetch_ms"` + AccountUpdateMs float64 `json:"account_update_ms"` + StateUpdateMs float64 `json:"state_update_ms"` + StateHashMs float64 `json:"state_hash_ms"` + AccountCommitMs float64 `json:"account_commit_ms"` + StorageCommitMs float64 `json:"storage_commit_ms"` + TrieDBCommitMs float64 `json:"triedb_commit_ms"` SnapshotCommitMs float64 `json:"snapshot_commit_ms"` } @@ -221,9 +201,8 @@ func durationToMs(d time.Duration) float64 { return float64(d.Nanoseconds()) / 1e6 } -// buildSlowBlockLog constructs the slow-block log JSON struct from execution -// statistics. Pure function — no side effects, no logging — to make the JSON -// shape directly testable. +// 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 { logEntry := slowBlockLog{ Level: "warn", @@ -278,7 +257,6 @@ func buildSlowBlockLog(s *ExecuteStats, block *types.Block) slowBlockLog { }, }, } - // Populate the parallel-execution extension only for BAL-processed blocks. if m := s.balTransitionStats; m != nil { logEntry.BAL = &slowBlockBAL{ ExecWallMs: durationToMs(s.ExecWall), diff --git a/core/parallel_state_processor.go b/core/parallel_state_processor.go index e65cee16f9..6d6ba2ba32 100644 --- a/core/parallel_state_processor.go +++ b/core/parallel_state_processor.go @@ -24,17 +24,14 @@ type ProcessResultWithMetrics struct { // the time it took to execute all txs in the block ExecTime time.Duration PostProcessTime time.Duration - // Counts is the per-StateDB sum of state-mutation counters across pre-tx, - // per-tx and post-tx phases. The caller may override AccountLoaded and - // StorageLoaded with deduplicated counts derived from block.AccessList() - // (per-StateDB sums over-count addresses touched by multiple phases). + // Counts sums state-mutation counters across pre-tx, per-tx and post-tx + // StateDBs. AccountLoaded/StorageLoaded are NOT deduplicated here — the + // caller overrides them from block.AccessList(). Counts state.StateCounts - // Reads is the sum of per-StateDB read times across pre-tx, per-tx and - // post-tx phases. Sum-of-CPU-time, not wall-clock. + // Reads sums per-StateDB read times (sum-of-CPU-time, not wall-clock). Reads state.ReadDurations - // CodeLoaded is the deduplicated count of unique contract addresses whose - // code body was fetched during the block (across all phase StateDBs). - // CodeLoadBytes is the sum of those code lengths. + // CodeLoaded/CodeLoadBytes are deduplicated by contract address across + // all phase StateDBs. CodeLoaded int CodeLoadBytes int } @@ -184,9 +181,7 @@ func (p *ParallelStateProcessor) prepareExecResult(block *types.Block, tExecStar tPostprocess := time.Since(tPostprocessStart) - // Fold post-tx statedb counts and reads into the aggregate. postTxState is - // local and would otherwise be discarded; this captures system-contract - // activity (withdrawal queue, consolidation queue) and engine.Finalize. + // Fold post-tx counts/reads in: postTxState is local and otherwise discarded. aggCounts.Add(postTxState.SnapshotCounts()) aggReads.Add(postTxState.SnapshotReads()) for addr, l := range postTxState.SnapshotCodeLoads() { @@ -230,14 +225,10 @@ type txExecResult struct { stateReads bal.StateAccesses - // Per-tx state-mutation counts and read durations, snapshotted from the - // worker statedb just before send. Aggregated single-threaded in - // resultHandler. - counts state.StateCounts - reads state.ReadDurations - // codeLoads is addr→codeLen for contracts whose code body was fetched - // in this tx. Deduped across all phases in resultHandler. - codeLoads map[common.Address]int + // Per-tx counts/reads/code-loads, aggregated single-threaded in resultHandler. + counts state.StateCounts + reads state.ReadDurations + codeLoads map[common.Address]int // addr → code len, deduped across phases } // resultHandler polls until all transactions have finished executing and the @@ -251,13 +242,9 @@ func (p *ParallelStateProcessor) resultHandler(block *types.Block, preTxAccesses var numTxComplete int // Seed aggregates with the pre-tx contribution (BeaconRoot, ParentBlockHash). - // Per-tx fold below; post-tx fold in prepareExecResult. accesses := preTxAccesses aggCounts := preCounts aggReads := preReads - // Dedup'd map of contract addresses whose code body was fetched by any - // phase StateDB. Address-keyed so multiple phases adding the same contract - // only count it once. aggCodeLoads := make(map[common.Address]int) for addr, l := range preCodeLoads { aggCodeLoads[addr] = l @@ -406,9 +393,7 @@ func (p *ParallelStateProcessor) processBlockPreTx(block *types.Block, statedb * if !accessList.MutationsAt(0).Eq(mutations) { return nil, state.StateCounts{}, state.ReadDurations{}, nil, fmt.Errorf("invalid block access list: mismatch between local/remote access list mutations at idx 0") } - // Snapshot the pre-tx statedb's counts/reads/code-loads so system-contract - // activity (BeaconRoot, ParentBlockHash) contributes to the aggregate; - // sdb is local and would otherwise be discarded. + // Snapshot pre-tx counts/reads/code-loads: sdb is local and otherwise discarded. return reads, sdb.SnapshotCounts(), sdb.SnapshotReads(), sdb.SnapshotCodeLoads(), nil } diff --git a/core/state/bal_state_transition.go b/core/state/bal_state_transition.go index ab7bcf37f8..3dbaedc344 100644 --- a/core/state/bal_state_transition.go +++ b/core/state/bal_state_transition.go @@ -41,20 +41,16 @@ type BALStateTransition struct { tries sync.Map //map[common.Address]Trie deletions map[common.Address]struct{} - // Storage counters use atomic.Int64 because they're written from per-address - // goroutines. The others are written single-threaded inside IntermediateRoot's - // serial mutation loop, so plain int matches StateCounts' int fields. + // Storage/read counters are atomic — written from per-address goroutines. + // account/code counters are plain int — written single-threaded. accountDeleted int accountUpdated int storageDeleted atomic.Int64 storageUpdated atomic.Int64 codeUpdated int codeUpdateBytes int - - // Read-time accumulators for state-root recomputation reads. Atomic - // because s.reader.Account/Storage is called from per-address goroutines. - accountReadNS atomic.Int64 - storageReadNS atomic.Int64 + accountReadNS atomic.Int64 + storageReadNS atomic.Int64 stateUpdate *stateUpdate @@ -64,22 +60,16 @@ type BALStateTransition struct { err error } -// Metrics returns the cached commit/hash-phase timings. Read-time atomics -// are exposed separately via ReadTimes; that decoupling avoids the -// snapshot-staleness pitfall when commitAccount runs more reads after -// Metrics is first called. func (s *BALStateTransition) Metrics() *BALStateTransitionMetrics { return &s.metrics } -// ReadTimes returns the current accumulated read times from atomic counters. -// Always live; safe to call at any point after IntermediateRoot/Commit work. +// ReadTimes returns the accumulated state-read times. func (s *BALStateTransition) ReadTimes() (account, storage time.Duration) { return time.Duration(s.accountReadNS.Load()), time.Duration(s.storageReadNS.Load()) } -// WriteCounts returns the state-mutation counts tracked during the parallel -// state-root computation. +// WriteCounts returns the state-mutation counts from the parallel state-root pass. func (s *BALStateTransition) WriteCounts() StateCounts { return StateCounts{ AccountUpdated: s.accountUpdated, @@ -523,11 +513,9 @@ func (s *BALStateTransition) IntermediateRoot(_ bool) common.Hash { } else { acct, code := s.updateAccount(mutatedAddr) - // Use len(code) > 0 (not code != nil) to match the non-BAL semantic - // at statedb.go (obj.dirtyCode && len(obj.code) > 0). In devnet-3 - // BAL access lists, an empty []byte is non-nil but encodes "no code - // install"; treating it as a code mutation would over-count and - // call UpdateContractCode with an empty payload. + // Empty []byte is non-nil but means "no code install" in devnet-3 + // BAL access lists; matches the obj.dirtyCode && len(obj.code) > 0 + // gate in statedb.go. if len(code) > 0 { codeHash := crypto.Keccak256Hash(code) acct.CodeHash = codeHash.Bytes() diff --git a/core/state/reader.go b/core/state/reader.go index 2128773ddf..9cfaab9b7c 100644 --- a/core/state/reader.go +++ b/core/state/reader.go @@ -565,9 +565,7 @@ func (r *reader) GetStats() ReaderStats { } } -// PrefetchReadTimes returns the prefetcher's accumulated read times if the -// underlying state reader exposes them (e.g. *prefetchStateReader). Returns -// zero if the wrapped reader doesn't track these (sequential paths, tests). +// PrefetchReadTimes forwards to the wrapped prefetcher, or returns zero. func (r *reader) PrefetchReadTimes() (account, storage time.Duration) { if pr, ok := r.StateReader.(interface { PrefetchReadTimes() (time.Duration, time.Duration) @@ -577,8 +575,7 @@ func (r *reader) PrefetchReadTimes() (account, storage time.Duration) { return 0, 0 } -// WaitPrefetch blocks until the wrapped prefetcher (if any) finishes its -// task list. No-op for non-prefetch readers. +// WaitPrefetch blocks until the wrapped prefetcher drains; no-op otherwise. func (r *reader) WaitPrefetch() { if pr, ok := r.StateReader.(interface{ Wait() error }); ok { _ = pr.Wait() diff --git a/core/state/reader_eip_7928.go b/core/state/reader_eip_7928.go index 6f451960fe..53949619a9 100644 --- a/core/state/reader_eip_7928.go +++ b/core/state/reader_eip_7928.go @@ -89,8 +89,7 @@ type prefetchStateReader struct { term chan struct{} closeOnce sync.Once - // Async-fetch read-time accumulators (atomic because process() runs - // across N goroutines). + // Atomic — process() runs across N goroutines. accountReadNS atomic.Int64 storageReadNS atomic.Int64 } @@ -374,9 +373,8 @@ func (r *readerTracker) TouchStorage(addr common.Address, slot common.Hash) { list[slot] = struct{}{} } -// GetStateStats forwards stats from the wrapped *stateReaderWithStats so the -// (*reader).GetStateStats type assertion succeeds. Without this, account/ -// storage cache hit/miss counts emit zero on BAL blocks. +// GetStateStats forwards stats from the wrapped reader; without this, BAL +// blocks would emit zero cache hit/miss counts. func (r *prefetchStateReader) GetStateStats() StateReaderStats { if stater, ok := r.StateReader.(StateReaderStater); ok { return stater.GetStateStats() @@ -384,9 +382,8 @@ func (r *prefetchStateReader) GetStateStats() StateReaderStats { return StateReaderStats{} } -// PrefetchReadTimes returns the accumulated wall-time-of-each-call durations -// for asynchronous account/storage prefetches. Sum-of-CPU-time across worker -// goroutines; not wall-clock total prefetch time. +// PrefetchReadTimes returns sum-of-CPU-time across worker goroutines (not +// wall-clock). func (r *prefetchStateReader) PrefetchReadTimes() (account, storage time.Duration) { return time.Duration(r.accountReadNS.Load()), time.Duration(r.storageReadNS.Load()) } diff --git a/core/state/reader_eip_7928_test.go b/core/state/reader_eip_7928_test.go index 57305031be..a6f4eb8ecd 100644 --- a/core/state/reader_eip_7928_test.go +++ b/core/state/reader_eip_7928_test.go @@ -291,23 +291,17 @@ func TestPrefetchStateReaderForwardsStats(t *testing.T) { } } -// TestReaderForwardsPrefetchReadTimes locks down that the *reader aggregator -// (the type returned by ReaderEIP7928) exposes PrefetchReadTimes via the -// inner *prefetchStateReader. Without the forwarding method on *reader, -// callers that hold a Reader interface would not see the prefetcher's -// accumulated read times even though the prefetcher tracks them. +// TestReaderForwardsPrefetchReadTimes locks down that *reader exposes the +// inner prefetcher's read-time counters via PrefetchReadTimes. func TestReaderForwardsPrefetchReadTimes(t *testing.T) { stub := newRefStateReader() cached := newStateReaderWithCache(stub) withStats := newStateReaderWithStats(cached) prefetch := newPrefetchStateReaderInternal(withStats, nil, 1) - // Seed timer values directly on the prefetcher. prefetch.accountReadNS.Store(123) prefetch.storageReadNS.Store(456) - // Wrap in *reader the way ReaderEIP7928 does (with a nil code reader for - // brevity; PrefetchReadTimes only inspects the state side). r := newReader(nil, prefetch) a, s := r.PrefetchReadTimes() @@ -319,8 +313,8 @@ func TestReaderForwardsPrefetchReadTimes(t *testing.T) { } } -// TestReaderPrefetchReadTimesNonPrefetch verifies the safe zero fallback when -// the wrapped state reader doesn't expose PrefetchReadTimes (sequential path). +// TestReaderPrefetchReadTimesNonPrefetch verifies the zero fallback when the +// wrapped reader doesn't expose PrefetchReadTimes. func TestReaderPrefetchReadTimesNonPrefetch(t *testing.T) { r := newReader(nil, newRefStateReader()) a, s := r.PrefetchReadTimes() diff --git a/core/state/statedb.go b/core/state/statedb.go index 04f90d8cf0..7f97822ca0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -223,9 +223,8 @@ func (s *StateDB) WithReader(reader Reader) *StateDB { return cpy } -// ReadDurations groups the {Account, Storage, Code} state-read times that are -// aggregated across pre-tx, per-tx and post-tx statedbs in the BAL parallel -// path. Sum-of-CPU-time, not wall-clock. +// ReadDurations groups the {Account, Storage, Code} state-read times. +// Sum-of-CPU-time when aggregated across BAL phase statedbs. type ReadDurations struct { Account time.Duration Storage time.Duration @@ -239,10 +238,8 @@ func (r *ReadDurations) Add(other ReadDurations) { r.Code += other.Code } -// StateCounts holds count-only statistics gathered during a block's state -// transition. Plain-int snapshot type, safe to copy through channels. -// Atomic counters on StateDB are converted at the snapshot boundary in -// SnapshotCounts. Read durations live in ReadDurations (separate type). +// StateCounts is a plain-int snapshot of state-mutation counters. Atomic +// fields on StateDB are Load()'d at the SnapshotCounts boundary. type StateCounts struct { AccountLoaded int // accounts retrieved from the database during the state transition AccountUpdated int // accounts updated during the state transition @@ -256,10 +253,7 @@ type StateCounts struct { CodeUpdateBytes int // total bytes of persisted code written } -// Add merges other into c. Plain integer addition — no atomics here, since -// StateCounts is the snapshot type. The receiver is the only mutated party; -// other is taken by value (the struct is small and value semantics matches -// the snapshot thesis stated above). +// Add merges other into c. func (c *StateCounts) Add(other StateCounts) { c.AccountLoaded += other.AccountLoaded c.AccountUpdated += other.AccountUpdated @@ -273,9 +267,7 @@ func (c *StateCounts) Add(other StateCounts) { c.CodeUpdateBytes += other.CodeUpdateBytes } -// SnapshotCounts returns a value-copy of the state-mutation counters as a -// plain-int StateCounts. Atomic fields are read via Load(); the result is -// safe to copy, pass through channels, and aggregate via StateCounts.Add. +// SnapshotCounts returns a plain-int copy of the state-mutation counters. func (s *StateDB) SnapshotCounts() StateCounts { return StateCounts{ AccountLoaded: s.AccountLoaded, @@ -291,8 +283,7 @@ func (s *StateDB) SnapshotCounts() StateCounts { } } -// SnapshotReads returns a value-copy of the {Account, Storage, Code} read -// durations accumulated on this StateDB. +// SnapshotReads returns the {Account, Storage, Code} read durations. func (s *StateDB) SnapshotReads() ReadDurations { return ReadDurations{ Account: s.AccountReads, @@ -301,9 +292,8 @@ func (s *StateDB) SnapshotReads() ReadDurations { } } -// SnapshotCodeLoads returns the addresses whose contract code body was -// fetched during this StateDB's lifetime, mapped to byte length. Used by the -// BAL parallel pipeline to deduplicate code-load events across phase StateDBs. +// SnapshotCodeLoads returns addresses whose code body was fetched, mapped to +// byte length. Used to deduplicate code-load events across BAL phase StateDBs. func (s *StateDB) SnapshotCodeLoads() map[common.Address]int { if len(s.stateObjects) == 0 { return nil