perf(core): create new cloners for batches only when the data objects…

…' shape changes
genome-spy · May 13, 2024 · 2eeb9c3 · 2eeb9c3
1 parent c26cc41
commit 2eeb9c3
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 12 deletions.
diff --git a/packages/core/src/data/transforms/clone.js b/packages/core/src/data/transforms/clone.js
@@ -1,4 +1,5 @@
-import createCloner from "../../utils/cloner.js";
+import { shallowArrayEquals } from "../../utils/arrayUtils.js";
+import createCloner, { getAllProperties } from "../../utils/cloner.js";
 import FlowNode, { BEHAVIOR_CLONES, isFileBatch } from "../flowNode.js";
 
 /**
@@ -9,15 +10,26 @@ export default class CloneTransform extends FlowNode {
         return BEHAVIOR_CLONES;
     }
 
+    /** @type {string[]} */
+    #lastBatchFields;
+
     constructor() {
         super();
 
         /** @param {any} datum */
         const setupCloner = (datum) => {
-            const clone = createCloner(datum);
+            // Create a new cloner if the fields have changed
+            const fields = getAllProperties(datum);
+            if (
+                !this.#lastBatchFields ||
+                !shallowArrayEquals(fields, this.#lastBatchFields)
+            ) {
+                this.#lastBatchFields = fields;
+                const clone = createCloner(datum);
 
-            /** @param {any} datum */
-            this.handle = (datum) => this._propagate(clone(datum));
+                /** @param {any} datum */
+                this.handle = (datum) => this._propagate(clone(datum));
+            }
 
             this.handle(datum);
         };
@@ -31,7 +43,6 @@ export default class CloneTransform extends FlowNode {
          */
         this.beginBatch = (flowBatch) => {
             if (isFileBatch(flowBatch)) {
-                // TODO: Only create new cloner if the props change
                 this.handle = setupCloner;
             }
             super.beginBatch(flowBatch);

diff --git a/packages/core/src/utils/cloner.js b/packages/core/src/utils/cloner.js
@@ -8,12 +8,12 @@
  *
  * @param {T} template The template object that
  * @returns {(function(T):T) & { properties: string[] }}
- * @template T
+ * @template {object} [T=object]
  */
 export default function createCloner(template) {
     // TODO: Check that only properties, not methods get cloned
     const properties = /** @type {string[]} */ (
-        Object.keys(template).filter((k) => typeof k == "string")
+        getAllProperties(template).filter((k) => typeof k == "string")
     );
 
     const cloner = /** @type {(function(T):T) & { properties: string[] }} */ (
@@ -32,3 +32,20 @@ export default function createCloner(template) {
 
     return cloner;
 }
+
+/**
+ * Needed for proxied Apache Arrow tables.
+ *
+ * @param {object} obj
+ */
+export function getAllProperties(obj) {
+    /** @type {string[]} */
+    let props = [];
+    do {
+        props = props.concat(Object.keys(obj));
+        obj = Object.getPrototypeOf(obj);
+    } while (obj && obj !== Object.prototype); // Traverse until the end of the prototype chain
+
+    const uniqueProps = Array.from(new Set(props));
+    return uniqueProps;
+}
diff --git a/packages/core/src/utils/cloner.test.js b/packages/core/src/utils/cloner.test.js
@@ -1,10 +1,11 @@
 import { expect, test } from "vitest";
-import createCloner from "./cloner.js";
+import createCloner, { getAllProperties } from "./cloner.js";
 
 const template = {
+    1: "iddqd",
     a: 1,
-    b: "xyzzy",
-    3: "iddqd",
+    c: "xyzzy",
+    b: "idclip",
 };
 
 test("Cloner clones object properly", () => {
@@ -14,11 +15,21 @@ test("Cloner clones object properly", () => {
     expect(makeClone(template)).not.toBe(template);
 
     const another = {
+        1: "hello",
         a: 2,
-        b: "idkfa",
-        3: "hello",
+        c: "idkfa",
+        b: "idclip",
     };
 
     expect(makeClone(another)).toEqual(another);
     expect(makeClone(another)).not.toBe(another);
 });
+
+test("getAllProperties", () => {
+    expect(getAllProperties(template)).toEqual(["1", "a", "c", "b"]);
+
+    const obj = Object.create(template);
+    obj.d = 42;
+
+    expect(getAllProperties(obj)).toEqual(["d", "1", "a", "c", "b"]);
+});