chore: send serialised updated and using klona/json instead of klona (#27319)

## Description

- We are no longer performing JSON.stringify(JSON.parse(value)) to
clone, clean up undefined values and serialised bigInt.
- We are performing deepClones using klona/json which is much faster and
later we perform the serialisation on the diff.
- We are sending serialised updated to the main thread and it is parsed
on the main thread, this helps in reducing structuredClone cost of
sending unserialised objects.
- Noticed a reduction of worker thread latency by about 75% and doubled
the databound our app can handle.

#### PR fixes following issue(s)
Fixes #26130

#### Type of change
- Chore (housekeeping or task changes that don't impact user perception)

## Testing
>
#### How Has This Been Tested?

- [x] Manual
- [x] JUnit
- [ ] Jest
- [ ] Cypress
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [x] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [x] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed

app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts

@@ -267,94 +267,5 @@ describe("7. Test util methods", () => {
         expect(evalProps).toEqual(initialEvalProps);
       });
     });
-
-    describe("serialise", () => {
-      it("should clean out all functions in the generated state", () => {
-        const state = {
-          Table1: {
-            filteredTableData: smallDataSet,
-            selectedRows: [],
-            // eslint-disable-next-line @typescript-eslint/no-empty-function
-            someFn: () => {},
-            pageSize: 0,
-            __evaluation__: {
-              evaluatedValues: {},
-            },
-          },
-        } as any;
-
-        const identicalEvalPathsPatches = {
-          "Table1.__evaluation__.evaluatedValues.['filteredTableData']":
-            "Table1.filteredTableData",
-        };
-        const evalProps = {
-          Table1: {
-            __evaluation__: {
-              evaluatedValues: {
-                someProp: "abc",
-                // eslint-disable-next-line @typescript-eslint/no-empty-function
-                someEvalFn: () => {},
-              },
-            },
-          },
-        } as any;
-        const dataTree = makeEntityConfigsAsObjProperties(state, {
-          sanitizeDataTree: true,
-          evalProps,
-          identicalEvalPathsPatches,
-        }) as any;
-        const expectedState = produce(state, (draft: any) => {
-          draft.Table1.__evaluation__.evaluatedValues.someProp = "abc";
-          delete draft.Table1.someFn;
-          draft.Table1.__evaluation__.evaluatedValues.filteredTableData =
-            smallDataSet;
-        });
-
-        expect(dataTree).toEqual(expectedState);
-        //function introduced by evalProps is cleaned out
-        expect(
-          dataTree.Table1.__evaluation__.evaluatedValues.someEvalFn,
-        ).toBeUndefined();
-      });
-
-      it("should serialise bigInteger values", () => {
-        const someBigInt = BigInt(121221);
-        const state = {
-          Table1: {
-            pageSize: someBigInt,
-            __evaluation__: {
-              evaluatedValues: {},
-            },
-          },
-        } as any;
-
-        const identicalEvalPathsPatches = {
-          "Table1.__evaluation__.evaluatedValues.['pageSize']":
-            "Table1.pageSize",
-        };
-        const evalProps = {
-          Table1: {
-            __evaluation__: {
-              evaluatedValues: {
-                someProp: someBigInt,
-              },
-            },
-          },
-        } as any;
-
-        const dataTree = makeEntityConfigsAsObjProperties(state, {
-          sanitizeDataTree: true,
-          evalProps,
-          identicalEvalPathsPatches,
-        });
-        const expectedState = produce(state, (draft: any) => {
-          draft.Table1.pageSize = "121221";
-          draft.Table1.__evaluation__.evaluatedValues.pageSize = "121221";
-          draft.Table1.__evaluation__.evaluatedValues.someProp = "121221";
-        });
-
-        expect(dataTree).toEqual(expectedState);
-      });
-    });
   });
 });

app/client/src/ce/workers/Evaluation/dataTreeUtils.ts

@@ -1,7 +1,7 @@
 import type { DataTree } from "entities/DataTree/dataTreeFactory";
+import { klona } from "klona/json";
 import { get, set, unset } from "lodash";
 import type { EvalProps } from "workers/common/DataTreeEvaluator";
-import { removeFunctionsAndSerialzeBigInt } from "@appsmith/workers/Evaluation/evaluationUtils";
 
 /**
  * This method loops through each entity object of dataTree and sets the entity config from prototype as object properties.
@@ -25,9 +25,7 @@ export function makeEntityConfigsAsObjProperties(
     const entity = dataTree[entityName];
     newDataTree[entityName] = Object.assign({}, entity);
   }
-  const dataTreeToReturn = sanitizeDataTree
+  const dataTreeToReturn = sanitizeDataTree ? klona(newDataTree) : newDataTree;
-    ? removeFunctionsAndSerialzeBigInt(newDataTree)
-    : newDataTree;
 
   if (!evalProps) return dataTreeToReturn;
 
@@ -58,9 +56,7 @@ export function makeEntityConfigsAsObjProperties(
     unset(evalProps, evalPath);
   });
 
-  const sanitizedEvalProps = removeFunctionsAndSerialzeBigInt(
+  const sanitizedEvalProps = klona(evalProps) as EvalProps;
-    evalProps,
-  ) as EvalProps;
   Object.entries(alreadySanitisedDataSet).forEach(([path, val]) => {
     // add it to sanitised Eval props
     set(sanitizedEvalProps, path, val);

app/client/src/ce/workers/Evaluation/evaluationUtils.ts

@@ -418,10 +418,11 @@ export function isDataTreeEntity(entity: unknown) {
   return !!entity && typeof entity === "object" && "ENTITY_TYPE" in entity;
 }
 
+export const serialiseToBigInt = (value: any) =>
+  JSON.stringify(value, (_, v) => (typeof v === "bigint" ? v.toString() : v));
+
 export const removeFunctionsAndSerialzeBigInt = (value: any) =>
-  JSON.parse(
+  JSON.parse(serialiseToBigInt(value));
-    JSON.stringify(value, (_, v) => (typeof v === "bigint" ? v.toString() : v)),
-  );
 // We need to remove functions from data tree to avoid any unexpected identifier while JSON parsing
 // Check issue https://github.com/appsmithorg/appsmith/issues/719
 export const removeFunctions = (value: any) => {

app/client/src/sagas/EvaluationSaga.utils.ts

@@ -0,0 +1,40 @@
+import log from "loglevel";
+import type { DiffWithReferenceState } from "workers/Evaluation/helpers";
+
+export const parseUpdatesAndDeleteUndefinedUpdates = (
+  updates: string,
+): DiffWithReferenceState[] => {
+  let parsedUpdates = [];
+  try {
+    //Parse updates from a string
+    parsedUpdates = JSON.parse(updates);
+  } catch (e) {
+    log.error("Failed to parse updates", e, updates);
+    return [];
+  }
+  //delete all undefined properties from the state
+  const { deleteUpdates, regularUpdates } = parsedUpdates.reduce(
+    (acc: any, curr: any) => {
+      const { kind, path, rhs } = curr;
+
+      if (rhs === undefined) {
+        //ignore any new undefined updates to the state if the value is undefined
+        if (kind === "N") {
+          return acc;
+        }
+        //convert undefined updates to delete updates
+        if (kind === "E") {
+          acc.deleteUpdates.push({ kind: "D", path });
+          return acc;
+        }
+      }
+
+      acc.regularUpdates.push(curr);
+      return acc;
+    },
+    { regularUpdates: [], deleteUpdates: [] },
+  );
+
+  const consolidatedUpdates = [...regularUpdates, ...deleteUpdates];
+  return consolidatedUpdates;
+};

app/client/src/sagas/EvaluationsSaga.ts

@@ -100,6 +100,7 @@ import { executeJSUpdates } from "actions/pluginActionActions";
 import { setEvaluatedActionSelectorField } from "actions/actionSelectorActions";
 import { waitForWidgetConfigBuild } from "./InitSagas";
 import { logDynamicTriggerExecution } from "@appsmith/sagas/analyticsSaga";
+import { parseUpdatesAndDeleteUndefinedUpdates } from "./EvaluationSaga.utils";
 const APPSMITH_CONFIGS = getAppsmithConfigs();
 export const evalWorker = new GracefulWorkerService(
   new Worker(
@@ -158,8 +159,8 @@ export function* updateDataTreeHandler(
   if (!isEmpty(staleMetaIds)) {
     yield put(resetWidgetsMetaState(staleMetaIds));
   }
-
+  const parsedUpdates = parseUpdatesAndDeleteUndefinedUpdates(updates);
-  yield put(setEvaluatedTree(updates));
+  yield put(setEvaluatedTree(parsedUpdates));
 
   ConfigTreeActions.setConfigTree(configTree);
 

app/client/src/utils/DynamicBindingUtils.ts

@@ -145,6 +145,7 @@ export enum EvalErrorTypes {
   PARSE_JS_ERROR = "PARSE_JS_ERROR",
   EXTRACT_DEPENDENCY_ERROR = "EXTRACT_DEPENDENCY_ERROR",
   CLONE_ERROR = "CLONE_ERROR",
+  SERIALIZATION_ERROR = "SERIALIZATION_ERROR",
 }
 
 export type EvalError = {

app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts

@@ -1,6 +1,14 @@
+/* eslint-disable @typescript-eslint/no-empty-function */
+import { applyChange } from "deep-diff";
 import produce from "immer";
 import { range } from "lodash";
-import { generateOptimisedUpdates } from "../helpers";
+import { parseUpdatesAndDeleteUndefinedUpdates } from "sagas/EvaluationSaga.utils";
+import { EvalErrorTypes } from "utils/DynamicBindingUtils";
+
+import {
+  generateOptimisedUpdates,
+  generateSerialisedUpdates,
+} from "../helpers";
 
 export const smallDataSet = [
   {
@@ -74,7 +82,7 @@ const oldState = {
   },
 };
 
-describe("optimised diff updates", () => {
+describe("generateOptimisedUpdates", () => {
   describe("regular diff", () => {
     test("should generate regular diff updates when a simple property changes in the widget property segment", () => {
       const newState = produce(oldState, (draft) => {
@@ -246,3 +254,217 @@ describe("optimised diff updates", () => {
     });
   });
 });
+
+//we are testing the flow of serialised updates generated from the worker thread and subsequently applied to the main thread state
+describe("generateSerialisedUpdates and parseUpdatesAndDeleteUndefinedUpdates", () => {
+  it("should ignore undefined updates", () => {
+    const oldStateWithUndefinedValues = produce(oldState, (draft: any) => {
+      draft.Table1.pageSize = undefined;
+    });
+
+    const { serialisedUpdates } = generateSerialisedUpdates(
+      oldStateWithUndefinedValues,
+      //new state has the same undefined value
+      oldStateWithUndefinedValues,
+      {},
+    );
+    //no change hence empty array
+    expect(serialisedUpdates).toEqual("[]");
+  });
+  it("should generate a delete patch when a property is transformed to undefined", () => {
+    const oldStateWithUndefinedValues = produce(oldState, (draft: any) => {
+      draft.Table1.pageSize = undefined;
+    });
+
+    const { serialisedUpdates } = generateSerialisedUpdates(
+      oldState,
+      oldStateWithUndefinedValues,
+      {},
+    );
+    const parsedUpdates =
+      parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates);
+    expect(parsedUpdates).toEqual([
+      {
+        kind: "D",
+        path: ["Table1", "pageSize"],
+      },
+    ]);
+  });
+  it("should generate an error when there is a serialisation error", () => {
+    const oldStateWithUndefinedValues = produce(oldState, (draft: any) => {
+      //generate a cyclical object
+      draft.Table1.filteredTableData = draft.Table1;
+    });
+    const { error, serialisedUpdates } = generateSerialisedUpdates(
+      oldState,
+      oldStateWithUndefinedValues,
+      {},
+    );
+
+    expect(error?.type).toEqual(EvalErrorTypes.SERIALIZATION_ERROR);
+    //when a serialisation error occurs we should not return an error
+    expect(serialisedUpdates).toEqual("[]");
+  });
+  //when functions are serialised they become undefined and these updates should be deleted from the state
+  describe("clean out all functions in the generated state", () => {
+    it("should clean out new function properties added to the generated state", () => {
+      const newStateWithSomeFnProperty = produce(oldState, (draft: any) => {
+        draft.Table1.someFn = () => {};
+        draft.Table1.__evaluation__.evaluatedValues.someEvalFn = () => {};
+      });
+
+      const { serialisedUpdates } = generateSerialisedUpdates(
+        oldState,
+        newStateWithSomeFnProperty,
+        {},
+      );
+
+      const parsedUpdates =
+        parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates);
+      //should ignore all function updates
+      expect(parsedUpdates).toEqual([]);
+
+      const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => {
+        parsedUpdates.forEach((v: any) => {
+          applyChange(draft, undefined, v);
+        });
+      });
+      //no change in state
+      expect(parseAndApplyUpdatesToOldState).toEqual(oldState);
+    });
+
+    it("should delete properties which get updated to a function", () => {
+      const newStateWithSomeFnProperty = produce(oldState, (draft: any) => {
+        draft.Table1.pageSize = () => {};
+        draft.Table1.__evaluation__.evaluatedValues.transientTableData =
+          () => {};
+      });
+
+      const { serialisedUpdates } = generateSerialisedUpdates(
+        oldState,
+        newStateWithSomeFnProperty,
+        {},
+      );
+
+      const parsedUpdates =
+        parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates);
+
+      expect(parsedUpdates).toEqual([
+        {
+          kind: "D",
+          path: ["Table1", "pageSize"],
+        },
+        {
+          kind: "D",
+          path: [
+            "Table1",
+            "__evaluation__",
+            "evaluatedValues",
+            "transientTableData",
+          ],
+        },
+      ]);
+
+      const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => {
+        parsedUpdates.forEach((v: any) => {
+          applyChange(draft, undefined, v);
+        });
+      });
+      const expectedState = produce(oldState, (draft: any) => {
+        delete draft.Table1.pageSize;
+        delete draft.Table1.__evaluation__.evaluatedValues.transientTableData;
+      });
+
+      expect(parseAndApplyUpdatesToOldState).toEqual(expectedState);
+    });
+    it("should delete function properties which get updated to undefined", () => {
+      const oldStateWithSomeFnProperty = produce(oldState, (draft: any) => {
+        // eslint-disable-next-line @typescript-eslint/no-empty-function
+        draft.Table1.pageSize = () => {};
+        draft.Table1.__evaluation__.evaluatedValues.transientTableData =
+          // eslint-disable-next-line @typescript-eslint/no-empty-function
+          () => {};
+      });
+      const newStateWithFnsTransformedToUndefined = produce(
+        oldState,
+        (draft: any) => {
+          draft.Table1.pageSize = undefined;
+          draft.Table1.__evaluation__.evaluatedValues.transientTableData =
+            undefined;
+        },
+      );
+
+      const { serialisedUpdates } = generateSerialisedUpdates(
+        oldStateWithSomeFnProperty,
+        newStateWithFnsTransformedToUndefined,
+        {},
+      );
+
+      const parsedUpdates =
+        parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates);
+
+      expect(parsedUpdates).toEqual([
+        {
+          kind: "D",
+          path: ["Table1", "pageSize"],
+        },
+        {
+          kind: "D",
+          path: [
+            "Table1",
+            "__evaluation__",
+            "evaluatedValues",
+            "transientTableData",
+          ],
+        },
+      ]);
+
+      const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => {
+        parsedUpdates.forEach((v: any) => {
+          applyChange(draft, undefined, v);
+        });
+      });
+      const expectedState = produce(oldState, (draft: any) => {
+        delete draft.Table1.pageSize;
+        delete draft.Table1.__evaluation__.evaluatedValues.transientTableData;
+      });
+
+      expect(parseAndApplyUpdatesToOldState).toEqual(expectedState);
+    });
+  });
+
+  it("should serialise bigInteger values", () => {
+    const someBigInt = BigInt(121221);
+    const newStateWithBigInt = produce(oldState, (draft: any) => {
+      draft.Table1.pageSize = someBigInt;
+    });
+    const { serialisedUpdates } = generateSerialisedUpdates(
+      oldState,
+      newStateWithBigInt,
+      {},
+    );
+
+    const parsedUpdates =
+      parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates);
+
+    //should generate serialised bigInt update
+    expect(parsedUpdates).toEqual([
+      {
+        kind: "E",
+        path: ["Table1", "pageSize"],
+        rhs: "121221",
+      },
+    ]);
+
+    const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => {
+      parsedUpdates.forEach((v: any) => {
+        applyChange(draft, undefined, v);
+      });
+    });
+    const expectedState = produce(oldState, (draft: any) => {
+      draft.Table1.pageSize = "121221";
+    });
+
+    expect(parseAndApplyUpdatesToOldState).toEqual(expectedState);
+  });
+});

app/client/src/workers/Evaluation/helpers.ts

@@ -1,9 +1,10 @@
+import { serialiseToBigInt } from "@appsmith/workers/Evaluation/evaluationUtils";
 import type { Diff } from "deep-diff";
 import { diff } from "deep-diff";
 import type { DataTree } from "entities/DataTree/dataTreeFactory";
 import equal from "fast-deep-equal";
-import produce from "immer";
+import { get, isNumber, isObject } from "lodash";
-import { get, isNumber, isObject, set } from "lodash";
+import { EvalErrorTypes } from "utils/DynamicBindingUtils";
 
 export interface DiffReferenceState {
   kind: "referenceState";
@@ -200,6 +201,15 @@ const generateDiffUpdates = (
 
       const lhs = get(oldDataTree, segmentedPath);
 
+      if (rhs === undefined) {
+        //if an undefined value is being set it should be a delete
+        if (lhs !== undefined) {
+          attachDirectly.push({ kind: "D", lhs, path: segmentedPath });
+        }
+        // if the lhs is also undefined ignore diff on this node
+        return true;
+      }
+
       const isLhsLarge = isLargeCollection(lhs);
       const isRhsLarge = isLargeCollection(rhs);
       if (!isLhsLarge && !isRhsLarge) {
@@ -246,17 +256,40 @@ export const generateOptimisedUpdates = (
   return updates;
 };
 
-export const decompressIdenticalEvalPaths = (
+export const generateSerialisedUpdates = (
-  dataTree: any,
+  prevState: any,
-  identicalEvalPathsPatches: Record<string, string>,
+  currentState: any,
-) =>
+  identicalEvalPathsPatches: any,
-  produce(dataTree, (draft: any) =>
+): {
-    Object.entries(identicalEvalPathsPatches || {}).forEach(([key, value]) => {
+  serialisedUpdates: string;
-      const referencePathValue = get(dataTree, value);
+  error?: { type: string; message: string };
-      set(draft, key, referencePathValue);
+} => {
-    }),
+  const updates = generateOptimisedUpdates(
+    prevState,
+    currentState,
+    identicalEvalPathsPatches,
   );
 
+  //remove lhs from diff to reduce the size of diff upload,
+  //it is not necessary to send lhs and we can make the payload to transfer to the main thread smaller for quicker transfer
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const removedLhs = updates.map(({ lhs, ...rest }: any) => rest);
+
+  try {
+    // serialise bigInt values and convert the updates to a string over here to minismise the cost of transfer
+    // to the main thread. In the main thread parse this object there.
+    return { serialisedUpdates: serialiseToBigInt(removedLhs) };
+  } catch (error) {
+    return {
+      serialisedUpdates: "[]",
+      error: {
+        type: EvalErrorTypes.SERIALIZATION_ERROR,
+        message: (error as Error).message,
+      },
+    };
+  }
+};
+
 export const generateOptimisedUpdatesAndSetPrevState = (
   dataTree: any,
   dataTreeEvaluator: any,
@@ -264,12 +297,15 @@ export const generateOptimisedUpdatesAndSetPrevState = (
   const identicalEvalPathsPatches =
     dataTreeEvaluator?.getEvalPathsIdenticalToState();
 
-  const updates = generateOptimisedUpdates(
+  const { error, serialisedUpdates } = generateSerialisedUpdates(
-    dataTreeEvaluator?.getPrevState(),
+    dataTreeEvaluator.getPrevState(),
     dataTree,
     identicalEvalPathsPatches,
   );
 
+  if (error) {
+    dataTreeEvaluator.errors.push(error);
+  }
   dataTreeEvaluator?.setPrevState(dataTree);
-  return updates;
+  return serialisedUpdates;
 };

app/client/src/workers/Evaluation/types.ts

@@ -18,7 +18,6 @@ import type { EvalMetaUpdates } from "@appsmith/workers/common/DataTreeEvaluator
 import type { WorkerRequest } from "@appsmith/workers/common/types";
 import type { DataTreeDiff } from "@appsmith/workers/Evaluation/evaluationUtils";
 import type { APP_MODE } from "entities/App";
-import type { DiffWithReferenceState } from "./helpers";
 
 export type EvalWorkerSyncRequest = WorkerRequest<any, EVAL_WORKER_SYNC_ACTION>;
 export type EvalWorkerASyncRequest = WorkerRequest<
@@ -57,7 +56,7 @@ export interface EvalTreeResponseData {
   isNewWidgetAdded: boolean;
   undefinedEvalValuesMap: Record<string, boolean>;
   jsVarsCreatedEvent?: { path: string; type: string }[];
-  updates: DiffWithReferenceState[];
+  updates: string;
 }
 
 export type JSVarMutatedEvents = Record<string, { path: string; type: string }>;