diff --git a/app/client/src/sagas/EvaluationsSaga.test.ts b/app/client/src/sagas/EvaluationsSaga.test.ts index 87686bb4a3a1..9411d9a072ba 100644 --- a/app/client/src/sagas/EvaluationsSaga.test.ts +++ b/app/client/src/sagas/EvaluationsSaga.test.ts @@ -1,10 +1,14 @@ import { defaultAffectedJSObjects, + evalAndLintingHandler, evalQueueBuffer, evaluateTreeSaga, + evaluationLoopWithDebounce, + type BUFFERED_ACTION, } from "./EvaluationsSaga"; import { evalWorker } from "utils/workerInstances"; -import { expectSaga } from "redux-saga-test-plan"; +import { expectSaga, testSaga } from "redux-saga-test-plan"; + import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { select } from "redux-saga/effects"; import { getMetaWidgets, getWidgets, getWidgetsMeta } from "./selectors"; @@ -76,6 +80,7 @@ describe("evaluateTreeSaga", () => { widgetsMeta: {}, shouldRespondWithLogs: true, affectedJSObjects: { ids: [], isAllAffected: false }, + actionDataPayloadConsolidated: undefined, }) .run(); }); @@ -121,6 +126,7 @@ describe("evaluateTreeSaga", () => { widgetsMeta: {}, shouldRespondWithLogs: false, affectedJSObjects: { ids: [], isAllAffected: false }, + actionDataPayloadConsolidated: undefined, }) .run(); }); @@ -175,6 +181,7 @@ describe("evaluateTreeSaga", () => { widgetsMeta: {}, shouldRespondWithLogs: false, affectedJSObjects, + actionDataPayloadConsolidated: undefined, }) .run(); }); @@ -385,3 +392,136 @@ describe("evalQueueBuffer", () => { }); }); }); + +describe("evaluationLoopWithDebounce", () => { + describe("debounce", () => { + test("should call a regular evaluation with the consolidated action data payload when both updateActionData and evaluation action is triggered", async () => { + const buffer = evalQueueBuffer(); + + buffer.put( + updateActionData([ + { + entityName: "widget1", + dataPath: "data", + data: { a: 1 }, + dataPathRef: "", + }, + ]), + ); + buffer.put( + updateActionData([ + { + entityName: "widget2", + dataPath: "data", + data: { a: 2 }, + dataPathRef: "", + }, + ]), + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + buffer.put(createJSCollectionSuccess({ id: "1" } as any)); + const action = buffer.take(); + + const mockChannel = "mock-channel"; + + // assert that a regular evaluation is only triggered and no evalTreeWithChanges evaluation is triggered + return ( + testSaga(evaluationLoopWithDebounce, mockChannel) + .next() + .take(mockChannel) + .next(action) + .call(evalAndLintingHandler, true, action, { + actionDataPayloadConsolidated: [ + { + entityName: "widget1", + dataPath: "data", + data: { a: 1 }, + dataPathRef: "", + }, + { + entityName: "widget2", + dataPath: "data", + data: { a: 2 }, + dataPathRef: "", + }, + ], + shouldReplay: undefined, + forceEvaluation: false, + requiresLogging: undefined, + affectedJSObjects: { isAllAffected: false, ids: ["1"] }, + }) + .next() + // wait for the next action in the event loop + .take(mockChannel) + ); + }); + test("should call an evalTreeWithChanges when only updateActionData actions are triggered", async () => { + const buffer = evalQueueBuffer(); + + buffer.put( + updateActionData([ + { + entityName: "widget1", + dataPath: "data", + data: { a: 1 }, + dataPathRef: "", + }, + ]), + ); + buffer.put( + updateActionData([ + { + entityName: "widget2", + dataPath: "data", + data: { a: 2 }, + dataPathRef: "", + }, + ]), + ); + const action = buffer.take() as unknown as BUFFERED_ACTION; + + const mockChannel = "mock-channel"; + + return ( + testSaga(evaluationLoopWithDebounce, mockChannel) + .next() + .take(mockChannel) + .next(action) + .call( + evalWorker.request, + EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA, + action.actionDataPayloadConsolidated, + ) + .next() + // wait for the next action in the event loop + .take(mockChannel) + ); + }); + test("should call a regular evaluation when evaluation actions are triggered", async () => { + const buffer = evalQueueBuffer(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + buffer.put(createJSCollectionSuccess({ id: "1" } as any)); + + const action = buffer.take(); + + const mockChannel = "mock-channel"; + + return ( + testSaga(evaluationLoopWithDebounce, mockChannel) + .next() + .take(mockChannel) + .next(action) + .call(evalAndLintingHandler, true, action, { + shouldReplay: undefined, + forceEvaluation: false, + requiresLogging: undefined, + affectedJSObjects: { isAllAffected: false, ids: ["1"] }, + }) + .next() + // wait for the next action in the event loop + .take(mockChannel) + ); + }); + }); +}); diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index f0b067526904..18c4e04aa490 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -249,6 +249,7 @@ export function* evaluateTreeSaga( forceEvaluation = false, requiresLogging = false, affectedJSObjects: AffectedJSObjects = defaultAffectedJSObjects, + actionDataPayloadConsolidated?: actionDataPayload, ) { const allActionValidationConfig: ReturnType< typeof getAllActionValidationConfig @@ -291,6 +292,7 @@ export function* evaluateTreeSaga( widgetsMeta, shouldRespondWithLogs, affectedJSObjects, + actionDataPayloadConsolidated, }; const workerResponse: EvalTreeResponseData = yield call( @@ -542,7 +544,7 @@ export const defaultAffectedJSObjects: AffectedJSObjects = { ids: [], }; -interface BUFFERED_ACTION { +export interface BUFFERED_ACTION { hasDebouncedHandleUpdate: boolean; hasBufferedAction: boolean; actionDataPayloadConsolidated: actionDataPayload[]; @@ -674,7 +676,7 @@ function getPostEvalActions( return postEvalActions; } -function* evalAndLintingHandler( +export function* evalAndLintingHandler( isBlockingCall = true, action: ReduxAction, options: Partial<{ @@ -682,11 +684,17 @@ function* evalAndLintingHandler( forceEvaluation: boolean; requiresLogging: boolean; affectedJSObjects: AffectedJSObjects; + actionDataPayloadConsolidated: actionDataPayload[]; }>, ) { const span = startRootSpan("evalAndLintingHandler"); - const { affectedJSObjects, forceEvaluation, requiresLogging, shouldReplay } = - options; + const { + actionDataPayloadConsolidated, + affectedJSObjects, + forceEvaluation, + requiresLogging, + shouldReplay, + } = options; const requiresLinting = getRequiresLinting(action); @@ -728,6 +736,7 @@ function* evalAndLintingHandler( forceEvaluation, requiresLogging, affectedJSObjects, + actionDataPayloadConsolidated, ), ); } @@ -800,6 +809,13 @@ function* evaluationChangeListenerSaga(): any { evalQueueBuffer(), ); + yield call(evaluationLoopWithDebounce, evtActionChannel); +} + +export function* evaluationLoopWithDebounce( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + evtActionChannel: ActionPattern>, +) { while (true) { const action: EvaluationReduxAction = yield take(evtActionChannel); @@ -834,6 +850,23 @@ function* evaluationChangeListenerSaga(): any { hasDebouncedHandleUpdate, } = action as unknown as BUFFERED_ACTION; + // when there are both debounced action updates evaluation and a regular evaluation + // we will convert that to a regular evaluation this should help in performance by + // not performing a debounced action updates evaluation + if (hasDebouncedHandleUpdate && hasBufferedAction) { + const affectedJSObjects = getAffectedJSObjectIdsFromAction(action); + + yield call(evalAndLintingHandler, true, action, { + actionDataPayloadConsolidated, + shouldReplay: get(action, "payload.shouldReplay"), + forceEvaluation: shouldForceEval(action), + requiresLogging: shouldLog(action), + affectedJSObjects, + }); + + continue; + } + if (hasDebouncedHandleUpdate) { yield call( evalWorker.request, @@ -856,7 +889,6 @@ function* evaluationChangeListenerSaga(): any { } } } - // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any export function* evaluateActionSelectorFieldSaga(action: any) { diff --git a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts index df79758b4ba9..86040aca12d2 100644 --- a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts +++ b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts @@ -4,7 +4,7 @@ import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger"; import type { UpdatedPathsMap } from "workers/Evaluation/JSObject/JSVariableUpdates"; import { applyJSVariableUpdatesToEvalTree } from "workers/Evaluation/JSObject/JSVariableUpdates"; import ExecutionMetaData from "./ExecutionMetaData"; -import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData"; +import type { UpdateActionProps } from "workers/Evaluation/handlers/types"; import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData"; import { getEntityNameAndPropertyPath } from "ee/workers/Evaluation/evaluationUtils"; import type { Patch } from "workers/Evaluation/JSObject/Collection"; diff --git a/app/client/src/workers/Evaluation/handlers/evalTree.ts b/app/client/src/workers/Evaluation/handlers/evalTree.ts index c8e53a4fc9e3..52b6ccacd908 100644 --- a/app/client/src/workers/Evaluation/handlers/evalTree.ts +++ b/app/client/src/workers/Evaluation/handlers/evalTree.ts @@ -34,6 +34,7 @@ import { import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer"; import type { MetaWidgetsReduxState } from "reducers/entityReducers/metaWidgetsReducer"; import type { Attributes } from "instrumentation/types"; +import { updateActionsToEvalTree } from "./updateActionData"; // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -70,6 +71,7 @@ export async function evalTree( let isNewWidgetAdded = false; const { + actionDataPayloadConsolidated, affectedJSObjects, allActionValidationConfig, appMode, @@ -190,6 +192,13 @@ export async function evalTree( }); staleMetaIds = dataTreeResponse.staleMetaIds; } else { + const tree = dataTreeEvaluator.getEvalTree(); + + // during update cycles update actions to the dataTree directly + // this is useful in cases where we have debounced updateActionData and a regular evaluation + // triggered together, in those cases we merge them both into a regular evaluation + updateActionsToEvalTree(tree, actionDataPayloadConsolidated); + if (dataTreeEvaluator && !isEmpty(allActionValidationConfig)) { dataTreeEvaluator.setAllActionValidationConfig( allActionValidationConfig, @@ -212,6 +221,7 @@ export async function evalTree( configTree, webworkerTelemetry, affectedJSObjects, + actionDataPayloadConsolidated, ), ); diff --git a/app/client/src/workers/Evaluation/handlers/types.ts b/app/client/src/workers/Evaluation/handlers/types.ts new file mode 100644 index 000000000000..a1e2d0b3dc02 --- /dev/null +++ b/app/client/src/workers/Evaluation/handlers/types.ts @@ -0,0 +1,6 @@ +export interface UpdateActionProps { + entityName: string; + dataPath: string; + data: unknown; + dataPathRef?: string; +} diff --git a/app/client/src/workers/Evaluation/handlers/updateActionData.ts b/app/client/src/workers/Evaluation/handlers/updateActionData.ts index dadf1f5654e5..ccb3f6693e69 100644 --- a/app/client/src/workers/Evaluation/handlers/updateActionData.ts +++ b/app/client/src/workers/Evaluation/handlers/updateActionData.ts @@ -4,13 +4,9 @@ import set from "lodash/set"; import { evalTreeWithChanges } from "../evalTreeWithChanges"; import DataStore from "../dataStore"; import { EVAL_WORKER_SYNC_ACTION } from "ee/workers/Evaluation/evalWorkerActions"; +import type { DataTree } from "entities/DataTree/dataTreeTypes"; +import type { UpdateActionProps } from "./types"; -export interface UpdateActionProps { - entityName: string; - dataPath: string; - data: unknown; - dataPathRef?: string; -} export default function (request: EvalWorkerSyncRequest) { const actionsDataToUpdate: UpdateActionProps[] = request.data; @@ -26,6 +22,29 @@ export function handleActionsDataUpdate(actionsToUpdate: UpdateActionProps[]) { const evalTree = dataTreeEvaluator.getEvalTree(); + updateActionsToEvalTree(evalTree, actionsToUpdate); + + const updatedProperties: string[][] = []; + + actionsToUpdate.forEach(({ dataPath, entityName }) => { + updatedProperties.push([entityName, dataPath]); + }); + evalTreeWithChanges({ + data: { + updatedValuePaths: updatedProperties, + metaUpdates: [], + }, + method: EVAL_WORKER_SYNC_ACTION.EVAL_TREE_WITH_CHANGES, + webworkerTelemetry: {}, + }); +} + +export function updateActionsToEvalTree( + evalTree: DataTree, + actionsToUpdate?: UpdateActionProps[], +) { + if (!actionsToUpdate) return; + for (const actionToUpdate of actionsToUpdate) { const { dataPath, dataPathRef, entityName } = actionToUpdate; let { data } = actionToUpdate; @@ -44,18 +63,4 @@ export function handleActionsDataUpdate(actionsToUpdate: UpdateActionProps[]) { DataStore.setActionData(path, data); } - - const updatedProperties: string[][] = []; - - actionsToUpdate.forEach(({ dataPath, entityName }) => { - updatedProperties.push([entityName, dataPath]); - }); - evalTreeWithChanges({ - data: { - updatedValuePaths: updatedProperties, - metaUpdates: [], - }, - method: EVAL_WORKER_SYNC_ACTION.EVAL_TREE_WITH_CHANGES, - webworkerTelemetry: {}, - }); } diff --git a/app/client/src/workers/Evaluation/types.ts b/app/client/src/workers/Evaluation/types.ts index 97adaa6fc2b4..a6e450e9d1cd 100644 --- a/app/client/src/workers/Evaluation/types.ts +++ b/app/client/src/workers/Evaluation/types.ts @@ -18,6 +18,7 @@ import type { APP_MODE } from "entities/App"; import type { WebworkerSpanData, Attributes } from "instrumentation/types"; import type { ICacheProps } from "../common/AppComputationCache/types"; import type { AffectedJSObjects } from "actions/EvaluationReduxActionTypes"; +import type { UpdateActionProps } from "./handlers/types"; // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -51,6 +52,7 @@ export interface EvalTreeRequestData { widgetsMeta: Record; shouldRespondWithLogs?: boolean; affectedJSObjects: AffectedJSObjects; + actionDataPayloadConsolidated?: UpdateActionProps[]; } export interface EvalTreeResponseData { diff --git a/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts b/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts index ee6a876ee267..514fc3691f91 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts @@ -379,6 +379,67 @@ describe("DataTreeEvaluator", () => { }); }); + describe("actionsToUpdate affects setupTree", () => { + beforeEach(async () => { + await dataTreeEvaluator.setupFirstTree( + unEvalTree as unknown as DataTree, + configTree as unknown as ConfigTree, + {}, + { + appId: "appId", + pageId: "pageId", + timestamp: "timestamp", + appMode: APP_MODE.PUBLISHED, + instanceId: "instanceId", + }, + ); + dataTreeEvaluator.evalAndValidateFirstTree(); + }); + describe("when unevalTree is the same", () => { + it("should not call the setupTree when there are actionsToUpdate", () => { + const spy = jest.spyOn(dataTreeEvaluator, "setupTree"); + + dataTreeEvaluator.setupUpdateTree( + unEvalTree as unknown as DataTree, + configTree as unknown as ConfigTree, + undefined, + undefined, + [], + ); + + expect(spy).not.toHaveBeenCalled(); + }); + it("should pass in actionsToUpdate as updatedValuePaths to the setupTree", () => { + const spy = jest.spyOn(dataTreeEvaluator, "setupTree"); + + dataTreeEvaluator.setupUpdateTree( + unEvalTree as unknown as DataTree, + configTree as unknown as ConfigTree, + undefined, + undefined, + [ + { + entityName: "JSAction1", + dataPath: "data", + data: ["Some data"], + }, + ], + ); + + expect(spy).toHaveBeenCalledWith( + expect.anything(), + [["JSAction1", "data"]], + { + dependenciesOfRemovedPaths: [], + pathsToSkipFromEval: ["JSAction1.data"], + removedPaths: [], + translatedDiffs: [], + }, + ); + }); + }); + }); + describe("array accessor dependency handling", () => { const dataTreeEvaluator = new DataTreeEvaluator(widgetConfigMap); diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 8f2eb2f09557..dd4e33d8ff88 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -155,6 +155,7 @@ import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv"; import type { WebworkerSpanData, Attributes } from "instrumentation/types"; import type { AffectedJSObjects } from "actions/EvaluationReduxActionTypes"; +import type { UpdateActionProps } from "workers/Evaluation/handlers/types"; type SortedDependencies = Array; export interface EvalProps { @@ -637,6 +638,7 @@ export default class DataTreeEvaluator { configTree: ConfigTree, webworkerTelemetry: Record = {}, affectedJSObjects: AffectedJSObjects = { isAllAffected: false, ids: [] }, + actionDataPayloadConsolidated?: UpdateActionProps[], ): { unEvalUpdates: DataTreeDiff[]; evalOrder: string[]; @@ -677,7 +679,7 @@ export default class DataTreeEvaluator { // Since eval tree is listening to possible events that don't cause differences // We want to check if no diffs are present and bail out early - if (differences.length === 0) { + if (differences.length === 0 && !actionDataPayloadConsolidated?.length) { return { removedPaths: [], unEvalUpdates: [], @@ -724,7 +726,12 @@ export default class DataTreeEvaluator { this.dependencies = dependencies; this.inverseDependencies = inverseDependencies; - const pathsChangedSet = new Set(); + const pathsChangedSet = new Set( + actionDataPayloadConsolidated?.map(({ dataPath, entityName }) => [ + entityName, + dataPath, + ]) || [], + ); for (const diff of differences) { if (isArray(diff.path)) { @@ -735,16 +742,23 @@ export default class DataTreeEvaluator { const updatedValuePaths = [...pathsChangedSet]; this.updateEvalTreeWithChanges({ differences }); - const setupUpdateTreeOutput = profileFn( "setupTree", undefined, webworkerTelemetry, () => { + const pathsToSkipFromEval = + actionDataPayloadConsolidated + ?.map(({ dataPath, entityName }) => { + return [entityName, dataPath]; + }) + .map((path: string[]) => path.join(".")) || []; + return this.setupTree(updatedUnEvalTreeJSObjects, updatedValuePaths, { dependenciesOfRemovedPaths, removedPaths, translatedDiffs, + pathsToSkipFromEval, }); }, );