From 9c6fd46a516a796d0efb9a27aa61ad449b2fcb34 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 17 Jun 2024 15:29:07 +0200 Subject: [PATCH] avoid computing state root just to know if storage is empty (#2380) The state root computation here is one of the major hotspots in block processing - in the cases the code only needs to know if it's empty or not, it can be done a lot faster. Adding a separate function for this looks fragile and should probably be revisited. --- nimbus/db/core_db/backend/aristo_db.nim | 3 +++ .../backend/aristo_db/handlers_aristo.nim | 14 ++++++++++++++ nimbus/db/core_db/base.nim | 17 +++++++++++++++++ nimbus/db/core_db/base/api_tracking.nim | 1 + nimbus/db/core_db/base/base_desc.nim | 3 +++ nimbus/db/ledger/accounts_ledger.nim | 10 +++++----- 6 files changed, 43 insertions(+), 5 deletions(-) diff --git a/nimbus/db/core_db/backend/aristo_db.nim b/nimbus/db/core_db/backend/aristo_db.nim index c9a483924..ae4a7a82d 100644 --- a/nimbus/db/core_db/backend/aristo_db.nim +++ b/nimbus/db/core_db/backend/aristo_db.nim @@ -150,6 +150,9 @@ proc baseMethods(db: AristoCoreDbRef): CoreDbBaseFns = levelFn: proc(): int = aBase.getLevel, + colStateEmptyFn: proc(col: CoreDbColRef): CoreDbRc[bool] = + aBase.rootHashEmpty(col, "rootHashFn()"), + colStateFn: proc(col: CoreDbColRef): CoreDbRc[Hash256] = aBase.rootHash(col, "rootHashFn()"), diff --git a/nimbus/db/core_db/backend/aristo_db/handlers_aristo.nim b/nimbus/db/core_db/backend/aristo_db/handlers_aristo.nim index ea7c0f861..d942a418f 100644 --- a/nimbus/db/core_db/backend/aristo_db/handlers_aristo.nim +++ b/nimbus/db/core_db/backend/aristo_db/handlers_aristo.nim @@ -645,6 +645,20 @@ proc colPrint*( result &= "$?" +proc rootHashEmpty*( + base: AristoBaseRef; + col: CoreDbColRef; + info: static[string]; + ): CoreDbRc[bool] = + let col = AristoColRef(col) + if not col.isValid: + return err(TrieInvalid.toError(base, info, HashNotAvailable)) + + let root = col.to(VertexID) + if not root.isValid: + return ok(true) + return ok(false) + proc rootHash*( base: AristoBaseRef; col: CoreDbColRef; diff --git a/nimbus/db/core_db/base.nim b/nimbus/db/core_db/base.nim index 7ac50e4e6..237f44383 100644 --- a/nimbus/db/core_db/base.nim +++ b/nimbus/db/core_db/base.nim @@ -445,6 +445,23 @@ proc `$$`*(col: CoreDbColRef): string = result = col.prettyText() #col.ifTrackNewApi: debug newApiTxt, api, elapsed, result +proc stateEmpty*(col: CoreDbColRef): CoreDbRc[bool] = + ## Getter (well, sort of). It retrieves the column state hash for the + ## argument `col` descriptor. The function might fail unless the current + ## state is available (e.g. on `Aristo`.) + ## + ## The value `EMPTY_ROOT_HASH` is returned on the void `col` descriptor + ## argument `CoreDbColRef(nil)`. + ## + col.setTrackNewApi BaseColStateEmptyFn + result = block: + if not col.isNil and col.ready: + col.parent.methods.colStateEmptyFn col + else: + ok true + # Note: tracker will be silent if `vid` is NIL + col.ifTrackNewApi: debug newApiTxt, api, elapsed, col, result + proc state*(col: CoreDbColRef): CoreDbRc[Hash256] = ## Getter (well, sort of). It retrieves the column state hash for the ## argument `col` descriptor. The function might fail unless the current diff --git a/nimbus/db/core_db/base/api_tracking.nim b/nimbus/db/core_db/base/api_tracking.nim index 273c388fa..29c4d180b 100644 --- a/nimbus/db/core_db/base/api_tracking.nim +++ b/nimbus/db/core_db/base/api_tracking.nim @@ -41,6 +41,7 @@ type AnyBackendFn = "any/backend" BaseColPrintFn = "$$" + BaseColStateEmptyFn = "stateEmpty" BaseColStateFn = "state" BaseDbTypeFn = "dbType" BaseFinishFn = "finish" diff --git a/nimbus/db/core_db/base/base_desc.nim b/nimbus/db/core_db/base/base_desc.nim index 58c034912..a6c872435 100644 --- a/nimbus/db/core_db/base/base_desc.nim +++ b/nimbus/db/core_db/base/base_desc.nim @@ -92,6 +92,8 @@ type CoreDbBaseDestroyFn* = proc(eradicate = true) {.noRaise.} CoreDbBaseColStateFn* = proc( col: CoreDbColRef): CoreDbRc[Hash256] {.noRaise.} + CoreDbBaseColStateEmptyFn* = proc( + col: CoreDbColRef): CoreDbRc[bool] {.noRaise.} CoreDbBaseColPrintFn* = proc(vid: CoreDbColRef): string {.noRaise.} CoreDbBaseErrorPrintFn* = proc(e: CoreDbErrorRef): string {.noRaise.} CoreDbBaseLevelFn* = proc(): int {.noRaise.} @@ -110,6 +112,7 @@ type CoreDbBaseFns* = object destroyFn*: CoreDbBaseDestroyFn colStateFn*: CoreDbBaseColStateFn + colStateEmptyFn*: CoreDbBaseColStateEmptyFn colPrintFn*: CoreDbBaseColPrintFn errorPrintFn*: CoreDbBaseErrorPrintFn levelFn*: CoreDbBaseLevelFn diff --git a/nimbus/db/ledger/accounts_ledger.nim b/nimbus/db/ledger/accounts_ledger.nim index 2f577fe80..52e0d499b 100644 --- a/nimbus/db/ledger/accounts_ledger.nim +++ b/nimbus/db/ledger/accounts_ledger.nim @@ -376,9 +376,9 @@ proc persistStorage(acc: AccountRef, ac: AccountsLedgerRef) = acc.statement.storage = storageLedger.getColumn() # No need to hold descriptors for longer than needed - let state = acc.statement.storage.state.valueOr: + let stateEmpty = acc.statement.storage.stateEmpty.valueOr: raiseAssert "Storage column state error: " & $$error - if state == EMPTY_ROOT_HASH: + if stateEmpty: acc.statement.storage = CoreDbColRef(nil) @@ -540,8 +540,8 @@ proc clearStorage*(ac: AccountsLedgerRef, address: EthAddress) = let acc = ac.getAccount(address) acc.flags.incl {Alive, NewlyCreated} - let accHash = acc.statement.storage.state.valueOr: return - if accHash != EMPTY_ROOT_HASH: + let empty = acc.statement.storage.stateEmpty.valueOr: return + if not empty: # need to clear the storage from the database first let acc = ac.makeDirty(address, cloneStorage = false) ac.ledger.freeStorage address @@ -613,7 +613,7 @@ proc clearEmptyAccounts(ac: AccountsLedgerRef) = ac.ripemdSpecial = false proc persist*(ac: AccountsLedgerRef, - clearEmptyAccount: bool = false) = + clearEmptyAccount: bool = false) {.deprecated.} = # make sure all savepoint already committed doAssert(ac.savePoint.parentSavepoint.isNil)