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: merge updateActionData with an evaluation #38548

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 3 additions & 0 deletions app/client/src/sagas/EvaluationsSaga.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ describe("evaluateTreeSaga", () => {
widgetsMeta: {},
shouldRespondWithLogs: true,
affectedJSObjects: { ids: [], isAllAffected: false },
actionDataPayloadConsolidated: undefined,
})
.run();
});
Expand Down Expand Up @@ -121,6 +122,7 @@ describe("evaluateTreeSaga", () => {
widgetsMeta: {},
shouldRespondWithLogs: false,
affectedJSObjects: { ids: [], isAllAffected: false },
actionDataPayloadConsolidated: undefined,
})
.run();
});
Expand Down Expand Up @@ -175,6 +177,7 @@ describe("evaluateTreeSaga", () => {
widgetsMeta: {},
shouldRespondWithLogs: false,
affectedJSObjects,
actionDataPayloadConsolidated: undefined,
})
.run();
});
Expand Down
32 changes: 29 additions & 3 deletions app/client/src/sagas/EvaluationsSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ export function* evaluateTreeSaga(
forceEvaluation = false,
requiresLogging = false,
affectedJSObjects: AffectedJSObjects = defaultAffectedJSObjects,
actionDataPayloadConsolidated?: actionDataPayload,
) {
const allActionValidationConfig: ReturnType<
typeof getAllActionValidationConfig
Expand Down Expand Up @@ -291,6 +292,7 @@ export function* evaluateTreeSaga(
widgetsMeta,
shouldRespondWithLogs,
affectedJSObjects,
actionDataPayloadConsolidated,
};

const workerResponse: EvalTreeResponseData = yield call(
Expand Down Expand Up @@ -545,7 +547,7 @@ export const defaultAffectedJSObjects: AffectedJSObjects = {
interface BUFFERED_ACTION {
hasDebouncedHandleUpdate: boolean;
hasBufferedAction: boolean;
actionDataPayloadConsolidated: actionDataPayload[];
actionDataPayloadConsolidated: actionDataPayload;
}

export function evalQueueBuffer() {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -728,6 +736,7 @@ function* evalAndLintingHandler(
forceEvaluation,
requiresLogging,
affectedJSObjects,
actionDataPayloadConsolidated,
),
);
}
Expand Down Expand Up @@ -834,6 +843,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,
Expand Down
7 changes: 7 additions & 0 deletions app/client/src/workers/Evaluation/handlers/evalTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -70,6 +71,7 @@ export async function evalTree(
let isNewWidgetAdded = false;

const {
actionDataPayloadConsolidated,
affectedJSObjects,
allActionValidationConfig,
appMode,
Expand Down Expand Up @@ -190,6 +192,10 @@ export async function evalTree(
});
staleMetaIds = dataTreeResponse.staleMetaIds;
} else {
const tree = dataTreeEvaluator.getEvalTree();

updateActionsToEvalTree(tree, actionDataPayloadConsolidated);

if (dataTreeEvaluator && !isEmpty(allActionValidationConfig)) {
dataTreeEvaluator.setAllActionValidationConfig(
allActionValidationConfig,
Expand All @@ -212,6 +218,7 @@ export async function evalTree(
configTree,
webworkerTelemetry,
affectedJSObjects,
actionDataPayloadConsolidated,
),
);

Expand Down
38 changes: 24 additions & 14 deletions app/client/src/workers/Evaluation/handlers/updateActionData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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";

export interface UpdateActionProps {
entityName: string;
Expand All @@ -26,6 +27,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;
Expand All @@ -44,18 +68,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: {},
});
}
2 changes: 2 additions & 0 deletions app/client/src/workers/Evaluation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { actionDataPayload } from "actions/pluginActionActions";

// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -51,6 +52,7 @@ export interface EvalTreeRequestData {
widgetsMeta: Record<string, any>;
shouldRespondWithLogs?: boolean;
affectedJSObjects: AffectedJSObjects;
actionDataPayloadConsolidated?: actionDataPayload;
}

export interface EvalTreeResponseData {
Expand Down
19 changes: 17 additions & 2 deletions app/client/src/workers/common/DataTreeEvaluator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/updateActionData";

type SortedDependencies = Array<string>;
export interface EvalProps {
Expand Down Expand Up @@ -637,6 +638,7 @@ export default class DataTreeEvaluator {
configTree: ConfigTree,
webworkerTelemetry: Record<string, WebworkerSpanData | Attributes> = {},
affectedJSObjects: AffectedJSObjects = { isAllAffected: false, ids: [] },
actionDataPayloadConsolidated?: UpdateActionProps[],
): {
unEvalUpdates: DataTreeDiff[];
evalOrder: string[];
Expand Down Expand Up @@ -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: [],
Expand Down Expand Up @@ -724,7 +726,12 @@ export default class DataTreeEvaluator {
this.dependencies = dependencies;
this.inverseDependencies = inverseDependencies;

const pathsChangedSet = new Set<string[]>();
const pathsChangedSet = new Set<string[]>(
actionDataPayloadConsolidated?.map(({ dataPath, entityName }) => [
entityName,
dataPath,
]) || [],
);

for (const diff of differences) {
if (isArray(diff.path)) {
Expand All @@ -741,10 +748,18 @@ export default class DataTreeEvaluator {
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,
});
},
);
Expand Down
Loading