Skip to content

Commit

Permalink
More shouldUpdate optimizations (#6075)
Browse files Browse the repository at this point in the history
**Problem:**
Follow-up to #6070

**Fix:**
I introduced two more optimizations:

1. The bigger one is that shouldUpdate called
`isElementInChildrenOrPropsTree` separately for each of the element
paths in `ElementsToRerenderGLOBAL`, even though all these runs
traversed the same subtree, because they were called with the same
`props` parameter. I updated `isElementInChildrenOrPropsTree` to work on
an array of element paths, and check for all of them in a single
execution of the function. I was a bit worried that the added complexity
of this function may slow down the most typical single element use case,
but I did not find the effect measurable.

This really heavily speeds up multiselection interactions, basically a
multiselection with n elements is nearly n times faster with this code.

3. I added an early return to not do anything at all when the
props.children is null or a string (happens frequently).

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Preview mode
  • Loading branch information
gbalint authored and liady committed Dec 13, 2024
1 parent afb0be1 commit d07f2b5
Showing 1 changed file with 25 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,22 @@ export function createComponentRendererComponent(params: {
const instancePath: ElementPath | null = tryToGetInstancePath(instancePathAny, pathsString)

function shouldUpdate() {
return (
ElementsToRerenderGLOBAL.current === 'rerender-all-elements' ||
ElementsToRerenderGLOBAL.current.some((er) => {
return (
(instancePath != null &&
(EP.pathsEqual(er, instancePath) || EP.isParentComponentOf(instancePath, er))) ||
isElementInChildrenOrPropsTree(EP.toString(er), realPassedProps)
)
})
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,
)
}

Expand Down Expand Up @@ -379,36 +386,39 @@ function isRenderProp(prop: any): prop is { props: { [UTOPIA_PATH_KEY]: string }
)
}

function isElementInChildrenOrPropsTree(elementPath: string, props: any): boolean {
function areElementsInChildrenOrPropsTree(elementPaths: Array<string>, props: any): boolean {
const childrenArr = fastReactChildrenToArray(props.children)

for (let c of childrenArr) {
if ((c.props as any)[UTOPIA_PATH_KEY] === elementPath) {
if (elementPaths.includes((c.props as any)[UTOPIA_PATH_KEY])) {
return true
}
}

for (let p in props) {
if (React.isValidElement(p) && (p.props as any)[UTOPIA_PATH_KEY] === elementPath) {
if (React.isValidElement(p) && elementPaths.includes((p.props as any)[UTOPIA_PATH_KEY])) {
return true
}
}

for (let c of childrenArr) {
if (isElementInChildrenOrPropsTree(elementPath, c.props)) {
if (areElementsInChildrenOrPropsTree(elementPaths, c.props)) {
return true
}
}

for (let p in props) {
if (isRenderProp(p) && isElementInChildrenOrPropsTree(elementPath, p.props)) {
if (isRenderProp(p) && areElementsInChildrenOrPropsTree(elementPaths, p.props)) {
return true
}
}

return false
}

function fastReactChildrenToArray(children: any) {
if (children == null || typeof children === 'string') {
return []
}
if (React.isValidElement(children)) {
return [children]
}
Expand Down

0 comments on commit d07f2b5

Please sign in to comment.