chore: cache theme value properties, since it is a frequent property (#41031)
## Description Added caching of theme property value since it is a frequent expression, it constitutes 20% of all binding expressions for a large customer app. Expecting a 400ms reduction in LCP for a large customer app. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!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" ### 🔍 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/15880337835> > Commit: 11fab20fe285aa1b3b59c164179902628d35d97d > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15880337835&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Wed, 25 Jun 2025 17:45:37 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 - **New Features** - Improved performance when evaluating theme-related properties by introducing caching for repeated values. - **Chores** - Added a utility to identify specific theme-related unevaluated values. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
bde2554782
commit
5232b3f89e
|
|
@ -84,3 +84,14 @@ export function isWidgetActionOrJsObject(
|
|||
): entity is ActionEntity | WidgetEntity | JSActionEntity {
|
||||
return isWidget(entity) || isAction(entity) || isJSAction(entity);
|
||||
}
|
||||
|
||||
const THEME_PROPERTIES = new Set([
|
||||
"{{appsmith.theme.fontFamily.appFont}}",
|
||||
"{{appsmith.theme.boxShadow.appBoxShadow}}",
|
||||
"{{appsmith.theme.colors.primaryColor}}",
|
||||
"{{appsmith.theme.borderRadius.appBorderRadius}}",
|
||||
]);
|
||||
|
||||
export function isThemeUnevaluatedValue(value: string): boolean {
|
||||
return THEME_PROPERTIES.has(value);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,7 +136,10 @@ 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 { isWidgetActionOrJsObject } from "ee/entities/DataTree/utils";
|
||||
import {
|
||||
isWidgetActionOrJsObject,
|
||||
isThemeUnevaluatedValue,
|
||||
} from "ee/entities/DataTree/utils";
|
||||
import DataStore from "workers/Evaluation/dataStore";
|
||||
import { updateTreeWithData } from "workers/Evaluation/dataStore/utils";
|
||||
import microDiff from "microdiff";
|
||||
|
|
@ -1178,6 +1181,8 @@ export default class DataTreeEvaluator {
|
|||
}
|
||||
|
||||
const dependencies = this.dependencyMap.dependencies;
|
||||
// Add a cache for specific appsmith theme properties
|
||||
const themePropertyCache = new Map<string, unknown>();
|
||||
|
||||
try {
|
||||
for (const fullPropertyPath of evaluationOrder) {
|
||||
|
|
@ -1266,16 +1271,32 @@ export default class DataTreeEvaluator {
|
|||
}
|
||||
|
||||
try {
|
||||
evalPropertyValue = this.getDynamicValue(
|
||||
unEvalPropertyValue,
|
||||
contextTree,
|
||||
oldConfigTree,
|
||||
evaluationSubstitutionType,
|
||||
contextData,
|
||||
undefined,
|
||||
fullPropertyPath,
|
||||
evalContextCache,
|
||||
);
|
||||
const themeEvaluatedValue =
|
||||
themePropertyCache.get(unEvalPropertyValue);
|
||||
|
||||
// use the cached value if it exists
|
||||
if (themeEvaluatedValue) {
|
||||
evalPropertyValue = themeEvaluatedValue;
|
||||
} else {
|
||||
evalPropertyValue = this.getDynamicValue(
|
||||
unEvalPropertyValue,
|
||||
contextTree,
|
||||
oldConfigTree,
|
||||
evaluationSubstitutionType,
|
||||
contextData,
|
||||
undefined,
|
||||
fullPropertyPath,
|
||||
evalContextCache,
|
||||
);
|
||||
|
||||
if (
|
||||
evalPropertyValue &&
|
||||
isThemeUnevaluatedValue(unEvalPropertyValue)
|
||||
) {
|
||||
// we are caching theme properties because its a frequent unevaluated value roughly constitues 20% of all bindings
|
||||
themePropertyCache.set(unEvalPropertyValue, evalPropertyValue);
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
this.errors.push({
|
||||
type: EvalErrorTypes.EVAL_PROPERTY_ERROR,
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user