Skip to content

Commit

Permalink
performance(editor) Improved buildTree. (#6325)
Browse files Browse the repository at this point in the history
- Reworked `buildTree_MUTATE` to build the tree from the root downwards,
combining it with some of the logic taken from `getChildrenPaths`.
- Further improvement to `isDescendantOf` replacing the previous
implementation.
- `getReorderedPaths` now avoids a lot of repeated logic for checking
the element type.
- Broke apart `ElementPathTree.children` into two separate fields
  to make for easier ordering.
- Added `getElementPathTreeChildren` and `forEachElementPathTreeChild`
utility functions.
- `buildTree_MUTATE` now adds to the two different children fields based
on the element path representing inner children over property children.
- `getReorderedIndexInPaths` further reworked to handle shifting by the
siblings.
- Fixed some tests that had expressions incorrectly ordered.
  • Loading branch information
seanparsons authored Sep 11, 2024
1 parent c929e92 commit 73b4df7
Show file tree
Hide file tree
Showing 14 changed files with 324 additions and 160 deletions.
4 changes: 2 additions & 2 deletions editor/src/benchmarks.benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { benchmarkOptics } from './core/shared/optics.benchmark'
import { benchmarkPropertyPathFunction } from './core/shared/property-path.benchmark'
import { benchmarkGetUniqueUids } from './core/model/get-unique-ids.benchmark'

// await benchmarkBuildTree()
await benchmarkBuildTree()
// await benchmarkElementPathFunction()
// await benchmarkPropertyPathFunction()
// await benchmarkAttributes()
// await benchmarkOptics()
await benchmarkGetUniqueUids()
// await benchmarkGetUniqueUids()
4 changes: 2 additions & 2 deletions editor/src/components/canvas/canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
} from '../editor/store/editor-state'
import * as EP from '../../core/shared/element-path'
import type { ElementPathTree, ElementPathTrees } from '../../core/shared/element-path-tree'
import { getSubTree } from '../../core/shared/element-path-tree'
import { forEachElementPathTreeChild, getSubTree } from '../../core/shared/element-path-tree'
import { assertNever, fastForEach } from '../../core/shared/utils'
import { memoize } from '../../core/shared/memoize'
import { maybeToArray } from '../../core/shared/optional-utils'
Expand Down Expand Up @@ -73,7 +73,7 @@ function getFramesInCanvasContextUncached(

let children: Array<ElementPathTree> = []
let unfurledComponents: Array<ElementPathTree> = []
fastForEach(Object.values(componentTree.children), (childTree) => {
forEachElementPathTreeChild(componentTree, (childTree) => {
if (EP.isRootElementOfInstance(childTree.path)) {
unfurledComponents.push(childTree)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { boundingArea, createInteractionViaMouse } from '../../canvas-strategies
import { windowToCanvasCoordinates } from '../../dom-lookup'
import { CanvasOffsetWrapper } from '../canvas-offset-wrapper'
import { isCommentMode, isSelectModeWithArea } from '../../../editor/editor-modes'
import { getSubTree } from '../../../../core/shared/element-path-tree'
import { getElementPathTreeChildren, getSubTree } from '../../../../core/shared/element-path-tree'

interface SceneLabelControlProps {
maybeHighlightOnHover: (target: ElementPath) => void
Expand Down Expand Up @@ -65,7 +65,7 @@ const SceneLabel = React.memo<SceneLabelProps>((props) => {
if (subTree == null) {
return false
} else {
return subTree.children.length === 1
return getElementPathTreeChildren(subTree).length === 1
}
},
'SceneLabel sceneHasSingleChild',
Expand Down
6 changes: 3 additions & 3 deletions editor/src/components/canvas/dom-sampler.spec.browser2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,16 +296,14 @@ export var storyboard = (
navigatorEntryToKey,
),
).toEqual([
'regular-sb/1e7',
'regular-sb/sc',
'regular-sb/sc/app',
'regular-sb/sc/app:app-root',
'regular-sb/sc/app:app-root/card',
'regular-sb/sc/app:app-root/card:card-root',
'regular-sb/sc/app:app-root/card:card-root/30d',
'regular-sb/sc/app:app-root/card:card-root/card-span',
'regular-sb/sc/app:app-root/card:card-root/30d',
'regular-sb/sc/app:app-root/card/card-child',
'regular-sb/sc/app:app-root/children-code-block',
'regular-sb/sc/app:app-root/frag',
'regular-sb/sc/app:app-root/frag/frag-child',
'regular-sb/sc/app:app-root/frag/cond-1',
Expand All @@ -319,7 +317,9 @@ export var storyboard = (
'synthetic-sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/d84-attribute',
'conditional-clause-sb/sc/app:app-root/frag/cond-1-false-case',
'synthetic-sb/sc/app:app-root/frag/cond-1/019-attribute',
'regular-sb/sc/app:app-root/children-code-block',
'regular-sb/sc/app/app-child',
'regular-sb/1e7',
])
})

Expand Down
8 changes: 6 additions & 2 deletions editor/src/components/canvas/gap-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import type { CSSNumber, FlexDirection } from '../inspector/common/css-utils'
import type { Sides } from 'utopia-api/core'
import { sides } from 'utopia-api/core'
import { styleStringInArray } from '../../utils/common-constants'
import { getSubTree, type ElementPathTrees } from '../../core/shared/element-path-tree'
import {
getElementPathTreeChildren,
getSubTree,
type ElementPathTrees,
} from '../../core/shared/element-path-tree'
import { isReversedFlexDirection } from '../../core/model/flex-utils'
import * as EP from '../../core/shared/element-path'
import { treatElementAsFragmentLike } from './canvas-strategies/strategies/fragment-like-helpers'
Expand Down Expand Up @@ -380,7 +384,7 @@ export function recurseIntoChildrenOfMapOrFragment(
return []
}

return subTree.children.flatMap((element) => {
return getElementPathTreeChildren(subTree).flatMap((element) => {
const elementPath = element.path
if (EP.isRootElementOfInstance(elementPath)) {
return []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2996,12 +2996,14 @@ export function ElementPathTreeKeepDeepEquality(
oldValue: ElementPathTree,
newValue: ElementPathTree,
): KeepDeepEqualityResult<ElementPathTree> {
return combine3EqualityCalls(
return combine4EqualityCalls(
(pathTree) => pathTree.path,
ElementPathKeepDeepEquality,
(pathTree) => pathTree.pathString,
createCallWithTripleEquals<string>(),
(pathTree) => pathTree.children,
(pathTree) => pathTree.innerChildren,
arrayDeepEquality(ElementPathTreeKeepDeepEquality),
(pathTree) => pathTree.propsChildren,
arrayDeepEquality(ElementPathTreeKeepDeepEquality),
elementPathTree,
)(oldValue, newValue)
Expand Down
18 changes: 12 additions & 6 deletions editor/src/components/navigator/navigator-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ import {
syntheticNavigatorEntry,
} from '../editor/store/editor-state'
import type { ElementPathTree, ElementPathTrees } from '../../core/shared/element-path-tree'
import { getCanvasRoots, getSubTree } from '../../core/shared/element-path-tree'
import {
getCanvasRoots,
getElementPathTreeChildren,
getSubTree,
} from '../../core/shared/element-path-tree'
import { assertNever } from '../../core/shared/utils'
import type { ConditionalCase } from '../../core/model/conditionals'
import { getConditionalClausePath } from '../../core/model/conditionals'
Expand Down Expand Up @@ -320,7 +324,9 @@ function walkRegularNavigatorEntry(

const childPath = EP.appendToPath(elementPath, getUtopiaID(propValue))

const subTreeChild = subTree?.children.find((child) => EP.pathsEqual(child.path, childPath))
const subTreeChild = getElementPathTreeChildren(subTree).find((child) =>
EP.pathsEqual(child.path, childPath),
)
if (subTreeChild != null) {
const childTreeEntry = createNavigatorSubtree(
metadata,
Expand Down Expand Up @@ -359,7 +365,7 @@ function walkRegularNavigatorEntry(
})
}

const childrenPaths = subTree.children.filter(
const childrenPaths = getElementPathTreeChildren(subTree).filter(
(child) => !processedAccumulator.has(EP.toString(child.path)),
)
const children: Array<NavigatorTree> = mapDropNulls((child) => {
Expand Down Expand Up @@ -457,7 +463,7 @@ function walkConditionalClause(
const branch = conditionalCase === 'true-case' ? conditional.whenTrue : conditional.whenFalse

// Walk the clause of the conditional.
const clausePathTrees = Object.values(conditionalSubTree.children).filter((childPath) => {
const clausePathTrees = getElementPathTreeChildren(conditionalSubTree).filter((childPath) => {
if (isDynamic(childPath.path) && hasElementsWithin(branch)) {
for (const element of Object.values(branch.elementsWithin)) {
const firstChildPath = EP.appendToPath(EP.parentPath(clausePath), element.uid)
Expand Down Expand Up @@ -513,7 +519,7 @@ function walkMapExpression(
const commentFlag = findUtopiaCommentFlag(element.comments, 'map-count')

const mapCountOverride = isUtopiaPropOrCommentFlagMapCount(commentFlag) ? commentFlag.value : null
const mappedChildren = Object.values(subTree.children).map((child) =>
const mappedChildren = getElementPathTreeChildren(subTree).map((child) =>
createNavigatorSubtree(
metadata,
elementPathTrees,
Expand All @@ -531,7 +537,7 @@ function walkMapExpression(
}

let invalidEntries: Array<NavigatorTree> = []
for (let i = Object.values(subTree.children).length; i < mapCountOverride; i++) {
for (let i = getElementPathTreeChildren(subTree).length; i < mapCountOverride; i++) {
const entry = invalidOverrideNavigatorEntry(
EP.appendToPath(subTree.path, `invalid-override-${i + 1}`),
'data source not found',
Expand Down
6 changes: 3 additions & 3 deletions editor/src/components/navigator/navigator.spec.browser2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5495,16 +5495,14 @@ describe('Navigator row order', () => {
navigatorEntryToKey,
),
).toEqual([
'regular-sb/1e7',
'regular-sb/sc',
'regular-sb/sc/app',
'regular-sb/sc/app:app-root',
'regular-sb/sc/app:app-root/card',
'regular-sb/sc/app:app-root/card:card-root',
'regular-sb/sc/app:app-root/card:card-root/30d',
'regular-sb/sc/app:app-root/card:card-root/card-span',
'regular-sb/sc/app:app-root/card:card-root/30d',
'regular-sb/sc/app:app-root/card/card-child',
'regular-sb/sc/app:app-root/children-code-block',
'regular-sb/sc/app:app-root/frag',
'regular-sb/sc/app:app-root/frag/frag-child',
'regular-sb/sc/app:app-root/frag/cond-1',
Expand All @@ -5518,7 +5516,9 @@ describe('Navigator row order', () => {
'synthetic-sb/sc/app:app-root/frag/cond-1/cond-1-true/cond-2/d84-attribute',
'conditional-clause-sb/sc/app:app-root/frag/cond-1-false-case',
'synthetic-sb/sc/app:app-root/frag/cond-1/019-attribute',
'regular-sb/sc/app:app-root/children-code-block',
'regular-sb/sc/app/app-child',
'regular-sb/1e7',
])
})

Expand Down
14 changes: 14 additions & 0 deletions editor/src/core/model/element-metadata-utils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,7 @@ describe('getElementLabel', () => {
spanElementMetadata.elementPath,
EP.toString(spanElementMetadata.elementPath),
[],
[],
),
[EP.toString(divElementMetadata.elementPath)]: elementPathTree(
divElementMetadata.elementPath,
Expand All @@ -1110,8 +1111,10 @@ describe('getElementLabel', () => {
spanElementMetadata.elementPath,
EP.toString(spanElementMetadata.elementPath),
[],
[],
),
],
[],
),
}
it('the label of a spin containing text is that text', () => {
Expand All @@ -1138,16 +1141,19 @@ describe('getting the root paths', () => {
testComponentSceneElement.elementPath,
EP.toString(testComponentSceneElement.elementPath),
[],
[],
)
const testStoryboardChildTree = elementPathTree(
testStoryboardChildElement.elementPath,
EP.toString(testStoryboardChildElement.elementPath),
[],
[],
)
const storyboardTree = elementPathTree(
EP.elementPath([[BakedInStoryboardUID]]),
EP.toString(EP.elementPath([[BakedInStoryboardUID]])),
[testComponentSceneTree, testStoryboardChildTree],
[],
)
const pathTrees: ElementPathTrees = {
[EP.toString(EP.elementPath([[BakedInStoryboardUID]]))]: storyboardTree,
Expand All @@ -1170,41 +1176,49 @@ describe('getting the root paths', () => {
testComponentSceneChildElement.elementPath,
EP.toString(testComponentSceneChildElement.elementPath),
[],
[],
)
const testComponentChild1Tree = elementPathTree(
testComponentMetadataChild1.elementPath,
EP.toString(testComponentMetadataChild1.elementPath),
[],
[],
)
const testComponentChild2Tree = elementPathTree(
testComponentMetadataChild2.elementPath,
EP.toString(testComponentMetadataChild2.elementPath),
[],
[],
)
const testComponentChild3Tree = elementPathTree(
testComponentMetadataChild3.elementPath,
EP.toString(testComponentMetadataChild3.elementPath),
[],
[],
)
const testComponentRoot1Tree = elementPathTree(
testComponentRoot1.elementPath,
EP.toString(testComponentRoot1.elementPath),
[testComponentChild1Tree, testComponentChild2Tree, testComponentChild3Tree],
[],
)
const testComponentSceneTree = elementPathTree(
testComponentSceneElement.elementPath,
EP.toString(testComponentSceneElement.elementPath),
[testComponentSceneChildTree, testComponentRoot1Tree],
[],
)
const testStoryboardChildTree = elementPathTree(
testStoryboardChildElement.elementPath,
EP.toString(testStoryboardChildElement.elementPath),
[],
[],
)
const storyboardTree = elementPathTree(
EP.elementPath([[BakedInStoryboardUID]]),
EP.toString(EP.elementPath([[BakedInStoryboardUID]])),
[testComponentSceneChildTree, testStoryboardChildTree],
[],
)
const pathTrees: ElementPathTrees = {
[EP.toString(EP.elementPath([[BakedInStoryboardUID]]))]: storyboardTree,
Expand Down
19 changes: 13 additions & 6 deletions editor/src/core/model/element-metadata-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,15 @@ import {
import type { ProjectContentTreeRoot } from '../../components/assets'
import { memoize } from '../shared/memoize'
import type { ElementPathTree, ElementPathTrees } from '../shared/element-path-tree'
import { buildTree, getSubTree, getCanvasRoots, elementPathTree } from '../shared/element-path-tree'
import {
buildTree,
getSubTree,
getCanvasRoots,
elementPathTree,
getElementPathTreeChildren,
forEachElementPathTreeChild,
printTree,
} from '../shared/element-path-tree'
import type { PropertyControlsInfo } from '../../components/custom-code/code-file'
import { findUnderlyingTargetComponentImplementationFromImportInfo } from '../../components/custom-code/code-file'
import type {
Expand Down Expand Up @@ -323,7 +331,6 @@ export const MetadataUtils = {
if (target == null) {
return []
}

const parentPath = EP.parentPath(target)
const siblingPathsOrNull = EP.isRootElementOfInstance(target)
? MetadataUtils.getRootViewPathsOrdered(metadata, pathTree, parentPath)
Expand Down Expand Up @@ -724,7 +731,7 @@ export const MetadataUtils = {
if (subTree == null) {
return []
} else {
return subTree.children
return getElementPathTreeChildren(subTree)
.map((child) => child.path)
.filter((path) => !EP.isRootElementOfInstance(path))
}
Expand Down Expand Up @@ -845,7 +852,7 @@ export const MetadataUtils = {
let result: Array<ElementPath> = []
function recurseElement(tree: ElementPathTree): void {
result.push(tree.path)
fastForEach(Object.values(tree.children), (childTree) => {
forEachElementPathTreeChild(tree, (childTree) => {
recurseElement(childTree)
})
}
Expand Down Expand Up @@ -878,7 +885,7 @@ export const MetadataUtils = {
if (tree != null) {
result.push(tree.path)

fastForEach(Object.values(tree.children), (childTree) => {
forEachElementPathTreeChild(tree, (childTree) => {
recurseElement(childTree)
})
}
Expand Down Expand Up @@ -1375,7 +1382,7 @@ export const MetadataUtils = {

let unfurledComponents: Array<ElementPathTree> = []

fastForEach(Object.values(subTree.children), (child) => {
forEachElementPathTreeChild(subTree, (child) => {
if (EP.isRootElementOfInstance(child.path)) {
unfurledComponents.push(child)
} else {
Expand Down
Loading

0 comments on commit 73b4df7

Please sign in to comment.