From b45b071e1d53330825817d7231b99b5b1c8059cf Mon Sep 17 00:00:00 2001 From: Balazs Bajorics <2226774+balazsbajorics@users.noreply.github.com> Date: Fri, 19 Jul 2024 11:55:57 +0200 Subject: [PATCH] Fix/memoized should update (#6098) **Problem:** image There's a `shouldUpdate` function in ComponentRendererComponent which is not cheap, and it is memoized, and it's called 8 times for each canvas update, even for components where shouldUpdate will eventually be false. **Fix:** Turning shouldUpdate into a memoized function, only recalculating the value when any of its inputs change: image The time spent goes from ~1.5ms to sub 0.2ms. This is doubly relevant because I have a follow-up PR which introduces shouldUpdate-style callbacks to even more places. --- .../ui-jsx-canvas-component-renderer.tsx | 70 ++++++++++++------- .../ui-jsx-canvas-contexts.tsx | 13 +++- .../src/components/canvas/ui-jsx-canvas.tsx | 29 ++++---- ...performance-regression-tests.spec.tsx.snap | 12 ++-- .../performance-regression-tests.spec.tsx | 4 +- 5 files changed, 83 insertions(+), 45 deletions(-) diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx index 0e525092a52a..55954b926d6a 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-component-renderer.tsx @@ -19,13 +19,13 @@ import type { UiJsxCanvasContextData, VariableData, } from '../ui-jsx-canvas' -import { - DomWalkerInvalidatePathsCtxAtom, - UiJsxCanvasCtxAtom, - ElementsToRerenderGLOBAL, -} from '../ui-jsx-canvas' +import { DomWalkerInvalidatePathsCtxAtom, UiJsxCanvasCtxAtom } from '../ui-jsx-canvas' import type { MutableUtopiaCtxRefData } from './ui-jsx-canvas-contexts' -import { RerenderUtopiaCtxAtom, SceneLevelUtopiaCtxAtom } from './ui-jsx-canvas-contexts' +import { + ElementsToRerenderContext, + RerenderUtopiaCtxAtom, + SceneLevelUtopiaCtxAtom, +} from './ui-jsx-canvas-contexts' import { applyPropsParamToPassedProps } from './ui-jsx-canvas-props-utils' import { runBlockUpdatingScope } from './ui-jsx-canvas-scope-utils' import * as EP from '../../../core/shared/element-path' @@ -50,6 +50,7 @@ import { mapArrayToDictionary } from '../../../core/shared/array-utils' import { assertNever } from '../../../core/shared/utils' import { addFakeSpyEntry } from './ui-jsx-canvas-spy-wrapper' import type { FilePathMappings } from '../../../core/model/project-file-utils' +import type { ElementsToRerender } from '../../editor/store/editor-state' function tryToGetInstancePath( maybePath: ElementPath | null, @@ -101,25 +102,10 @@ export function createComponentRendererComponent(params: { const instancePath: ElementPath | null = tryToGetInstancePath(instancePathAny, pathsString) - function shouldUpdate() { - if (ElementsToRerenderGLOBAL.current === 'rerender-all-elements') { - return true - } - - if ( - instancePath != null && - ElementsToRerenderGLOBAL.current.some( - (er) => EP.pathsEqual(er, instancePath) || EP.isParentComponentOf(instancePath, er), - ) - ) { - return true - } - - return areElementsInChildrenOrPropsTree( - ElementsToRerenderGLOBAL.current.map(EP.toString), - realPassedProps, - ) - } + const shouldUpdate = useAllowRerenderForPath( + instancePath ?? 'rerender-all-elements', + realPassedProps, + ) const rerenderUtopiaContext = usePubSubAtomReadOnly(RerenderUtopiaCtxAtom, shouldUpdate) @@ -286,6 +272,7 @@ export function createComponentRendererComponent(params: { ) applyBlockReturnFunctions(scope) + // possibly problematic: this should ONLY run if shouldUpdate() is true const arbitraryBlockResult = runBlockUpdatingScope( rootElementPath, params.filePath, @@ -427,3 +414,36 @@ function fastReactChildrenToArray(children: any) { } return React.Children.toArray(children).filter(React.isValidElement) } + +export function useAllowRerenderForPath( + elementPath: ElementPath | 'rerender-all-elements', + realPassedProps: any, +) { + const elementsToRerender = React.useContext(ElementsToRerenderContext) + const shouldUpdate = React.useMemo( + () => shouldUpdateInner(elementPath, realPassedProps, elementsToRerender), + [elementPath, realPassedProps, elementsToRerender], + ) + return React.useCallback(() => shouldUpdate, [shouldUpdate]) +} + +function shouldUpdateInner( + elementPath: ElementPath | 'rerender-all-elements', + realPassedProps: any, + elementsToRerender: ElementsToRerender, +) { + if (elementsToRerender === 'rerender-all-elements') { + return true + } + + if ( + elementPath != 'rerender-all-elements' && + elementsToRerender.some( + (er) => EP.pathsEqual(er, elementPath) || EP.isParentComponentOf(elementPath, er), + ) + ) { + return true + } + + return areElementsInChildrenOrPropsTree(elementsToRerender.map(EP.toString), realPassedProps) +} diff --git a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx index f223928216b2..40e6daf4202b 100644 --- a/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas-renderer/ui-jsx-canvas-contexts.tsx @@ -1,4 +1,4 @@ -import type React from 'react' +import React from 'react' import { emptySet } from '../../../core/shared/set-utils' import type { MapLike } from 'typescript' import { atomWithPubSub } from '../../../core/shared/atom-with-pub-sub' @@ -6,7 +6,11 @@ import type { Either } from '../../../core/shared/either' import { left } from '../../../core/shared/either' import type { ElementPath } from '../../../core/shared/project-file-types' import type { ProjectContentTreeRoot } from '../../assets' -import type { TransientFilesState, UIFileBase64Blobs } from '../../editor/store/editor-state' +import type { + ElementsToRerender, + TransientFilesState, + UIFileBase64Blobs, +} from '../../editor/store/editor-state' import type { VariableData } from '../ui-jsx-canvas' import type { FilePathMappings } from '../../../core/model/project-file-utils' @@ -78,3 +82,8 @@ export const SceneLevelUtopiaCtxAtom = atomWithPubSub({ validPaths: new Set(), }, }) + +type ElementsToRerenderContextValue = ElementsToRerender + +export const ElementsToRerenderContext = + React.createContext('rerender-all-elements') diff --git a/editor/src/components/canvas/ui-jsx-canvas.tsx b/editor/src/components/canvas/ui-jsx-canvas.tsx index 92bf28f445e1..a82a516c1c5f 100644 --- a/editor/src/components/canvas/ui-jsx-canvas.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas.tsx @@ -54,6 +54,7 @@ import parse from 'html-react-parser' import type { ComponentRendererComponent } from './ui-jsx-canvas-renderer/component-renderer-component' import type { MutableUtopiaCtxRefData } from './ui-jsx-canvas-renderer/ui-jsx-canvas-contexts' import { + ElementsToRerenderContext, RerenderUtopiaCtxAtom, SceneLevelUtopiaCtxAtom, UtopiaProjectCtxAtom, @@ -559,18 +560,22 @@ export const UiJsxCanvas = React.memo((props) }} > {parse(linkTags)} - - - - - {StoryboardRoot} - - - - + + + + + + {StoryboardRoot} + + + + + ) }) diff --git a/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap b/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap index 91a952f52245..8164402bac7a 100644 --- a/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap +++ b/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap @@ -1273,8 +1273,9 @@ Array [ "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//Symbol(react.forward_ref)(CanvasContainer)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", + "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedExoticType(Symbol(react.provider))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//div", - "//div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", + "/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/Symbol(react.forward_ref)(CanvasContainer)/div:data-testid='canvas-container'", "/Symbol(react.forward_ref)(CanvasContainer)/div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", @@ -1571,8 +1572,9 @@ Array [ "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//Symbol(react.forward_ref)(CanvasContainer)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", + "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedExoticType(Symbol(react.provider))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//div", - "//div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", + "/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/Symbol(react.forward_ref)(CanvasContainer)/div:data-testid='canvas-container'", "/Symbol(react.forward_ref)(CanvasContainer)/div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", @@ -2413,8 +2415,9 @@ Array [ "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//Symbol(react.forward_ref)(CanvasContainer)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", + "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedExoticType(Symbol(react.provider))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//div", - "//div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", + "/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/Symbol(react.forward_ref)(CanvasContainer)/div:data-testid='canvas-container'", "/Symbol(react.forward_ref)(CanvasContainer)/div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", @@ -2699,8 +2702,9 @@ Array [ "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//Symbol(react.forward_ref)(CanvasContainer)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedFunctionComponent(Provider)", + "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//UtopiaSpiedExoticType(Symbol(react.provider))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))//div", - "//div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", + "/UtopiaSpiedExoticType(Symbol(react.provider))/Symbol(react.provider)/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", "/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))/Symbol(react.forward_ref)(CanvasContainer)/div:data-testid='canvas-container'", "/Symbol(react.forward_ref)(CanvasContainer)/div/UtopiaSpiedFunctionComponent(Provider)/UtopiaSpiedExoticType(Symbol(react.fragment))", diff --git a/editor/src/core/performance/performance-regression-tests.spec.tsx b/editor/src/core/performance/performance-regression-tests.spec.tsx index 46411581651f..c22ce1d81936 100644 --- a/editor/src/core/performance/performance-regression-tests.spec.tsx +++ b/editor/src/core/performance/performance-regression-tests.spec.tsx @@ -65,7 +65,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`787`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`789`) expect(renderResult.getRenderInfo()).toMatchSnapshot() }) @@ -127,7 +127,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`1135`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`1137`) expect(renderResult.getRenderInfo()).toMatchSnapshot() })