From 57dd91f85eef10548cb2b991d0bc2ae2f9c96bcf Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Thu, 25 Apr 2024 19:43:35 +0530 Subject: [PATCH] chore: Remove config tree from eval worker response (#32900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Sending configTree from the webworker is a redundant operation, we can directly the same from the main thread state. By removing this property we reduce the transmission cost between the main thread to webworker, we are seeing as much as 10% reduction in main thread scripting from these changes. Fixes #32969 > [!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" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 58ad75ebf73714e7a6603a95f074826fcd11ec67 > Cypress dashboard url: Click here! ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **Refactor** - Optimized the evaluation process by streamlining how configuration data is retrieved and handled across different components. - Updated data retrieval methods in sagas and workers to improve efficiency and maintain consistency. --- app/client/src/sagas/EvalWorkerActionSagas.ts | 6 +++++- app/client/src/sagas/EvaluationsSaga.ts | 17 +++++++++++++---- .../workers/Evaluation/evalTreeWithChanges.ts | 9 +-------- .../src/workers/Evaluation/handlers/evalTree.ts | 1 - app/client/src/workers/Evaluation/types.ts | 6 +----- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/client/src/sagas/EvalWorkerActionSagas.ts b/app/client/src/sagas/EvalWorkerActionSagas.ts index 6b08899238..93d0e06465 100644 --- a/app/client/src/sagas/EvalWorkerActionSagas.ts +++ b/app/client/src/sagas/EvalWorkerActionSagas.ts @@ -1,4 +1,4 @@ -import { all, call, put, spawn, take } from "redux-saga/effects"; +import { all, call, put, select, spawn, take } from "redux-saga/effects"; import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants"; import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; import log from "loglevel"; @@ -23,6 +23,7 @@ import type { UnEvalTree } from "entities/DataTree/dataTreeTypes"; import { sortJSExecutionDataByCollectionId } from "workers/Evaluation/JSObject/utils"; import type { LintTreeSagaRequestData } from "plugins/Linting/types"; import { evalErrorHandler } from "./EvalErrorHandler"; +import { getUnevaluatedDataTree } from "selectors/dataTreeSelectors"; export interface UpdateDataTreeMessageData { workerResponse: EvalTreeResponseData; unevalTree: UnEvalTree; @@ -140,10 +141,13 @@ export function* handleEvalWorkerMessage(message: TMessage) { } case MAIN_THREAD_ACTION.UPDATE_DATATREE: { const { unevalTree, workerResponse } = data as UpdateDataTreeMessageData; + const unEvalAndConfigTree: ReturnType = + yield select(getUnevaluatedDataTree); yield call(updateDataTreeHandler, { evalTreeResponse: workerResponse as EvalTreeResponseData, unevalTree, requiresLogging: false, + configTree: unEvalAndConfigTree.configTree, }); break; } diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index a6bd4727b4..80237723d2 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -84,7 +84,11 @@ import { getAllJSActionsData, } from "@appsmith/selectors/entitiesSelector"; import type { WidgetEntityConfig } from "@appsmith/entities/DataTree/types"; -import type { DataTree, UnEvalTree } from "entities/DataTree/dataTreeTypes"; +import type { + ConfigTree, + DataTree, + UnEvalTree, +} from "entities/DataTree/dataTreeTypes"; import { initiateLinting, lintWorker } from "./LintingSagas"; import type { EvalTreeRequestData, @@ -126,15 +130,15 @@ export function* updateDataTreeHandler( evalTreeResponse: EvalTreeResponseData; unevalTree: UnEvalTree; requiresLogging: boolean; + configTree: ConfigTree; }, postEvalActions?: Array, ) { - const { evalTreeResponse, requiresLogging, unevalTree } = data; + const { configTree, evalTreeResponse, requiresLogging, unevalTree } = data; const postEvalActionsToDispatch: Array = postEvalActions || []; const { - configTree, dependencies, errors, evalMetaUpdates = [], @@ -286,7 +290,12 @@ export function* evaluateTreeSaga( yield call( updateDataTreeHandler, - { evalTreeResponse: workerResponse, unevalTree, requiresLogging }, + { + evalTreeResponse: workerResponse, + unevalTree, + configTree: unEvalAndConfigTree.configTree, + requiresLogging, + }, postEvalActions, ); } diff --git a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts index be7c34d20e..8835a78d48 100644 --- a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts +++ b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts @@ -1,8 +1,4 @@ -import type { - ConfigTree, - DataTree, - UnEvalTree, -} from "entities/DataTree/dataTreeTypes"; +import type { DataTree, UnEvalTree } from "entities/DataTree/dataTreeTypes"; import { dataTreeEvaluator } from "./handlers/evalTree"; import type { DataTreeDiff } from "@appsmith/workers/Evaluation/evaluationUtils"; import type { EvalMetaUpdates } from "@appsmith/workers/common/DataTreeEvaluator/types"; @@ -35,7 +31,6 @@ export function evalTreeWithChanges( let staleMetaIds: string[] = []; const removedPaths: Array<{ entityId: string; fullpath: string }> = []; let unevalTree: UnEvalTree = {}; - let configTree: ConfigTree = {}; if (dataTreeEvaluator) { const setupUpdateTreeResponse = @@ -62,7 +57,6 @@ export function evalTreeWithChanges( staleMetaIds = updateResponse.staleMetaIds; unevalTree = dataTreeEvaluator.getOldUnevalTree(); - configTree = dataTreeEvaluator.oldConfigTree; } const allUnevalUpdates = unEvalUpdates.map( (update) => update.payload.propertyPath, @@ -94,7 +88,6 @@ export function evalTreeWithChanges( logs, unEvalUpdates, isCreateFirstTree, - configTree, staleMetaIds, removedPaths, isNewWidgetAdded: false, diff --git a/app/client/src/workers/Evaluation/handlers/evalTree.ts b/app/client/src/workers/Evaluation/handlers/evalTree.ts index ca62439233..0864fa50a3 100644 --- a/app/client/src/workers/Evaluation/handlers/evalTree.ts +++ b/app/client/src/workers/Evaluation/handlers/evalTree.ts @@ -298,7 +298,6 @@ export function evalTree(request: EvalWorkerSyncRequest) { logs: shouldRespondWithLogs ? logs : [], unEvalUpdates, isCreateFirstTree, - configTree, staleMetaIds, removedPaths, isNewWidgetAdded, diff --git a/app/client/src/workers/Evaluation/types.ts b/app/client/src/workers/Evaluation/types.ts index 55017e5475..6ac92e7e39 100644 --- a/app/client/src/workers/Evaluation/types.ts +++ b/app/client/src/workers/Evaluation/types.ts @@ -1,7 +1,4 @@ -import type { - ConfigTree, - unEvalAndConfigTree, -} from "entities/DataTree/dataTreeTypes"; +import type { unEvalAndConfigTree } from "entities/DataTree/dataTreeTypes"; import type { ActionValidationConfigMap } from "constants/PropertyControlConstants"; import type { AppTheme } from "entities/AppTheming"; @@ -55,7 +52,6 @@ export interface EvalTreeResponseData { logs: unknown[]; unEvalUpdates: DataTreeDiff[]; isCreateFirstTree: boolean; - configTree: ConfigTree; staleMetaIds: string[]; removedPaths: Array<{ entityId: string; fullpath: string }>; isNewWidgetAdded: boolean;