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: 834a437d377baf45cc9c187eedaff261b7de6155 > <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 -->
This commit is contained in:
parent
2246bde9bb
commit
f9664a3b7c
|
|
@ -388,7 +388,7 @@ export const bindDataOnCanvas = (payload: {
|
|||
};
|
||||
};
|
||||
|
||||
type actionDataPayload = {
|
||||
export type actionDataPayload = {
|
||||
entityName: string;
|
||||
dataPath: string;
|
||||
data: unknown;
|
||||
|
|
|
|||
|
|
@ -108,6 +108,7 @@ export const EVALUATE_REDUX_ACTIONS = [
|
|||
ReduxActionTypes.BUFFERED_ACTION,
|
||||
// Generic
|
||||
ReduxActionTypes.TRIGGER_EVAL,
|
||||
ReduxActionTypes.UPDATE_ACTION_DATA,
|
||||
];
|
||||
// Topics used for datasource and query form evaluations
|
||||
export const FORM_EVALUATION_REDUX_ACTIONS = [
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ import {
|
|||
takeLatest,
|
||||
} from "redux-saga/effects";
|
||||
import * as Sentry from "@sentry/react";
|
||||
import type { updateActionDataPayloadType } from "actions/pluginActionActions";
|
||||
import {
|
||||
clearActionResponse,
|
||||
executePageLoadActions,
|
||||
|
|
@ -104,7 +103,6 @@ import { EMPTY_RESPONSE } from "components/editorComponents/emptyResponse";
|
|||
import type { AppState } from "ee/reducers";
|
||||
import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "ee/constants/ApiConstants";
|
||||
import { evaluateActionBindings } from "sagas/EvaluationsSaga";
|
||||
import { evalWorker } from "utils/workerInstances";
|
||||
import { isBlobUrl, parseBlobUrl } from "utils/AppsmithUtils";
|
||||
import { getType, Types } from "utils/TypeHelpers";
|
||||
import { matchPath } from "react-router";
|
||||
|
|
@ -152,7 +150,6 @@ import {
|
|||
getCurrentEnvironmentDetails,
|
||||
getCurrentEnvironmentName,
|
||||
} from "ee/selectors/environmentSelectors";
|
||||
import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions";
|
||||
import { getIsActionCreatedInApp } from "ee/utils/getIsActionCreatedInApp";
|
||||
import {
|
||||
endSpan,
|
||||
|
|
@ -1656,22 +1653,6 @@ function* softRefreshActionsSaga() {
|
|||
yield put({ type: ReduxActionTypes.SWITCH_ENVIRONMENT_SUCCESS });
|
||||
}
|
||||
|
||||
function* handleUpdateActionData(
|
||||
action: ReduxAction<updateActionDataPayloadType>,
|
||||
) {
|
||||
const { actionDataPayload, parentSpan } = action.payload;
|
||||
|
||||
yield call(
|
||||
evalWorker.request,
|
||||
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
|
||||
actionDataPayload,
|
||||
);
|
||||
|
||||
if (parentSpan) {
|
||||
endSpan(parentSpan);
|
||||
}
|
||||
}
|
||||
|
||||
export function* watchPluginActionExecutionSagas() {
|
||||
yield all([
|
||||
takeLatest(ReduxActionTypes.RUN_ACTION_REQUEST, runActionSaga),
|
||||
|
|
@ -1685,6 +1666,5 @@ export function* watchPluginActionExecutionSagas() {
|
|||
),
|
||||
takeLatest(ReduxActionTypes.PLUGIN_SOFT_REFRESH, softRefreshActionsSaga),
|
||||
takeEvery(ReduxActionTypes.EXECUTE_JS_UPDATES, makeUpdateJSCollection),
|
||||
takeEvery(ReduxActionTypes.UPDATE_ACTION_DATA, handleUpdateActionData),
|
||||
]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import {
|
|||
getCurrentApplicationId,
|
||||
getCurrentPageId,
|
||||
} from "selectors/editorSelectors";
|
||||
import { updateActionData } from "actions/pluginActionActions";
|
||||
|
||||
jest.mock("loglevel");
|
||||
|
||||
|
|
@ -190,6 +191,9 @@ describe("evalQueueBuffer", () => {
|
|||
const bufferedAction = buffer.take();
|
||||
|
||||
expect(bufferedAction).toEqual({
|
||||
actionDataPayloadConsolidated: [],
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: false,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: defaultAffectedJSObjects,
|
||||
postEvalActions: [],
|
||||
|
|
@ -207,6 +211,9 @@ describe("evalQueueBuffer", () => {
|
|||
const bufferedAction = buffer.take();
|
||||
|
||||
expect(bufferedAction).toEqual({
|
||||
actionDataPayloadConsolidated: [],
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: false,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: { ids: ["1", "2"], isAllAffected: false },
|
||||
postEvalActions: [],
|
||||
|
|
@ -228,6 +235,9 @@ describe("evalQueueBuffer", () => {
|
|||
const bufferedAction = buffer.take();
|
||||
|
||||
expect(bufferedAction).toEqual({
|
||||
actionDataPayloadConsolidated: [],
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: false,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: { ids: [], isAllAffected: true },
|
||||
postEvalActions: [],
|
||||
|
|
@ -243,6 +253,9 @@ describe("evalQueueBuffer", () => {
|
|||
const bufferedAction = buffer.take();
|
||||
|
||||
expect(bufferedAction).toEqual({
|
||||
actionDataPayloadConsolidated: [],
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: false,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: { ids: ["1"], isAllAffected: false },
|
||||
postEvalActions: [],
|
||||
|
|
@ -255,9 +268,120 @@ describe("evalQueueBuffer", () => {
|
|||
const bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
|
||||
|
||||
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
|
||||
actionDataPayloadConsolidated: [],
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: false,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: defaultAffectedJSObjects,
|
||||
postEvalActions: [],
|
||||
});
|
||||
});
|
||||
test("should debounce UPDATE_ACTION_DATA actions together when the buffer is busy", () => {
|
||||
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 bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
|
||||
|
||||
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
|
||||
actionDataPayloadConsolidated: [
|
||||
{
|
||||
data: {
|
||||
a: 1,
|
||||
},
|
||||
dataPath: "data",
|
||||
dataPathRef: "",
|
||||
entityName: "widget1",
|
||||
},
|
||||
{
|
||||
data: {
|
||||
a: 2,
|
||||
},
|
||||
dataPath: "data",
|
||||
dataPathRef: "",
|
||||
entityName: "widget2",
|
||||
},
|
||||
],
|
||||
|
||||
hasBufferedAction: false,
|
||||
hasDebouncedHandleUpdate: true,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: defaultAffectedJSObjects,
|
||||
postEvalActions: [],
|
||||
});
|
||||
});
|
||||
test("should be able to debounce UPDATE_ACTION_DATA actions and BUFFERED_ACTION together when the buffer is busy", () => {
|
||||
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 bufferedActionsWithDefaultAffectedJSObjects = buffer.take();
|
||||
|
||||
expect(bufferedActionsWithDefaultAffectedJSObjects).toEqual({
|
||||
actionDataPayloadConsolidated: [
|
||||
{
|
||||
data: {
|
||||
a: 1,
|
||||
},
|
||||
dataPath: "data",
|
||||
dataPathRef: "",
|
||||
entityName: "widget1",
|
||||
},
|
||||
{
|
||||
data: {
|
||||
a: 2,
|
||||
},
|
||||
dataPath: "data",
|
||||
dataPathRef: "",
|
||||
entityName: "widget2",
|
||||
},
|
||||
],
|
||||
|
||||
hasBufferedAction: true,
|
||||
hasDebouncedHandleUpdate: true,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
affectedJSObjects: { ids: ["1"], isAllAffected: false },
|
||||
postEvalActions: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -94,7 +94,11 @@ import type {
|
|||
import type { ActionDescription } from "ee/workers/Evaluation/fns";
|
||||
import { handleEvalWorkerRequestSaga } from "./EvalWorkerActionSagas";
|
||||
import { getAppsmithConfigs } from "ee/configs";
|
||||
import { executeJSUpdates } from "actions/pluginActionActions";
|
||||
import {
|
||||
executeJSUpdates,
|
||||
type actionDataPayload,
|
||||
type updateActionDataPayloadType,
|
||||
} from "actions/pluginActionActions";
|
||||
import { setEvaluatedActionSelectorField } from "actions/actionSelectorActions";
|
||||
import { waitForWidgetConfigBuild } from "./InitSagas";
|
||||
import { logDynamicTriggerExecution } from "ee/sagas/analyticsSaga";
|
||||
|
|
@ -536,8 +540,17 @@ export const defaultAffectedJSObjects: AffectedJSObjects = {
|
|||
ids: [],
|
||||
};
|
||||
|
||||
interface BUFFERED_ACTION {
|
||||
hasDebouncedHandleUpdate: boolean;
|
||||
hasBufferedAction: boolean;
|
||||
actionDataPayloadConsolidated: actionDataPayload[];
|
||||
}
|
||||
export function evalQueueBuffer() {
|
||||
let canTake = false;
|
||||
let hasDebouncedHandleUpdate = false;
|
||||
let hasBufferedAction = false;
|
||||
let actionDataPayloadConsolidated: actionDataPayload = [];
|
||||
|
||||
// TODO: Fix this the next time the file is edited
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
let collectedPostEvalActions: any = [];
|
||||
|
|
@ -552,8 +565,19 @@ export function evalQueueBuffer() {
|
|||
|
||||
collectedAffectedJSObjects = defaultAffectedJSObjects;
|
||||
canTake = false;
|
||||
const actionDataPayloadConsolidatedRes = actionDataPayloadConsolidated;
|
||||
|
||||
const hasDebouncedHandleUpdateRes = hasDebouncedHandleUpdate;
|
||||
const hasBufferedActionRes = hasBufferedAction;
|
||||
|
||||
actionDataPayloadConsolidated = [];
|
||||
hasDebouncedHandleUpdate = false;
|
||||
hasBufferedAction = false;
|
||||
|
||||
return {
|
||||
hasDebouncedHandleUpdate: hasDebouncedHandleUpdateRes,
|
||||
hasBufferedAction: hasBufferedActionRes,
|
||||
actionDataPayloadConsolidated: actionDataPayloadConsolidatedRes,
|
||||
postEvalActions: resp,
|
||||
affectedJSObjects,
|
||||
type: ReduxActionTypes.BUFFERED_ACTION,
|
||||
|
|
@ -573,6 +597,25 @@ export function evalQueueBuffer() {
|
|||
return;
|
||||
}
|
||||
|
||||
if (action.type === ReduxActionTypes.UPDATE_ACTION_DATA) {
|
||||
const { actionDataPayload } =
|
||||
action.payload as updateActionDataPayloadType;
|
||||
|
||||
if (actionDataPayload && actionDataPayload.length) {
|
||||
actionDataPayloadConsolidated = [
|
||||
...actionDataPayloadConsolidated,
|
||||
...actionDataPayload,
|
||||
];
|
||||
}
|
||||
|
||||
hasDebouncedHandleUpdate = true;
|
||||
canTake = true;
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
hasBufferedAction = true;
|
||||
|
||||
canTake = true;
|
||||
// extract the affected JS action ids from the action and pass them
|
||||
// as a part of the buffered action
|
||||
|
|
@ -758,6 +801,45 @@ function* evaluationChangeListenerSaga(): any {
|
|||
const action: EvaluationReduxAction<unknown | unknown[]> =
|
||||
yield take(evtActionChannel);
|
||||
|
||||
const { payload, type } = action;
|
||||
|
||||
if (type === ReduxActionTypes.UPDATE_ACTION_DATA) {
|
||||
yield call(
|
||||
evalWorker.request,
|
||||
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
|
||||
(payload as updateActionDataPayloadType).actionDataPayload,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (type !== ReduxActionTypes.BUFFERED_ACTION) {
|
||||
const affectedJSObjects = getAffectedJSObjectIdsFromAction(action);
|
||||
|
||||
yield call(evalAndLintingHandler, true, action, {
|
||||
shouldReplay: get(action, "payload.shouldReplay"),
|
||||
forceEvaluation: shouldForceEval(action),
|
||||
requiresLogging: shouldLog(action),
|
||||
affectedJSObjects,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// all buffered debounced actions are handled here
|
||||
const {
|
||||
actionDataPayloadConsolidated,
|
||||
hasBufferedAction,
|
||||
hasDebouncedHandleUpdate,
|
||||
} = action as unknown as BUFFERED_ACTION;
|
||||
|
||||
if (hasDebouncedHandleUpdate) {
|
||||
yield call(
|
||||
evalWorker.request,
|
||||
EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA,
|
||||
actionDataPayloadConsolidated,
|
||||
);
|
||||
}
|
||||
|
||||
if (hasBufferedAction) {
|
||||
// We are dequing actions from the buffer and inferring the JS actions affected by each
|
||||
// action. Through this we know ahead the nodes we need to specifically diff, thereby improving performance.
|
||||
const affectedJSObjects = getAffectedJSObjectIdsFromAction(action);
|
||||
|
|
@ -770,6 +852,7 @@ function* evaluationChangeListenerSaga(): any {
|
|||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Fix this the next time the file is edited
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user