Skip to content

Commit f9664a3

Browse files
vsvamsi1vsvamsi1
authored
chore: debounce handle action updates (#38396)
## Description Debounced handleActionUpdate actions together with bufferedActions, this has reduced the webworker scripting and LCP by about 25-30% on a windows machine. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12542044958> > Commit: 834a437 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12542044958&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 30 Dec 2024 06:24:18 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced action data handling and evaluation mechanisms - Improved Redux action processing for more efficient updates - **Refactor** - Streamlined saga logic for action data management - Updated type definitions to improve code clarity and type safety - **Tests** - Added comprehensive test cases for action data buffering and consolidation <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 2246bde commit f9664a3

File tree

5 files changed

+219
-31
lines changed

5 files changed

+219
-31
lines changed

‎app/client/src/actions/pluginActionActions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ export const bindDataOnCanvas = (payload: {
388388
};
389389
};
390390

391-
type actionDataPayload = {
391+
export type actionDataPayload = {
392392
entityName: string;
393393
dataPath: string;
394394
data: unknown;

‎app/client/src/ce/actions/evaluationActionsList.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ export const EVALUATE_REDUX_ACTIONS = [
108108
ReduxActionTypes.BUFFERED_ACTION,
109109
// Generic
110110
ReduxActionTypes.TRIGGER_EVAL,
111+
ReduxActionTypes.UPDATE_ACTION_DATA,
111112
];
112113
// Topics used for datasource and query form evaluations
113114
export const FORM_EVALUATION_REDUX_ACTIONS = [

‎app/client/src/sagas/ActionExecution/PluginActionSaga.ts‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
takeLatest,
1010
} from "redux-saga/effects";
1111
import * as Sentry from "@sentry/react";
12-
import type { updateActionDataPayloadType } from "actions/pluginActionActions";
1312
import {
1413
clearActionResponse,
1514
executePageLoadActions,
@@ -104,7 +103,6 @@ import { EMPTY_RESPONSE } from "components/editorComponents/emptyResponse";
104103
import type { AppState } from "ee/reducers";
105104
import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "ee/constants/ApiConstants";
106105
import { evaluateActionBindings } from "sagas/EvaluationsSaga";
107-
import { evalWorker } from "utils/workerInstances";
108106
import { isBlobUrl, parseBlobUrl } from "utils/AppsmithUtils";
109107
import { getType, Types } from "utils/TypeHelpers";
110108
import { matchPath } from "react-router";
@@ -152,7 +150,6 @@ import {
152150
getCurrentEnvironmentDetails,
153151
getCurrentEnvironmentName,
154152
} from "ee/selectors/environmentSelectors";
155-
import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions";
156153
import { getIsActionCreatedInApp } from "ee/utils/getIsActionCreatedInApp";
157154
import {
158155
endSpan,
@@ -1656,22 +1653,6 @@ function* softRefreshActionsSaga() {
16561653
yield put({ type: ReduxActionTypes.SWITCH_ENVIRONMENT_SUCCESS });
16571654
}
16581655

1659-
function* handleUpdateActionData(
1660-
action: ReduxAction<updateActionDataPayloadType>,
1661-
) {
1662-
const { actionDataPayload, parentSpan } = action.payload;
1663-
1664-
yield call(
1665-
evalWorker.request,
1666-
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
1667-
actionDataPayload,
1668-
);
1669-
1670-
if (parentSpan) {
1671-
endSpan(parentSpan);
1672-
}
1673-
}
1674-
16751656
export function* watchPluginActionExecutionSagas() {
16761657
yield all([
16771658
takeLatest(ReduxActionTypes.RUN_ACTION_REQUEST, runActionSaga),
@@ -1685,6 +1666,5 @@ export function* watchPluginActionExecutionSagas() {
16851666
),
16861667
takeLatest(ReduxActionTypes.PLUGIN_SOFT_REFRESH, softRefreshActionsSaga),
16871668
takeEvery(ReduxActionTypes.EXECUTE_JS_UPDATES, makeUpdateJSCollection),
1688-
takeEvery(ReduxActionTypes.UPDATE_ACTION_DATA, handleUpdateActionData),
16891669
]);
16901670
}

‎app/client/src/sagas/EvaluationsSaga.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
getCurrentApplicationId,
2727
getCurrentPageId,
2828
} from "selectors/editorSelectors";
29+
import { updateActionData } from "actions/pluginActionActions";
2930

3031
jest.mock("loglevel");
3132

@@ -190,6 +191,9 @@ describe("evalQueueBuffer", () => {
190191
const bufferedAction = buffer.take();
191192

192193
expect(bufferedAction).toEqual({
194+
actionDataPayloadConsolidated: [],
195+
hasBufferedAction: true,
196+
hasDebouncedHandleUpdate: false,
193197
type: ReduxActionTypes.BUFFERED_ACTION,
194198
affectedJSObjects: defaultAffectedJSObjects,
195199
postEvalActions: [],
@@ -207,6 +211,9 @@ describe("evalQueueBuffer", () => {
207211
const bufferedAction = buffer.take();
208212

209213
expect(bufferedAction).toEqual({
214+
actionDataPayloadConsolidated: [],
215+
hasBufferedAction: true,
216+
hasDebouncedHandleUpdate: false,
210217
type: ReduxActionTypes.BUFFERED_ACTION,
211218
affectedJSObjects: { ids: ["1", "2"], isAllAffected: false },
212219
postEvalActions: [],
@@ -228,6 +235,9 @@ describe("evalQueueBuffer", () => {
228235
const bufferedAction = buffer.take();
229236

230237
expect(bufferedAction).toEqual({
238+
actionDataPayloadConsolidated: [],
239+
hasBufferedAction: true,
240+
hasDebouncedHandleUpdate: false,
231241
type: ReduxActionTypes.BUFFERED_ACTION,
232242
affectedJSObjects: { ids: [], isAllAffected: true },
233243
postEvalActions: [],
@@ -243,6 +253,9 @@ describe("evalQueueBuffer", () => {
243253
const bufferedAction = buffer.take();
244254

245255
expect(bufferedAction).toEqual({
256+
actionDataPayloadConsolidated: [],
257+
hasBufferedAction: true,
258+
hasDebouncedHandleUpdate: false,
246259
type: ReduxActionTypes.BUFFERED_ACTION,
247260
affectedJSObjects: { ids: ["1"], isAllAffected: false },
248261
postEvalActions: [],
@@ -255,9 +268,120 @@ describe("evalQueueBuffer", () => {
255268
const bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
256269

257270
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
271+
actionDataPayloadConsolidated: [],
272+
hasBufferedAction: true,
273+
hasDebouncedHandleUpdate: false,
258274
type: ReduxActionTypes.BUFFERED_ACTION,
259275
affectedJSObjects: defaultAffectedJSObjects,
260276
postEvalActions: [],
261277
});
262278
});
279+
test("should debounce UPDATE_ACTION_DATA actions together when the buffer is busy", () => {
280+
const buffer = evalQueueBuffer();
281+
282+
buffer.put(
283+
updateActionData([
284+
{
285+
entityName: "widget1",
286+
dataPath: "data",
287+
data: { a: 1 },
288+
dataPathRef: "",
289+
},
290+
]),
291+
);
292+
buffer.put(
293+
updateActionData([
294+
{
295+
entityName: "widget2",
296+
dataPath: "data",
297+
data: { a: 2 },
298+
dataPathRef: "",
299+
},
300+
]),
301+
);
302+
const bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
303+
304+
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
305+
actionDataPayloadConsolidated: [
306+
{
307+
data: {
308+
a: 1,
309+
},
310+
dataPath: "data",
311+
dataPathRef: "",
312+
entityName: "widget1",
313+
},
314+
{
315+
data: {
316+
a: 2,
317+
},
318+
dataPath: "data",
319+
dataPathRef: "",
320+
entityName: "widget2",
321+
},
322+
],
323+
324+
hasBufferedAction: false,
325+
hasDebouncedHandleUpdate: true,
326+
type: ReduxActionTypes.BUFFERED_ACTION,
327+
affectedJSObjects: defaultAffectedJSObjects,
328+
postEvalActions: [],
329+
});
330+
});
331+
test("should be able to debounce UPDATE_ACTION_DATA actions and BUFFERED_ACTION together when the buffer is busy", () => {
332+
const buffer = evalQueueBuffer();
333+
334+
buffer.put(
335+
updateActionData([
336+
{
337+
entityName: "widget1",
338+
dataPath: "data",
339+
data: { a: 1 },
340+
dataPathRef: "",
341+
},
342+
]),
343+
);
344+
buffer.put(
345+
updateActionData([
346+
{
347+
entityName: "widget2",
348+
dataPath: "data",
349+
data: { a: 2 },
350+
dataPathRef: "",
351+
},
352+
]),
353+
);
354+
355+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
356+
buffer.put(createJSCollectionSuccess({ id: "1" } as any));
357+
358+
const bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
359+
360+
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
361+
actionDataPayloadConsolidated: [
362+
{
363+
data: {
364+
a: 1,
365+
},
366+
dataPath: "data",
367+
dataPathRef: "",
368+
entityName: "widget1",
369+
},
370+
{
371+
data: {
372+
a: 2,
373+
},
374+
dataPath: "data",
375+
dataPathRef: "",
376+
entityName: "widget2",
377+
},
378+
],
379+
380+
hasBufferedAction: true,
381+
hasDebouncedHandleUpdate: true,
382+
type: ReduxActionTypes.BUFFERED_ACTION,
383+
affectedJSObjects: { ids: ["1"], isAllAffected: false },
384+
postEvalActions: [],
385+
});
386+
});
263387
});

‎app/client/src/sagas/EvaluationsSaga.ts‎

Lines changed: 93 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ import type {
9494
import type { ActionDescription } from "ee/workers/Evaluation/fns";
9595
import { handleEvalWorkerRequestSaga } from "./EvalWorkerActionSagas";
9696
import { getAppsmithConfigs } from "ee/configs";
97-
import { executeJSUpdates } from "actions/pluginActionActions";
97+
import {
98+
executeJSUpdates,
99+
type actionDataPayload,
100+
type updateActionDataPayloadType,
101+
} from "actions/pluginActionActions";
98102
import { setEvaluatedActionSelectorField } from "actions/actionSelectorActions";
99103
import { waitForWidgetConfigBuild } from "./InitSagas";
100104
import { logDynamicTriggerExecution } from "ee/sagas/analyticsSaga";
@@ -536,8 +540,17 @@ export const defaultAffectedJSObjects: AffectedJSObjects = {
536540
ids: [],
537541
};
538542

543+
interface BUFFERED_ACTION {
544+
hasDebouncedHandleUpdate: boolean;
545+
hasBufferedAction: boolean;
546+
actionDataPayloadConsolidated: actionDataPayload[];
547+
}
539548
export function evalQueueBuffer() {
540549
let canTake = false;
550+
let hasDebouncedHandleUpdate = false;
551+
let hasBufferedAction = false;
552+
let actionDataPayloadConsolidated: actionDataPayload = [];
553+
541554
// TODO: Fix this the next time the file is edited
542555
// eslint-disable-next-line @typescript-eslint/no-explicit-any
543556
let collectedPostEvalActions: any = [];
@@ -552,8 +565,19 @@ export function evalQueueBuffer() {
552565

553566
collectedAffectedJSObjects = defaultAffectedJSObjects;
554567
canTake = false;
568+
const actionDataPayloadConsolidatedRes = actionDataPayloadConsolidated;
569+
570+
const hasDebouncedHandleUpdateRes = hasDebouncedHandleUpdate;
571+
const hasBufferedActionRes = hasBufferedAction;
572+
573+
actionDataPayloadConsolidated = [];
574+
hasDebouncedHandleUpdate = false;
575+
hasBufferedAction = false;
555576

556577
return {
578+
hasDebouncedHandleUpdate: hasDebouncedHandleUpdateRes,
579+
hasBufferedAction: hasBufferedActionRes,
580+
actionDataPayloadConsolidated: actionDataPayloadConsolidatedRes,
557581
postEvalActions: resp,
558582
affectedJSObjects,
559583
type: ReduxActionTypes.BUFFERED_ACTION,
@@ -573,6 +597,25 @@ export function evalQueueBuffer() {
573597
return;
574598
}
575599

600+
if (action.type === ReduxActionTypes.UPDATE_ACTION_DATA) {
601+
const { actionDataPayload } =
602+
action.payload as updateActionDataPayloadType;
603+
604+
if (actionDataPayload && actionDataPayload.length) {
605+
actionDataPayloadConsolidated = [
606+
...actionDataPayloadConsolidated,
607+
...actionDataPayload,
608+
];
609+
}
610+
611+
hasDebouncedHandleUpdate = true;
612+
canTake = true;
613+
614+
return;
615+
}
616+
617+
hasBufferedAction = true;
618+
576619
canTake = true;
577620
// extract the affected JS action ids from the action and pass them
578621
// as a part of the buffered action
@@ -758,16 +801,56 @@ function* evaluationChangeListenerSaga(): any {
758801
const action: EvaluationReduxAction<unknown | unknown[]> =
759802
yield take(evtActionChannel);
760803

761-
// We are dequing actions from the buffer and inferring the JS actions affected by each
762-
// action. Through this we know ahead the nodes we need to specifically diff, thereby improving performance.
763-
const affectedJSObjects = getAffectedJSObjectIdsFromAction(action);
804+
const { payload, type } = action;
764805

765-
yield call(evalAndLintingHandler, true, action, {
766-
shouldReplay: get(action, "payload.shouldReplay"),
767-
forceEvaluation: shouldForceEval(action),
768-
requiresLogging: shouldLog(action),
769-
affectedJSObjects,
770-
});
806+
if (type === ReduxActionTypes.UPDATE_ACTION_DATA) {
807+
yield call(
808+
evalWorker.request,
809+
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
810+
(payload as updateActionDataPayloadType).actionDataPayload,
811+
);
812+
continue;
813+
}
814+
815+
if (type !== ReduxActionTypes.BUFFERED_ACTION) {
816+
const affectedJSObjects = getAffectedJSObjectIdsFromAction(action);
817+
818+
yield call(evalAndLintingHandler, true, action, {
819+
shouldReplay: get(action, "payload.shouldReplay"),
820+
forceEvaluation: shouldForceEval(action),
821+
requiresLogging: shouldLog(action),
822+
affectedJSObjects,
823+
});
824+
continue;
825+
}
826+
827+
// all buffered debounced actions are handled here
828+
const {
829+
actionDataPayloadConsolidated,
830+
hasBufferedAction,
831+
hasDebouncedHandleUpdate,
832+
} = action as unknown as BUFFERED_ACTION;
833+
834+
if (hasDebouncedHandleUpdate) {
835+
yield call(
836+
evalWorker.request,
837+
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
838+
actionDataPayloadConsolidated,
839+
);
840+
}
841+
842+
if (hasBufferedAction) {
843+
// We are dequing actions from the buffer and inferring the JS actions affected by each
844+
// action. Through this we know ahead the nodes we need to specifically diff, thereby improving performance.
845+
const affectedJSObjects = getAffectedJSObjectIdsFromAction(action);
846+
847+
yield call(evalAndLintingHandler, true, action, {
848+
shouldReplay: get(action, "payload.shouldReplay"),
849+
forceEvaluation: shouldForceEval(action),
850+
requiresLogging: shouldLog(action),
851+
affectedJSObjects,
852+
});
853+
}
771854
}
772855
}
773856

0 commit comments

Comments
 (0)