chore: improved performance of getUnevaluatedDataTree (#37189)

## Description
Refactored getUnevaluatedDataTree selector to be more resilient to state
changes thereby reselect cache gets triggered less often. Identified an
action which was firing the selectors unnecessarily, fixed that as well.
For our customer during page load it used to take about 800ms
cumulatively, it has now dropped to about 160ms by about 80%. This is
also an impactful selector which cumulatively takes about 50,000 seconds
for all our client systems per week, hence we focussed our optimisation
here

## 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/11658078700>
> Commit: 557172d47b2232800355e1dc78c921d9cb56c725
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11658078700&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Mon, 04 Nov 2024 06:00:06 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

## Summary by CodeRabbit

- **Bug Fixes**
- Improved state management by preventing unnecessary updates to loading
entities, enhancing app performance.

- **Refactor**
- Updated the `loadingEntitiesReducer` to include an equality check for
loading entities before state updates.
- Enhanced date handling capabilities in the DatePicker widget tests and
commands.
- Restructured the `DataTreeFactory` class for improved modularity and
clarity in data tree creation.
- Simplified selector functions for better readability and
maintainability in data tree selectors.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Vemparala Surya Vamsi 2024-11-04 16:03:03 +05:30 committed by GitHub
parent bc165cf827
commit c8943509a0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 217 additions and 169 deletions

View File

@ -30,13 +30,13 @@ describe(
formWidgetsPage.datepickerWidget,
widgetsPage.widgetNameSpan,
);
// change the date to next day
cy.get(formWidgetsPage.defaultDate).click();
/**
* setDate--> is a Command to select the date in the date picker
*/
cy.setDate(1);
const nextDay = dayjs().add(1, "days").format("DD/MM/YYYY");
cy.log(nextDay);

View File

@ -1,6 +1,5 @@
# To run only limited tests - give the spec names in below format:
cypress/e2e/Regression/ClientSide/Templates/Fork_Template_spec.js
# For running all specs - uncomment below:
#cypress/e2e/**/**/*

View File

@ -2,6 +2,7 @@
/* eslint-disable cypress/no-assigning-return-values */
/* This file is used to maintain comman methods across tests , refer other *.js files for adding common methods */
import { ANVIL_EDITOR_TEST } from "./Constants.js";
import advancedFormat from "dayjs/plugin/advancedFormat";
import EditorNavigation, {
EntityType,
@ -18,6 +19,7 @@ import { v4 as uuidv4 } from "uuid";
const dayjs = require("dayjs");
const loginPage = require("../locators/LoginPage.json");
import homePage from "../locators/HomePage";
dayjs.extend(advancedFormat);
const commonlocators = require("../locators/commonlocators.json");
const widgetsPage = require("../locators/Widgets.json");
@ -525,7 +527,7 @@ Cypress.Commands.add("getDate", (date, dateFormate) => {
});
Cypress.Commands.add("setDate", (date) => {
const expDate = dayjs().add(date, "days").format("dddd, MMMM DD");
const expDate = dayjs().add(date, "days").format("dddd, MMMM Do, YYYY");
cy.get(`.react-datepicker__day[aria-label^="Choose ${expDate}"]`).click();
});

View File

@ -199,7 +199,6 @@ export interface DataTreeSeed {
pluginDependencyConfig: Record<string, DependencyMap>;
widgets: CanvasWidgetsReduxState;
widgetsMeta: MetaState;
pageList: Page[];
appData: AppDataState;
jsActions: JSCollectionDataState;
theme: AppTheme["properties"];

View File

@ -1,88 +1,71 @@
import { generateDataTreeAction } from "ee/entities/DataTree/dataTreeAction";
import { generateDataTreeJSAction } from "ee/entities/DataTree/dataTreeJSAction";
import { generateDataTreeWidget } from "entities/DataTree/dataTreeWidget";
import log from "loglevel";
import {
ENTITY_TYPE,
EvaluationSubstitutionType,
} from "ee/entities/DataTree/types";
import { generateDataTreeModuleInputs } from "ee/entities/DataTree/utils";
import type {
DataTreeSeed,
AppsmithEntity,
EntityTypeValue,
} from "ee/entities/DataTree/types";
import type {
unEvalAndConfigTree,
ConfigTree,
UnEvalTree,
} from "entities/DataTree/dataTreeTypes";
import type { EntityTypeValue } from "ee/entities/DataTree/types";
import type { ConfigTree, UnEvalTree } from "entities/DataTree/dataTreeTypes";
import { isEmpty } from "lodash";
import { generateModuleInstance } from "ee/entities/DataTree/dataTreeModuleInstance";
import {
endSpan,
startNestedSpan,
startRootSpan,
} from "UITelemetry/generateTraces";
import { endSpan, startRootSpan } from "UITelemetry/generateTraces";
import type { ActionDataState } from "ee/reducers/entityReducers/actionsReducer";
import type { JSCollectionDataState } from "ee/reducers/entityReducers/jsActionsReducer";
import type { LayoutSystemTypes } from "layoutSystems/types";
import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer";
import type { MetaState } from "reducers/entityReducers/metaReducer";
import type { LoadingEntitiesState } from "reducers/evaluationReducers/loadingEntitiesReducer";
import type { MetaWidgetsReduxState } from "reducers/entityReducers/metaWidgetsReducer";
import type { Module } from "ee/constants/ModuleConstants";
import type { ModuleInstance } from "ee/constants/ModuleInstanceConstants";
import type {
DependencyMap,
FormEditorConfigs,
} from "utils/DynamicBindingUtils";
export class DataTreeFactory {
static create({
actions,
appData,
editorConfigs,
isMobile,
jsActions,
layoutSystemType,
loadingEntities,
metaWidgets,
moduleInputs,
moduleInstanceEntities,
moduleInstances,
pluginDependencyConfig,
theme,
widgets,
widgetsMeta,
}: DataTreeSeed): unEvalAndConfigTree {
public static metaWidgets(
metaWidgets: MetaWidgetsReduxState,
widgetsMeta: MetaState,
loadingEntities: LoadingEntitiesState,
) {
const dataTree: UnEvalTree = {};
const configTree: ConfigTree = {};
const start = performance.now();
const startActions = performance.now();
const rootSpan = startRootSpan("DataTreeFactory.create");
const actionsSpan = startNestedSpan("DataTreeFactory.actions", rootSpan);
const metaWidgetsSpan = startRootSpan("DataTreeFactory.metaWidgets");
actions.forEach((action) => {
const editorConfig = editorConfigs[action.config.pluginId];
const dependencyConfig = pluginDependencyConfig[action.config.pluginId];
const { configEntity, unEvalEntity } = generateDataTreeAction(
action,
editorConfig,
dependencyConfig,
Object.values(metaWidgets).forEach((widget) => {
const { configEntity, unEvalEntity } = generateDataTreeWidget(
widget,
widgetsMeta[widget.metaWidgetId || widget.widgetId],
loadingEntities,
);
dataTree[action.config.name] = unEvalEntity;
configTree[action.config.name] = configEntity;
dataTree[widget.widgetName] = unEvalEntity;
configTree[widget.widgetName] = configEntity;
});
const endActions = performance.now();
endSpan(metaWidgetsSpan);
endSpan(actionsSpan);
return {
dataTree,
configTree,
};
}
const startJsActions = performance.now();
const jsActionsSpan = startNestedSpan(
"DataTreeFactory.jsActions",
rootSpan,
);
jsActions.forEach((js) => {
const { configEntity, unEvalEntity } = generateDataTreeJSAction(js);
dataTree[js.config.name] = unEvalEntity;
configTree[js.config.name] = configEntity;
});
const endJsActions = performance.now();
endSpan(jsActionsSpan);
const startWidgets = performance.now();
const widgetsSpan = startNestedSpan("DataTreeFactory.widgets", rootSpan);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public static widgets(
moduleInputs: Module["inputsForm"],
moduleInstances: Record<string, ModuleInstance> | null,
moduleInstanceEntities: null,
widgets: CanvasWidgetsReduxState,
widgetsMeta: MetaState,
loadingEntities: LoadingEntitiesState,
layoutSystemType: LayoutSystemTypes,
isMobile: boolean,
) {
const dataTree: UnEvalTree = {};
const configTree: ConfigTree = {};
const widgetsSpan = startRootSpan("DataTreeFactory.widgets");
if (!isEmpty(moduleInputs)) {
const { configEntity, unEvalEntity } =
@ -120,54 +103,60 @@ export class DataTreeFactory {
dataTree[widget.widgetName] = unEvalEntity;
configTree[widget.widgetName] = configEntity;
});
const endWidgets = performance.now();
endSpan(widgetsSpan);
dataTree.appsmith = {
...appData,
// combine both persistent and transient state with the transient state
// taking precedence in case the key is the same
store: appData.store,
theme,
} as AppsmithEntity;
(dataTree.appsmith as AppsmithEntity).ENTITY_TYPE = ENTITY_TYPE.APPSMITH;
const startMetaWidgets = performance.now();
const metaWidgetsSpan = startNestedSpan(
"DataTreeFactory.metaWidgets",
rootSpan,
);
Object.values(metaWidgets).forEach((widget) => {
const { configEntity, unEvalEntity } = generateDataTreeWidget(
widget,
widgetsMeta[widget.metaWidgetId || widget.widgetId],
loadingEntities,
);
dataTree[widget.widgetName] = unEvalEntity;
configTree[widget.widgetName] = configEntity;
});
const endMetaWidgets = performance.now();
endSpan(metaWidgetsSpan);
endSpan(rootSpan);
const end = performance.now();
const out = {
total: end - start,
widgets: endWidgets - startWidgets,
actions: endActions - startActions,
jsActions: endJsActions - startJsActions,
metaWidgets: endMetaWidgets - startMetaWidgets,
return {
dataTree,
configTree,
};
}
log.debug("### Create unevalTree timing", out);
public static jsActions(jsActions: JSCollectionDataState) {
const dataTree: UnEvalTree = {};
const configTree: ConfigTree = {};
const actionsSpan = startRootSpan("DataTreeFactory.jsActions");
return { unEvalTree: dataTree, configTree };
jsActions.forEach((js) => {
const { configEntity, unEvalEntity } = generateDataTreeJSAction(js);
dataTree[js.config.name] = unEvalEntity;
configTree[js.config.name] = configEntity;
});
endSpan(actionsSpan);
return {
dataTree,
configTree,
};
}
public static actions(
actions: ActionDataState,
editorConfigs: FormEditorConfigs,
pluginDependencyConfig: Record<string, DependencyMap>,
) {
const dataTree: UnEvalTree = {};
const configTree: ConfigTree = {};
const actionsSpan = startRootSpan("DataTreeFactory.actions");
actions.forEach((action) => {
const editorConfig = editorConfigs[action.config.pluginId];
const dependencyConfig = pluginDependencyConfig[action.config.pluginId];
const { configEntity, unEvalEntity } = generateDataTreeAction(
action,
editorConfig,
dependencyConfig,
);
dataTree[action.config.name] = unEvalEntity;
configTree[action.config.name] = configEntity;
});
endSpan(actionsSpan);
return {
dataTree,
configTree,
};
}
}

View File

@ -225,8 +225,6 @@ describe("generateDataTreeWidget", () => {
parentColumnSpace: 0,
parentRowSpace: 0,
rightColumn: 0,
renderMode: RenderModes.CANVAS,
version: 0,
topRow: 0,
widgetId: "123",
widgetName: "Input1",

View File

@ -1,5 +1,5 @@
import { getAllPathsFromPropertyConfig } from "entities/Widget/utils";
import _, { get, isEmpty } from "lodash";
import _, { get, isEmpty, omit } from "lodash";
import memoize from "micro-memoize";
import type { FlattenedWidgetProps } from "reducers/entityReducers/canvasWidgetsReducer";
import type { DynamicPath } from "utils/DynamicBindingUtils";
@ -21,6 +21,7 @@ import WidgetFactory from "WidgetProvider/factory";
import { getComponentDimensions } from "layoutSystems/common/utils/ComponentSizeUtils";
import type { LoadingEntitiesState } from "reducers/evaluationReducers/loadingEntitiesReducer";
import { LayoutSystemTypes } from "layoutSystems/types";
import { WIDGET_PROPS_TO_SKIP_FROM_EVAL } from "constants/WidgetConstants";
/**
*
@ -176,13 +177,17 @@ export function getSetterConfig(
// Widget changes only when dynamicBindingPathList changes.
// Only meta properties change very often, for example typing in an input or selecting a table row.
const generateDataTreeWidgetWithoutMeta = (
widget: FlattenedWidgetProps,
widgetWithEval: FlattenedWidgetProps,
): {
dataTreeWidgetWithoutMetaProps: WidgetEntity;
overridingMetaPropsMap: Record<string, boolean>;
defaultMetaProps: Record<string, unknown>;
entityConfig: WidgetEntityConfig;
} => {
const widget = omit(
widgetWithEval,
Object.keys(WIDGET_PROPS_TO_SKIP_FROM_EVAL),
) as FlattenedWidgetProps;
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const derivedProps: any = {};

View File

@ -1,6 +1,7 @@
import { createReducer } from "utils/ReducerUtils";
import type { ReduxAction } from "ee/constants/ReduxActionConstants";
import { ReduxActionTypes } from "ee/constants/ReduxActionConstants";
import { isEqual } from "lodash";
export type LoadingEntitiesState = Set<string>;
@ -10,7 +11,16 @@ const loadingEntitiesReducer = createReducer(initialState, {
[ReduxActionTypes.SET_LOADING_ENTITIES]: (
state: LoadingEntitiesState,
action: ReduxAction<Set<string>>,
): LoadingEntitiesState => action.payload,
): LoadingEntitiesState => {
const newLoadingEntities = action.payload;
// its just a set with string properties time complexity of equal is not too bad
if (isEqual(state, newLoadingEntities)) {
return state;
}
return newLoadingEntities;
},
[ReduxActionTypes.FETCH_PAGE_INIT]: () => initialState,
});

View File

@ -23,6 +23,7 @@ import { sortJSExecutionDataByCollectionId } from "workers/Evaluation/JSObject/u
import type { LintTreeSagaRequestData } from "plugins/Linting/types";
import { evalErrorHandler } from "./EvalErrorHandler";
import { getUnevaluatedDataTree } from "selectors/dataTreeSelectors";
import { endSpan, startRootSpan } from "UITelemetry/generateTraces";
export interface UpdateDataTreeMessageData {
workerResponse: EvalTreeResponseData;
@ -166,9 +167,13 @@ export function* handleEvalWorkerMessage(message: TMessage<any>) {
}
case MAIN_THREAD_ACTION.UPDATE_DATATREE: {
const { workerResponse } = data as UpdateDataTreeMessageData;
const rootSpan = startRootSpan("DataTreeFactory.create");
const unEvalAndConfigTree: ReturnType<typeof getUnevaluatedDataTree> =
yield select(getUnevaluatedDataTree);
endSpan(rootSpan);
yield call(updateDataTreeHandler, {
evalTreeResponse: workerResponse as EvalTreeResponseData,
unevalTree: unEvalAndConfigTree.unEvalTree || {},

View File

@ -350,10 +350,14 @@ export function* evaluateAndExecuteDynamicTrigger(
callbackData?: Array<any>,
globalContext?: Record<string, unknown>,
) {
const rootSpan = startRootSpan("DataTreeFactory.create");
const unEvalTree: ReturnType<typeof getUnevaluatedDataTree> = yield select(
getUnevaluatedDataTree,
);
endSpan(rootSpan);
log.debug({ execute: dynamicTrigger });
const response: { errors: EvaluationError[]; result: unknown } = yield call(
evalWorker.request,
@ -497,8 +501,12 @@ export function* executeJSFunction(
export // TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function* validateProperty(property: string, value: any, props: WidgetProps) {
const rootSpan = startRootSpan("DataTreeFactory.create");
const unEvalAndConfigTree: ReturnType<typeof getUnevaluatedDataTree> =
yield select(getUnevaluatedDataTree);
endSpan(rootSpan);
const configTree = unEvalAndConfigTree.configTree;
const entityConfig = configTree[props.widgetName] as WidgetEntityConfig;
const validation = entityConfig?.validationPaths[property];
@ -661,9 +669,14 @@ function* evalAndLintingHandler(
return;
}
const rootSpan = startRootSpan("DataTreeFactory.create");
// Generate all the data needed for both eval and linting
const unEvalAndConfigTree: ReturnType<typeof getUnevaluatedDataTree> =
yield select(getUnevaluatedDataTree);
endSpan(rootSpan);
const postEvalActions = getPostEvalActions(action);
const fn: (...args: unknown[]) => CallEffect<unknown> | ForkEffect<unknown> =
isBlockingCall ? call : fork;

View File

@ -11,17 +11,23 @@ import {
getCurrentModuleActions,
getCurrentModuleJSCollections,
} from "ee/selectors/entitiesSelector";
import type { WidgetEntity } from "ee/entities/DataTree/types";
import type { DataTree } from "entities/DataTree/dataTreeTypes";
import { DataTreeFactory } from "entities/DataTree/dataTreeFactory";
import type { AppsmithEntity, WidgetEntity } from "ee/entities/DataTree/types";
import type {
ConfigTree,
DataTree,
UnEvalTree,
} from "entities/DataTree/dataTreeTypes";
import {
DataTreeFactory,
ENTITY_TYPE,
} from "entities/DataTree/dataTreeFactory";
import {
getIsMobileBreakPoint,
getMetaWidgets,
getWidgetsForEval,
getWidgets,
getWidgetsMeta,
} from "sagas/selectors";
import "url-search-params-polyfill";
import { getPageList } from "./appViewSelectors";
import type { AppState } from "ee/reducers";
import { getSelectedAppThemeProperties } from "./appThemingSelectors";
import type { LoadingEntitiesState } from "reducers/evaluationReducers/loadingEntitiesReducer";
@ -55,25 +61,20 @@ const getLayoutSystemPayload = createSelector(
},
);
const getCurrentActionEntities = createSelector(
const getCurrentActionsEntities = createSelector(
getCurrentActions,
getCurrentModuleActions,
getCurrentWorkflowActions,
(actions, moduleActions, workflowActions) => {
return [...actions, ...moduleActions, ...workflowActions];
},
);
const getCurrentJSActionsEntities = createSelector(
getCurrentJSCollections,
getCurrentModuleJSCollections,
getCurrentWorkflowJSActions,
(
actions,
moduleActions,
workflowActions,
jsActions,
moduleJSActions,
workflowJsActions,
) => {
return {
actions: [...actions, ...moduleActions, ...workflowActions],
jsActions: [...jsActions, ...moduleJSActions, ...workflowJsActions],
};
(jsActions, moduleJSActions, workflowJsActions) => {
return [...jsActions, ...moduleJSActions, ...workflowJsActions];
},
);
@ -90,49 +91,76 @@ const getModulesData = createSelector(
},
);
export const getUnevaluatedDataTree = createSelector(
getCurrentActionEntities,
getWidgetsForEval,
getWidgetsMeta,
getPageList,
getAppData,
const getActionsFromUnevaluatedDataTree = createSelector(
getCurrentActionsEntities,
getPluginEditorConfigs,
getPluginDependencyConfig,
getSelectedAppThemeProperties,
getMetaWidgets,
getLayoutSystemPayload,
getLoadingEntities,
getModulesData,
(
currentActionEntities,
widgets,
widgetsMeta,
pageListPayload,
appData,
editorConfigs,
pluginDependencyConfig,
selectedAppThemeProperty,
metaWidgets,
layoutSystemPayload,
loadingEntities,
modulesData,
) => {
const pageList = pageListPayload || [];
(actions, editorConfigs, pluginDependencyConfig) =>
DataTreeFactory.actions(actions, editorConfigs, pluginDependencyConfig),
);
return DataTreeFactory.create({
...currentActionEntities,
const getJSActionsFromUnevaluatedDataTree = createSelector(
getCurrentJSActionsEntities,
(jsActions) => DataTreeFactory.jsActions(jsActions),
);
const getWidgetsFromUnevaluatedDataTree = createSelector(
getModulesData,
getWidgets,
getWidgetsMeta,
getLoadingEntities,
getLayoutSystemPayload,
(moduleData, widgets, widgetsMeta, loadingEntities, layoutSystemPayload) =>
DataTreeFactory.widgets(
moduleData.moduleInputs,
moduleData.moduleInstances,
moduleData.moduleInstanceEntities,
widgets,
widgetsMeta,
pageList,
appData,
editorConfigs,
pluginDependencyConfig,
theme: selectedAppThemeProperty,
metaWidgets,
loadingEntities,
...layoutSystemPayload,
...modulesData,
});
layoutSystemPayload.layoutSystemType,
layoutSystemPayload.isMobile,
),
);
const getMetaWidgetsFromUnevaluatedDataTree = createSelector(
getMetaWidgets,
getWidgetsMeta,
getLoadingEntities,
(metaWidgets, widgetsMeta, loadingEntities) =>
DataTreeFactory.metaWidgets(metaWidgets, widgetsMeta, loadingEntities),
);
export const getUnevaluatedDataTree = createSelector(
getActionsFromUnevaluatedDataTree,
getJSActionsFromUnevaluatedDataTree,
getWidgetsFromUnevaluatedDataTree,
getMetaWidgetsFromUnevaluatedDataTree,
getAppData,
getSelectedAppThemeProperties,
(actions, jsActions, widgets, metaWidgets, appData, theme) => {
let dataTree: UnEvalTree = {
...actions.dataTree,
...jsActions.dataTree,
...widgets.dataTree,
};
let configTree: ConfigTree = {
...actions.configTree,
...jsActions.configTree,
...widgets.configTree,
};
dataTree.appsmith = {
...appData,
// combine both persistent and transient state with the transient state
// taking precedence in case the key is the same
store: appData.store,
theme,
} as AppsmithEntity;
(dataTree.appsmith as AppsmithEntity).ENTITY_TYPE = ENTITY_TYPE.APPSMITH;
dataTree = { ...dataTree, ...metaWidgets.dataTree };
configTree = { ...configTree, ...metaWidgets.configTree };
return { unEvalTree: dataTree, configTree };
},
);