fix: optimize commit() for Container and ArrayComposite ViewDUs

ChainSafe · Jul 11, 2024 · d6c87cf · d6c87cf
1 parent deecd52
commit d6c87cf
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 31 deletions.
diff --git a/packages/ssz/src/viewDU/arrayComposite.ts b/packages/ssz/src/viewDU/arrayComposite.ts
@@ -184,7 +184,6 @@ export class ArrayCompositeTreeViewDU<
       return;
     }
 
-    const nodesChanged: {index: number; node: Node}[] = [];
     // each view may mutate HashComputationGroup at offset + depth
     const hashCompsView =
       hashComps != null && isOldRootHashed
@@ -195,25 +194,31 @@ export class ArrayCompositeTreeViewDU<
           }
         : null;
 
-    for (const [index, view] of this.viewsChanged) {
+    const indexesChanged = Array.from(this.viewsChanged.keys()).sort((a, b) => a - b);
+    const indexes: number[] = [];
+    const nodes: Node[] = [];
+    for (const index of indexesChanged) {
+      const view = this.viewsChanged.get(index);
+      if (!view) {
+        // should not happen
+        throw Error("View not found in viewsChanged, index=" + index);
+      }
+
       const node = this.type.elementType.commitViewDU(view, hashCompsView);
       // there's a chance the view is not changed, no need to rebind nodes in that case
       if (this.nodes[index] !== node) {
         // Set new node in nodes array to ensure data represented in the tree and fast nodes access is equal
         this.nodes[index] = node;
-        nodesChanged.push({index, node});
+        // nodesChanged.push({index, node});
+        indexes.push(index);
+        nodes.push(node);
       }
 
       // Cache the view's caches to preserve it's data after 'this.viewsChanged.clear()'
       const cache = this.type.elementType.cacheOfViewDU(view);
       if (cache) this.caches[index] = cache;
     }
 
-    // TODO: Optimize to loop only once, Numerical sort ascending
-    const nodesChangedSorted = nodesChanged.sort((a, b) => a.index - b.index);
-    const indexes = nodesChangedSorted.map((entry) => entry.index);
-    const nodes = nodesChangedSorted.map((entry) => entry.node);
-
     const chunksNode = this.type.tree_getChunksNode(this._rootNode);
     const hashCompsThis =
       hashComps != null && isOldRootHashed

diff --git a/packages/ssz/src/viewDU/container.ts b/packages/ssz/src/viewDU/container.ts
@@ -89,38 +89,40 @@ class ContainerTreeViewDU<Fields extends Record<string, Type<unknown>>> extends
       return;
     }
 
-    const nodesChanged: {index: number; node: Node}[] = [];
-
     let hashCompsView: HashComputationGroup | null = null;
     // if old root is not hashed, no need to pass HashComputationGroup to child view bc we need to do full traversal here
     if (hashComps != null && isOldRootHashed) {
       // each view may mutate HashComputationGroup at offset + depth
       hashCompsView = {byLevel: hashComps.byLevel, offset: hashComps.offset + this.type.depth};
     }
-    for (const [index, view] of this.viewsChanged) {
-      const fieldType = this.type.fieldsEntries[index].fieldType as unknown as CompositeTypeAny;
-      const node = fieldType.commitViewDU(view, hashCompsView);
-      // there's a chance the view is not changed, no need to rebind nodes in that case
-      if (this.nodes[index] !== node) {
-        // Set new node in nodes array to ensure data represented in the tree and fast nodes access is equal
-        this.nodes[index] = node;
-        nodesChanged.push({index, node});
-      }
-
-      // Cache the view's caches to preserve it's data after 'this.viewsChanged.clear()'
-      const cache = fieldType.cacheOfViewDU(view);
-      if (cache) this.caches[index] = cache;
-    }
 
-    for (const index of this.nodesChanged) {
-      nodesChanged.push({index, node: this.nodes[index]});
+    // union all changes then sort, they should not be duplicated
+    const combinedIndexes = [...this.nodesChanged, ...Array.from(this.viewsChanged.keys())].sort((a, b) => a - b);
+    const indexes: number[] = [];
+    const nodes: Node[] = [];
+    for (const index of combinedIndexes) {
+      const view = this.viewsChanged.get(index);
+      if (view) {
+        // composite type
+        const fieldType = this.type.fieldsEntries[index].fieldType as unknown as CompositeTypeAny;
+        const node = fieldType.commitViewDU(view, hashCompsView);
+        // there's a chance the view is not changed, no need to rebind nodes in that case
+        if (this.nodes[index] !== node) {
+          // Set new node in nodes array to ensure data represented in the tree and fast nodes access is equal
+          this.nodes[index] = node;
+          indexes.push(index);
+          nodes.push(node);
+        }
+        // Cache the view's caches to preserve it's data after 'this.viewsChanged.clear()'
+        const cache = fieldType.cacheOfViewDU(view);
+        if (cache) this.caches[index] = cache;
+      } else {
+        // basic type
+        indexes.push(index);
+        nodes.push(this.nodes[index]);
+      }
     }
 
-    // TODO: Optimize to loop only once, Numerical sort ascending
-    const nodesChangedSorted = nodesChanged.sort((a, b) => a.index - b.index);
-    const indexes = nodesChangedSorted.map((entry) => entry.index);
-    const nodes = nodesChangedSorted.map((entry) => entry.node);
-
     this._rootNode = setNodesAtDepth(
       this._rootNode,
       this.type.depth,