avoid unnecessary memory allocations and lookups (#2334)

* use `withValue` instead of `hasKey` + `[]`
* avoid `@` et al
* parse database data inside `onData` instead of making seq then parsing

fluffy/eth_data/era1.nim

@@ -453,7 +453,7 @@ proc buildAccumulator*(f: Era1File): Result[EpochAccumulatorCached, string] =
       HeaderRecord(blockHash: blockHeader.blockHash(), totalDifficulty: totalDifficulty)
     )
 
-  ok(EpochAccumulatorCached.init(@headerRecords))
+  ok(EpochAccumulatorCached.init(headerRecords))
 
 proc verify*(f: Era1File): Result[Digest, string] =
   let

nimbus/db/aristo/aristo_init/rocks_db.nim

@@ -30,7 +30,6 @@ import
   eth/common,
   rocksdb,
   results,
-  ../aristo_constants,
   ../aristo_desc,
   ../aristo_desc/desc_backend,
   ../aristo_blobify,

nimbus/db/aristo/aristo_init/rocks_db/rdb_desc.nim

@@ -32,9 +32,6 @@ type
     basePath*: string                  ## Database directory
     noFq*: bool                        ## No filter queues available
 
-  RdbKey* = array[1 + sizeof VertexID, byte]
-    ## Sub-table key, <pfx> + VertexID
-
   # Alien interface
   RdbGuest* = enum
     ## The guest CF was worth a try, but there are better solutions and this

nimbus/db/aristo/aristo_init/rocks_db/rdb_get.nim

@@ -64,9 +64,10 @@ proc getKey*(
     return ok(move(rc.value))
 
   # Otherwise fetch from backend database
-  var res: Blob
+  var res: Result[HashKey,(AristoError,string)]
   let onData = proc(data: openArray[byte]) =
-    res = @data
+    res = HashKey.fromBytes(data).mapErr(proc(): auto =
+      (RdbHashKeyExpected,""))
 
   let gotData = rdb.keyCol.get(vid.toOpenArray, onData).valueOr:
      const errSym = RdbBeDriverGetKeyError
@@ -75,16 +76,13 @@ proc getKey*(
      return err((errSym,error))
 
   # Correct result if needed
-  let key = block:
-    if gotData:
-      HashKey.fromBytes(res).valueOr:
-        return err((RdbHashKeyExpected,""))
-    else:
-      VOID_HASH_KEY
+  if not gotData:
+    res = ok(VOID_HASH_KEY)
+  elif res.isErr():
+    return res # Parsing failed
 
   # Update cache and return
-  ok rdb.rdKeyLru.lruAppend(vid, key, RdKeyLruMaxSize)
-
+  ok rdb.rdKeyLru.lruAppend(vid, res.value(), RdKeyLruMaxSize)
 
 proc getVtx*(
     rdb: var RdbInst;
@@ -96,9 +94,10 @@ proc getVtx*(
     return ok(move(rc.value))
 
   # Otherwise fetch from backend database
-  var res: Blob
+  var res: Result[VertexRef,(AristoError,string)]
   let onData = proc(data: openArray[byte]) =
-    res = @data
+    res = data.deblobify(VertexRef).mapErr(proc(error: AristoError): auto =
+      (error,""))
 
   let gotData = rdb.vtxCol.get(vid.toOpenArray, onData).valueOr:
     const errSym = RdbBeDriverGetVtxError
@@ -106,15 +105,13 @@ proc getVtx*(
       trace logTxt "getVtx", vid, error=errSym, info=error
     return err((errSym,error))
 
-  var vtx = VertexRef(nil)
-  if gotData:
-    let rc = res.deblobify VertexRef
-    if rc.isErr:
-      return err((rc.error,""))
-    vtx = rc.value
+  if not gotData:
+    res = ok(VertexRef(nil))
+  elif res.isErr():
+    return res # Parsing failed
 
   # Update cache and return
-  ok rdb.rdVtxLru.lruAppend(vid, vtx, RdVtxLruMaxSize)
+  ok rdb.rdVtxLru.lruAppend(vid, res.value(), RdVtxLruMaxSize)
 
 # ------------------------------------------------------------------------------
 # End

nimbus/db/aristo/aristo_layers.nim

@@ -62,39 +62,39 @@ func nLayersKey*(db: AristoDbRef): int =
 # Public functions: getter variants
 # ------------------------------------------------------------------------------
 
-func layersGetVtx*(db: AristoDbRef; vid: VertexID): Result[VertexRef,void] =
+func layersGetVtx*(db: AristoDbRef; vid: VertexID): Opt[VertexRef] =
   ## Find a vertex on the cache layers. An `ok()` result might contain a
   ## `nil` vertex if it is stored on the cache  that way.
   ##
-  if db.top.delta.sTab.hasKey vid:
-    return ok(db.top.delta.sTab.getOrVoid vid)
+  db.top.delta.sTab.withValue(vid, item):
+    return Opt.some(item[])
 
   for w in db.rstack:
-    if w.delta.sTab.hasKey vid:
-      return ok(w.delta.sTab.getOrVoid vid)
+    w.delta.sTab.withValue(vid, item):
+      return Opt.some(item[])
 
-  err()
+  Opt.none(VertexRef)
 
 func layersGetVtxOrVoid*(db: AristoDbRef; vid: VertexID): VertexRef =
   ## Simplified version of `layersGetVtx()`
   db.layersGetVtx(vid).valueOr: VertexRef(nil)
 
 
-func layersGetKey*(db: AristoDbRef; vid: VertexID): Result[HashKey,void] =
+func layersGetKey*(db: AristoDbRef; vid: VertexID): Opt[HashKey] =
   ## Find a hash key on the cache layers. An `ok()` result might contain a void
   ## hash key if it is stored on the cache that way.
   ##
-  if db.top.delta.kMap.hasKey vid:
+  db.top.delta.kMap.withValue(vid, item):
     # This is ok regardless of the `dirty` flag. If this vertex has become
     # dirty, there is an empty `kMap[]` entry on this layer.
-    return ok(db.top.delta.kMap.getOrVoid vid)
+    return Opt.some(item[])
 
   for w in db.rstack:
-    if w.delta.kMap.hasKey vid:
+    w.delta.kMap.withValue(vid, item):
       # Same reasoning as above regarding the `dirty` flag.
-      return ok(w.delta.kMap.getOrVoid vid)
+      return ok(item[])
 
-  err()
+  Opt.none(HashKey)
 
 func layersGetKeyOrVoid*(db: AristoDbRef; vid: VertexID): HashKey =
   ## Simplified version of `layersGetkey()`

nimbus/db/aristo/aristo_merge/merge_proof.nim

@@ -162,9 +162,9 @@ proc mergeProof*(
     ## Check for embedded nodes, i.e. fully encoded node instead of a hash.
     ## They need to be treated as full nodes, here.
     if key.isValid and key.len < 32:
-      let lid = @(key.data).digestTo(HashKey)
+      let lid = key.data.digestTo(HashKey)
       if not seen.hasKey lid:
-        let node = @(key.data).decode(NodeRef)
+        let node = key.data.decode(NodeRef)
         discard todo.append node
         seen[lid] = node
 

nimbus/db/kvt/kvt_layers.nim

@@ -31,29 +31,34 @@ func nLayersKeys*(db: KvtDbRef): int =
 # Public functions: get function
 # ------------------------------------------------------------------------------
 
-func layersHasKey*(db: KvtDbRef; key: openArray[byte]): bool =
+func layersHasKey*(db: KvtDbRef; key: openArray[byte]|seq[byte]): bool =
   ## Return `true` id the argument key is cached.
   ##
-  if db.top.delta.sTab.hasKey @key:
+  when key isnot seq[byte]:
+    let key = @key
+  if db.top.delta.sTab.hasKey key:
     return true
 
   for w in db.rstack:
-    if w.delta.sTab.hasKey @key:
+    if w.delta.sTab.hasKey key:
       return true
 
 
-func layersGet*(db: KvtDbRef; key: openArray[byte]): Result[Blob,void] =
+func layersGet*(db: KvtDbRef; key: openArray[byte]|seq[byte]): Opt[Blob] =
   ## Find an item on the cache layers. An `ok()` result might contain an
   ## empty value if it is stored on the cache  that way.
   ##
-  if db.top.delta.sTab.hasKey @key:
-    return ok(db.top.delta.sTab.getOrVoid @key)
+  when key isnot seq[byte]:
+    let key = @key
+
+  db.top.delta.sTab.withValue(key, item):
+    return Opt.some(item[])
 
   for w in db.rstack:
-    if w.delta.sTab.hasKey @key:
-      return ok(w.delta.sTab.getOrVoid @key)
+    w.delta.sTab.withValue(key, item):
+      return Opt.some(item[])
 
-  err()
+  Opt.none(Blob)
 
 # ------------------------------------------------------------------------------
 # Public functions: put function

nimbus/db/kvt/kvt_utils.nim

@@ -106,7 +106,7 @@ proc hasKey*(
   if key.len == 0:
     return err(KeyInvalid)
 
-  if db.layersHasKey @key:
+  if db.layersHasKey key:
     return ok(true)
 
   let rc = db.getBe key

scripts/block-import-stats.py

@@ -12,16 +12,10 @@ from pandas.plotting import register_matplotlib_converters
 register_matplotlib_converters()
 
 
-def readStats(name: str, min_block_number: int):
+def readStats(name: str):
     df = pd.read_csv(name).convert_dtypes()
     # at least one item - let it lag in the beginning until we reach the min
     # block number or the table will be empty
-    if df.block_number.iloc[-1] > min_block_number + df.block_number.iloc[0]:
-        cutoff = min(
-            df.block_number.iloc[-1] - min_block_number,
-            min_block_number,
-        )
-        df = df[df.block_number >= cutoff]
     df.set_index("block_number", inplace=True)
     df.time /= 1000000000
     df.drop(columns=["gas"], inplace=True)
@@ -72,19 +66,27 @@ parser.add_argument(
     help="Skip block blocks below the given number",
 )
 args = parser.parse_args()
+min_block_number = args.min_block_number
 
-baseline = readStats(args.baseline, 0)
-contender = readStats(args.contender, args.min_block_number)
+baseline = readStats(args.baseline)
+contender = readStats(args.contender)
 
 # Pick out the rows to match - a more sophisticated version of this would
 # interpolate, perhaps - also, maybe should check for non-matching block/tx counts
 df = baseline.merge(contender, on=("block_number", "blocks", "txs"))
+df.reset_index(inplace=True)
+
+if df.block_number.iloc[-1] > min_block_number + df.block_number.iloc[0]:
+    cutoff = min(
+        df.block_number.iloc[-1] - min_block_number,
+        min_block_number,
+    )
+    df = df[df.block_number >= cutoff]
 
 df["bpsd"] = ((df.bps_y - df.bps_x) / df.bps_x)
 df["tpsd"] = ((df.tps_y - df.tps_x) / df.tps_x.replace(0, 1))
 df["timed"] = (df.time_y - df.time_x) / df.time_x
 
-df.reset_index(inplace=True)
 
 if args.plot:
     plt.rcParams["axes.grid"] = True

vendor/nim-stew

@@ -1 +1 @@
-Subproject commit 104132fd0217e846b04dd26a5fbe3e43a4929a05
+Subproject commit 28743363fff5d751b9dd1809b0c7ccf332eb8d79