feat: Remove Action/Query/JS data from unevalTree (#27056)

## Description

This PR reduces the size of the unevalTree by removing action/query/js
function data from it. This improves the performance of Apps by

1. Reducing the overall time for generating dataTree diffs
2. Decreasing the time taken to generate allKeys 
3. Reducing the number of nodes in the dependency graph thereby
improving dependency graph operations like

    -    Sorting dependencies
    -    Adding nodes to the dep graph



### Performance

Release

<img width="294" alt="Screenshot 2023-09-27 at 20 22 31"
src="https://github.com/appsmithorg/appsmith/assets/46670083/df4667e5-33c3-44c6-bfd4-a170edaa43b8">


DP

<img width="304" alt="Screenshot 2023-09-27 at 20 24 16"
src="https://github.com/appsmithorg/appsmith/assets/46670083/598d4a2d-9a32-4bcf-81e7-25f178f779d5">


37.8% improvement in worker scripting time for fairly large App.



#### PR fixes following issue(s)

Fixes #23570


#### Type of change

- New feature (non-breaking change which adds functionality)



#### How Has This Been Tested?
> Please describe the tests that you ran to verify your changes. Also
list any relevant details for your test configuration.
> Delete anything that is not relevant
- [ ] Manual
- [ ] JUnit
- [ ] Jest
- [ ] Cypress
>
>
#### Test Plan
1. Validating the Crude app/ api query and JS object
2. Validating the chart/table/Select/Tree select for Query and API
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Favour Ohanekwu 2023-09-27 23:03:38 +01:00 committed by GitHub
parent df31f8def2
commit e37d3b8dba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 257 additions and 19 deletions

View File

@ -340,6 +340,25 @@ export const bindDataOnCanvas = (payload: {
};
};
export const updateActionData = ({
data,
dataPath,
entityName,
}: {
entityName: string;
dataPath: string;
data: unknown;
}) => {
return {
type: ReduxActionTypes.UPDATE_ACTION_DATA,
payload: {
entityName,
dataPath,
data,
},
};
};
export default {
createAction: createActionRequest,
fetchActions,

View File

@ -79,7 +79,6 @@ export const EVALUATE_REDUX_ACTIONS = [
ReduxActionTypes.FETCH_JS_ACTIONS_VIEW_MODE_SUCCESS,
ReduxActionErrorTypes.FETCH_JS_ACTIONS_VIEW_MODE_ERROR,
ReduxActionTypes.UPDATE_JS_ACTION_BODY_SUCCESS,
ReduxActionTypes.SET_JS_FUNCTION_EXECUTION_DATA,
// App Data
ReduxActionTypes.SET_APP_MODE,
ReduxActionTypes.FETCH_USER_DETAILS_SUCCESS,

View File

@ -867,6 +867,7 @@ const ActionTypes = {
DELETE_MULTIPLE_APPLICATION_SUCCESS: "DELETE_MULTIPLE_APPLICATION_SUCCESS",
DELETE_MULTIPLE_APPLICATION_CANCEL: "DELETE_MULTIPLE_APPLICATION_CANCEL",
TRIGGER_EVAL: "TRIGGER_EVAL",
UPDATE_ACTION_DATA: "UPDATE_ACTION_DATA",
};
export const ReduxActionTypes = {

View File

@ -7,7 +7,14 @@ import type {
} from "constants/AppsmithActionConstants/ActionConstants";
import { TriggerKind } from "constants/AppsmithActionConstants/ActionConstants";
import * as log from "loglevel";
import { all, call, put, takeEvery, takeLatest } from "redux-saga/effects";
import {
all,
call,
put,
takeEvery,
takeLatest,
select,
} from "redux-saga/effects";
import {
evaluateActionSelectorFieldSaga,
evaluateAndExecuteDynamicTrigger,
@ -19,7 +26,10 @@ import copySaga from "sagas/ActionExecution/CopyActionSaga";
import resetWidgetActionSaga from "sagas/ActionExecution/ResetWidgetActionSaga";
import showAlertSaga from "sagas/ActionExecution/ShowAlertActionSaga";
import executePluginActionTriggerSaga from "sagas/ActionExecution/PluginActionSaga";
import { clearActionResponse } from "actions/pluginActionActions";
import {
clearActionResponse,
updateActionData,
} from "actions/pluginActionActions";
import {
closeModalSaga,
openModalSaga,
@ -32,6 +42,8 @@ import {
} from "sagas/ActionExecution/geolocationSaga";
import { postMessageSaga } from "sagas/ActionExecution/PostMessageSaga";
import type { ActionDescription } from "@appsmith/workers/Evaluation/fns";
import { getActionById } from "selectors/editorSelectors";
import type { AppState } from "@appsmith/reducers";
export type TriggerMeta = {
source?: TriggerSource;
@ -58,6 +70,25 @@ export function* executeActionTriggers(
break;
case "CLEAR_PLUGIN_ACTION":
yield put(clearActionResponse(trigger.payload.actionId));
const action: ReturnType<typeof getActionById> = yield select(
(state: AppState) =>
getActionById(state, {
match: {
params: {
apiId: trigger.payload.actionId,
},
},
}),
);
if (action) {
yield put(
updateActionData({
entityName: action.name,
dataPath: "data",
data: undefined,
}),
);
}
break;
case "NAVIGATE_TO":
yield call(navigateActionSaga, trigger);

View File

@ -11,6 +11,7 @@ export enum EVAL_WORKER_SYNC_ACTION {
INIT_FORM_EVAL = "INIT_FORM_EVAL",
UNINSTALL_LIBRARY = "UNINSTALL_LIBRARY",
LINT_TREE = "LINT_TREE",
UPDATE_ACTION_DATA = "UPDATE_ACTION_DATA",
}
export enum EVAL_WORKER_ASYNC_ACTION {

View File

@ -964,7 +964,7 @@ export function convertJSFunctionsToString(
for (const funcName in jsFunctions) {
if (jsCollection[funcName] instanceof String) {
if (has(jsCollection, [funcName, "data"])) {
set(jsCollection, [`${funcName}.data`], jsCollection[funcName].data);
set(jsCollection, [`${funcName}.data`], {});
}
set(jsCollection, funcName, jsCollection[funcName].toString());
}

View File

@ -54,7 +54,9 @@ export const generateDataTreeAction = (
actionId: action.config.id,
run: {},
clear: {},
data: action.data ? action.data.body : undefined,
// Data is always set to undefined in the unevalTree
// Action data is updated directly to the dataTree (see updateActionData.ts)
data: undefined,
isLoading: action.isLoading,
responseMeta: {
statusCode: action.data?.statusCode,

View File

@ -124,11 +124,7 @@ describe("generateDataTreeJSAction", () => {
},
],
},
data: {
abcd: {
users: [{ id: 1, name: "John" }],
},
},
data: {},
};
const expectedData = {
myVar1: [],
@ -137,9 +133,7 @@ describe("generateDataTreeJSAction", () => {
body: "export default {\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t},\n\tmyFun2: async () => {\n\t\t//use async-await or promises\n\t}\n}",
myFun2: {
data: {
users: [{ id: 1, name: "John" }],
},
data: {},
},
myFun1: {
data: {},
@ -336,9 +330,7 @@ describe("generateDataTreeJSAction", () => {
body: "export default {\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t return JSObject2.myFun2},\n\tmyFun2: async () => {\n\t\t//use async-await or promises\n\t}\n}",
ENTITY_TYPE: "JSACTION",
myFun2: {
data: {
users: [{ id: 1, name: "John" }],
},
data: {},
},
myFun1: {
data: {},

View File

@ -42,7 +42,9 @@ export const generateDataTreeJSAction = (js: JSCollectionData): any => {
dynamicBindingPathList.push({ key: action.name });
dependencyMap["body"].push(action.name);
actionsData[action.name] = {
data: (js.data && js.data[`${action.id}`]) || {},
// Data is always set to {} in the unevalTree
// Action data is updated directly to the dataTree (see updateActionData.ts)
data: {},
};
}
}

View File

@ -15,6 +15,7 @@ import {
executePluginActionSuccess,
runAction,
updateAction,
updateActionData,
} from "actions/pluginActionActions";
import { makeUpdateJSCollection } from "sagas/JSPaneSagas";
@ -101,7 +102,7 @@ import * as log from "loglevel";
import { EMPTY_RESPONSE } from "components/editorComponents/emptyResponse";
import type { AppState } from "@appsmith/reducers";
import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "@appsmith/constants/ApiConstants";
import { evaluateActionBindings } from "sagas/EvaluationsSaga";
import { evalWorker, evaluateActionBindings } from "sagas/EvaluationsSaga";
import { isBlobUrl, parseBlobUrl } from "utils/AppsmithUtils";
import { getType, Types } from "utils/TypeHelpers";
import { matchPath } from "react-router";
@ -158,6 +159,7 @@ import {
getCurrentEnvironmentDetails,
getCurrentEnvironmentName,
} from "@appsmith/selectors/environmentSelectors";
import { EVAL_WORKER_ACTIONS } from "@appsmith/workers/Evaluation/evalWorkerActions";
enum ActionResponseDataTypes {
BINARY = "BINARY",
@ -1171,6 +1173,13 @@ function* executePageLoadAction(pageAction: PageAction) {
data: payload,
}),
);
yield put(
updateActionData({
entityName: action.name,
dataPath: "data",
data: payload.body,
}),
);
PerformanceTracker.stopAsyncTracking(
PerformanceTransactionName.EXECUTE_ACTION,
{
@ -1226,6 +1235,13 @@ function* executePageLoadAction(pageAction: PageAction) {
isPageLoad: true,
}),
);
yield put(
updateActionData({
entityName: action.name,
dataPath: "data",
data: payload.body,
}),
);
yield take(ReduxActionTypes.SET_EVALUATED_TREE);
}
}
@ -1379,6 +1395,14 @@ function* executePluginActionSaga(
response: payload,
}),
);
yield put(
updateActionData({
entityName: pluginAction.name,
dataPath: "data",
data: payload.body,
}),
);
// TODO: Plugins are not always fetched before on page load actions are executed.
try {
let plugin: Plugin | undefined;
@ -1432,6 +1456,13 @@ function* executePluginActionSaga(
response: EMPTY_RESPONSE,
}),
);
yield put(
updateActionData({
entityName: pluginAction.name,
dataPath: "data",
data: EMPTY_RESPONSE.body,
}),
);
if (e instanceof UserCancelledActionExecutionError) {
// Case: user cancelled the request of file upload
if (filePickerInstrumentation.numberOfFiles > 0) {
@ -1500,6 +1531,13 @@ function* clearTriggerActionResponse() {
// Clear the action response if the action has data and is not executeOnLoad.
if (action.data && !action.config.executeOnLoad) {
yield put(clearActionResponse(action.config.id));
yield put(
updateActionData({
entityName: action.config.name,
dataPath: "data",
data: undefined,
}),
);
}
}
}
@ -1542,6 +1580,23 @@ function* softRefreshActionsSaga() {
});
}
function* handleUpdateActionData(
action: ReduxAction<{
entityName: string;
dataPath: string;
data: unknown;
}>,
) {
const { data, dataPath, entityName } = action.payload;
yield call(evalWorker.request, EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA, [
{
entityName,
dataPath,
data,
},
]);
}
export function* watchPluginActionExecutionSagas() {
yield all([
takeLatest(ReduxActionTypes.RUN_ACTION_REQUEST, runActionSaga),
@ -1555,5 +1610,6 @@ export function* watchPluginActionExecutionSagas() {
),
takeLatest(ReduxActionTypes.PLUGIN_SOFT_REFRESH, softRefreshActionsSaga),
takeEvery(ReduxActionTypes.EXECUTE_JS_UPDATES, makeUpdateJSCollection),
takeEvery(ReduxActionTypes.UPDATE_ACTION_DATA, handleUpdateActionData),
]);
}

View File

@ -57,7 +57,10 @@ import { RenderModes } from "constants/WidgetConstants";
import log from "loglevel";
import { getDataTree } from "selectors/dataTreeSelectors";
import { getWidgets } from "./selectors";
import { clearActionResponse } from "actions/pluginActionActions";
import {
clearActionResponse,
updateActionData,
} from "actions/pluginActionActions";
import {
importApplication,
updateApplicationLayout,
@ -216,6 +219,13 @@ function* setUpTourAppSaga() {
// Update getCustomers query body
const query: ActionData | undefined = yield select(getQueryAction);
yield put(clearActionResponse(query?.config.id ?? ""));
yield put(
updateActionData({
entityName: query?.config.name || "",
dataPath: "data",
data: undefined,
}),
);
const applicationId: string = yield select(getCurrentApplicationId);
yield put(
updateApplicationLayout(applicationId || "", {

View File

@ -0,0 +1,34 @@
import { convertPathToString } from "@appsmith/workers/Evaluation/evaluationUtils";
import type { Diff } from "deep-diff";
import type { DataTree } from "entities/DataTree/dataTreeFactory";
import { get, set, unset } from "lodash";
export type TDataStore = Record<string, Record<string, unknown>>;
export default class DataStore {
private static store: TDataStore = {};
static setActionData(fullPath: string, value: unknown) {
set(this.store, fullPath, value);
}
static getActionData(fullPath: string): unknown | undefined {
return get(this.store, fullPath, undefined);
}
static getDataStore() {
return this.store;
}
static deleteActionData(fullPath: string) {
unset(this.store, fullPath);
}
static clear() {
this.store = {};
}
static update(dataTreeDiff: Diff<DataTree, DataTree>[]) {
const deleteDiffs = dataTreeDiff.filter((diff) => diff.kind === "D");
deleteDiffs.forEach((diff) => {
const deletedPath = diff.path || [];
const deletedPathString = convertPathToString(deletedPath);
this.deleteActionData(deletedPathString);
});
}
}

View File

@ -0,0 +1,28 @@
import type { DataTree } from "entities/DataTree/dataTreeFactory";
import type { TDataStore } from ".";
import {
isAction,
isJSAction,
} from "@appsmith/workers/Evaluation/evaluationUtils";
import { get, isEmpty, set } from "lodash";
export function updateTreeWithData(tree: DataTree, dataStore: TDataStore) {
if (isEmpty(dataStore)) return;
for (const entityName of Object.keys(tree)) {
const entity = tree[entityName];
if (!dataStore.hasOwnProperty(entityName)) continue;
if (isAction(entity)) {
set(entity, "data", get(dataStore, `${entityName}.data`));
}
if (isJSAction(entity)) {
const allFunctionsInStore = Object.keys(dataStore[entityName]);
allFunctionsInStore.forEach((functionName) => {
set(
entity[functionName],
`data`,
get(dataStore, `${entityName}.${functionName}.data`),
);
});
}
}
}

View File

@ -15,6 +15,9 @@ import type {
TriggerKind,
TriggerSource,
} from "constants/AppsmithActionConstants/ActionConstants";
import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData";
import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData";
import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils";
const _internalSetTimeout = self.setTimeout;
const _internalClearTimeout = self.clearTimeout;
@ -123,6 +126,20 @@ const fnExecutionDataHandler = priorityBatchedActionHandler((data) => {
{ JSExecutionData: {}, JSExecutionErrors: {} },
);
const updateActionProps: UpdateActionProps[] = Object.entries(
batchedData.JSExecutionData,
).map(([jsFnFullName, data]) => {
const { entityName, propertyPath: funcName } =
getEntityNameAndPropertyPath(jsFnFullName);
return {
entityName,
dataPath: `${funcName}.data`,
data,
};
});
handleActionsDataUpdate(updateActionProps);
WorkerMessenger.ping({
method: MAIN_THREAD_ACTION.PROCESS_JS_FUNCTION_EXECUTION,
data: batchedData,

View File

@ -24,6 +24,7 @@ import { setEvalContext } from "../evaluate";
import { getJSVariableCreatedEvents } from "../JSObject/JSVariableEvents";
import { errorModifier } from "../errorModifier";
import { generateOptimisedUpdatesAndSetPrevState } from "../helpers";
import DataStore from "../dataStore";
export let replayMap: Record<string, ReplayEntity<any>> | undefined;
export let dataTreeEvaluator: DataTreeEvaluator | undefined;
@ -250,5 +251,6 @@ export function clearCache() {
dataTreeEvaluator = undefined;
clearAllIntervals();
JSObjectCollection.clear();
DataStore.clear();
return true;
}

View File

@ -16,6 +16,7 @@ import setupEvaluationEnvironment, {
setEvaluationVersion,
} from "./setupEvalEnv";
import validateProperty from "./validateProperty";
import updateActionData from "./updateActionData";
const syncHandlerMap: Record<
EVAL_WORKER_SYNC_ACTION,
@ -33,6 +34,7 @@ const syncHandlerMap: Record<
[EVAL_WORKER_ACTIONS.CLEAR_CACHE]: clearCache,
[EVAL_WORKER_ACTIONS.SET_EVALUATION_VERSION]: setEvaluationVersion,
[EVAL_WORKER_ACTIONS.INIT_FORM_EVAL]: initFormEval,
[EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA]: updateActionData,
};
const asyncHandlerMap: Record<

View File

@ -0,0 +1,36 @@
import { dataTreeEvaluator } from "./evalTree";
import type { EvalWorkerSyncRequest } from "../types";
import { set } from "lodash";
import { evalTreeWithChanges } from "../evalTreeWithChanges";
import DataStore from "../dataStore";
export interface UpdateActionProps {
entityName: string;
dataPath: string;
data: unknown;
}
export default function (request: EvalWorkerSyncRequest) {
const actionsDataToUpdate: UpdateActionProps[] = request.data;
handleActionsDataUpdate(actionsDataToUpdate);
}
export function handleActionsDataUpdate(actionsToUpdate: UpdateActionProps[]) {
if (!dataTreeEvaluator) {
return {};
}
const evalTree = dataTreeEvaluator.getEvalTree();
for (const actionToUpdate of actionsToUpdate) {
const { data, dataPath, entityName } = actionToUpdate;
// update the evaltree
set(evalTree, `${entityName}.[${dataPath}]`, data);
// Update context
set(self, `${entityName}.[${dataPath}]`, data);
// Update the datastore
DataStore.setActionData(`${entityName}.${dataPath}`, data);
}
const updatedProperties: string[][] = actionsToUpdate.map(
({ dataPath, entityName }) => [entityName, dataPath],
);
evalTreeWithChanges(updatedProperties, []);
}

View File

@ -94,6 +94,7 @@ import type {
ValidationConfig,
} from "constants/PropertyControlConstants";
import { klona } from "klona/full";
import { klona as klonaJSON } from "klona/json";
import type { EvalMetaUpdates } from "@appsmith/workers/common/DataTreeEvaluator/types";
import {
updateDependencyMap,
@ -123,6 +124,8 @@ import userLogs from "workers/Evaluation/fns/overrides/console";
import ExecutionMetaData from "workers/Evaluation/fns/utils/ExecutionMetaData";
import DependencyMap from "entities/DependencyMap";
import { DependencyMapUtils } from "entities/DependencyMap/DependencyMapUtils";
import DataStore from "workers/Evaluation/dataStore";
import { updateTreeWithData } from "workers/Evaluation/dataStore/utils";
type SortedDependencies = Array<string>;
export type EvalProps = {
@ -339,6 +342,7 @@ export default class DataTreeEvaluator {
const evaluationStartTime = performance.now();
const evaluationOrder = this.sortedDependencies;
// Evaluate
const { evalMetaUpdates, evaluatedTree, staleMetaIds } = this.evaluateTree(
this.oldUnEvalTree,
@ -495,6 +499,7 @@ export default class DataTreeEvaluator {
isNewWidgetAdded: false,
};
}
DataStore.update(differences);
let isNewWidgetAdded = false;
//find all differences which can lead to updating of dependency map
@ -937,6 +942,7 @@ export default class DataTreeEvaluator {
staleMetaIds: string[];
} {
const tree = klona(oldUnevalTree);
updateTreeWithData(tree, klonaJSON(DataStore.getDataStore()));
errorModifier.updateAsyncFunctions(
tree,