Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: debounce handle action updates #38396

Merged
merged 1 commit into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/src/actions/pluginActionActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ export const bindDataOnCanvas = (payload: {
};
};

type actionDataPayload = {
export type actionDataPayload = {
entityName: string;
dataPath: string;
data: unknown;
Expand Down
1 change: 1 addition & 0 deletions app/client/src/ce/actions/evaluationActionsList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
20 changes: 0 additions & 20 deletions app/client/src/sagas/ActionExecution/PluginActionSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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),
]);
}
124 changes: 124 additions & 0 deletions app/client/src/sagas/EvaluationsSaga.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
getCurrentApplicationId,
getCurrentPageId,
} from "selectors/editorSelectors";
import { updateActionData } from "actions/pluginActionActions";

jest.mock("loglevel");

Expand Down Expand Up @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
});
});
});
103 changes: 93 additions & 10 deletions app/client/src/sagas/EvaluationsSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 = [];
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -758,16 +801,56 @@ function* evaluationChangeListenerSaga(): any {
const action: EvaluationReduxAction<unknown | unknown[]> =
yield take(evtActionChannel);

// 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);
const { payload, type } = action;

yield call(evalAndLintingHandler, true, action, {
shouldReplay: get(action, "payload.shouldReplay"),
forceEvaluation: shouldForceEval(action),
requiresLogging: shouldLog(action),
affectedJSObjects,
});
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);

yield call(evalAndLintingHandler, true, action, {
shouldReplay: get(action, "payload.shouldReplay"),
forceEvaluation: shouldForceEval(action),
requiresLogging: shouldLog(action),
affectedJSObjects,
});
}
}
}

Expand Down
Loading