core, cmd/utils: when configured via flag, in block execution: prefetch all reads from account/storage tries #29807

jwasinger · 2024-05-20T04:23:19Z

Pulled out from #29719

zhaoxiangjunupi · 2024-05-20T08:32:01Z

jiejuelema

core/state/statedb.go

cmd/utils/flags.go

core/state/state_object.go

Mazzika1 · 2024-05-21T02:32:12Z

Okay

Mazzika1 · 2024-05-21T02:32:52Z

Oaky

jwasinger · 2024-05-21T05:44:30Z

~~Haven't quite got this working yet...~~. Seems somewhat okay now.

jwasinger · 2024-05-23T04:53:22Z

Full sync benchmark nodes are running around block 12.2 million (bench7 is running this PR with the witness collection flag enabled, bench8 is master)

Witness collection mode is causing additional prefetch reads which all hit the db cache. There appears to be no performance impact.

Prefetcher duplicate/waste metrics cease to be meaningful if --collectwitnesses is passed. The account/storage loads are higher (but again, these hit the db cache so it's not a problem).

karalabe · 2024-06-03T11:30:55Z

cmd/utils/flags.go

@@ -954,6 +954,12 @@ Please note that --` + MetricsHTTPFlag.Name + ` must be set to start the server.
 Value: metrics.DefaultConfig.InfluxDBOrganization,
 Category: flags.MetricsCategory,
 }
+
+ CollectWitnessesFlag = &cli.BoolFlag{
+ Name: "collectwitnesses",


Please name this debug.collectwitness and move it to the debug namespace in the usage section so we know it's not something to rely on.

As for the description, perhaps: Enable state witness generation during block execution. Work in progress flag, don't use.

Also Pls rename the flag to DebugCollectWitness

Please name this debug.collectwitness and move it to the debug namespace in the usage section so we know it's not something to rely on.

core/blockchain.go

eth/ethconfig/config.go

eth/backend.go

core/state/statedb.go

karalabe · 2024-06-03T11:46:01Z

core/state/statedb.go

@@ -604,6 +610,9 @@ func (s *StateDB) getStateObject(addr common.Address) *stateObject {
 if acc == nil {
 return nil
 }
+ if s.collectWitness && s.prefetcher != nil {
+ s.prefetcher.prefetch(common.Hash{}, s.originalRoot, common.Address{}, [][]byte{addr[:]})
+ }


You need to add the read to the prefetcher if you load from the trie too. Since the prefetcher's trie and the statedb trie are different. Yes, it's a bit of double work, but we only do that while generating the snapshot, so it's fine really.

I don't follow.

core/blockchain.go

cmd/utils/flags.go

jwasinger · 2024-06-04T05:38:35Z

@karalabe I think I've mostly addressed your feedback and simplified the PR.

cmd/utils/flags.go

…g: prefetch all reads from account/storage tries, terminate prefetcher synchronously.

core/state/statedb.go

karalabe

SGTM

jwasinger requested review from karalabe, holiman and rjl493456442 as code owners May 20, 2024 04:23

holiman reviewed May 20, 2024

View reviewed changes

core/state/statedb.go Outdated Show resolved Hide resolved

holiman reviewed May 20, 2024

View reviewed changes

cmd/utils/flags.go Outdated Show resolved Hide resolved

holiman reviewed May 20, 2024

View reviewed changes

core/state/state_object.go Outdated Show resolved Hide resolved

Mazzika1 approved these changes May 21, 2024

View reviewed changes