Skip to content

Commit

Permalink
Cache getComponentDescriptorForTarget (#6294)
Browse files Browse the repository at this point in the history
**Problem:**
In deriveState we check the autofocusability of all elements. To do that
we need to call getComponentDescriptorForTarget for each of them, which
is quite slow.
The component descriptor only depends on projectContents and
propertyControlsInfo, so we can use the new `valueDependentCache` to
cache this function. We just need to extend it to support a custom
equality function for the value.
With this change the time to check autofocusability of all elements in
derivestate is reduced from 1-2ms to ~0.3ms when there is not project
content change (e.g. during scrolling).

**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 3b7c954 commit a70170d
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 25 deletions.
8 changes: 4 additions & 4 deletions editor/src/components/inspector/inspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -977,10 +977,10 @@ function useShouldExpandStyleSection(): boolean {
Substores.propertyControlsInfo,
(store) => {
return store.editor.selectedViews.every((target) => {
const { propertyControlsInfo, projectContents } = store.editor
const descriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
target,
store.editor.propertyControlsInfo,
store.editor.projectContents,
)

if (descriptor == null || descriptor.inspector == null) {
Expand Down Expand Up @@ -1010,10 +1010,10 @@ function useShouldHideInspectorSections(): boolean {
Substores.propertyControlsInfo,
(store) =>
store.editor.selectedViews.some((target) => {
const { propertyControlsInfo, projectContents } = store.editor
const descriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
target,
store.editor.propertyControlsInfo,
store.editor.projectContents,
)

if (descriptor?.inspector == null) {
Expand Down
15 changes: 5 additions & 10 deletions editor/src/core/model/element-metadata-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1088,9 +1088,8 @@ export const MetadataUtils = {
return 'supportsChildren'
}
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
target,
propertyControlsInfo,
projectContents,
)
if (componentDescriptor != null && !componentDescriptor.supportsChildren) {
return 'doesNotSupportChildren'
Expand Down Expand Up @@ -2090,9 +2089,8 @@ export const MetadataUtils = {
projectContents: ProjectContentTreeRoot,
): boolean {
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
path,
propertyControlsInfo,
projectContents,
)
return (
componentDescriptor?.focus === 'always' ||
Expand Down Expand Up @@ -2129,9 +2127,8 @@ export const MetadataUtils = {
projectContents: ProjectContentTreeRoot,
): boolean {
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
path,
propertyControlsInfo,
projectContents,
)
if (componentDescriptor != null && componentDescriptor.focus !== 'default') {
return false
Expand Down Expand Up @@ -2168,9 +2165,8 @@ export const MetadataUtils = {
): Emphasis {
// Look up the emphasis of the component from the property controls.
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
path,
propertyControlsInfo,
projectContents,
)
if (componentDescriptor != null) {
return componentDescriptor.emphasis
Expand Down Expand Up @@ -2223,9 +2219,8 @@ export const MetadataUtils = {
projectContents: ProjectContentTreeRoot,
): Icon | null {
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
path,
propertyControlsInfo,
projectContents,
)
return componentDescriptor?.icon ?? null
},
Expand Down
3 changes: 1 addition & 2 deletions editor/src/core/model/element-template-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1523,9 +1523,8 @@ export function elementChildSupportsChildrenAlsoText(
propertyControlsInfo: PropertyControlsInfo,
): ElementSupportsChildren | null {
const componentDescriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
path,
propertyControlsInfo,
projectContents,
)
if (componentDescriptor != null && !componentDescriptor.supportsChildren) {
return 'doesNotSupportChildren'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,16 @@ export const App = (props) => {
it('works with the regular path', () => {
const projectContents = getProjectContents('regular-path')
const actualResult = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
EP.fromString(`sample-storyboard/sample-scene/sample-app:div/component`),
propertyControlsInfo,
projectContents,
)
expect(actualResult).toEqual(componentDescriptor)
})
it('works with a mapped path', () => {
const projectContents = getProjectContents('mapped-path')
const actualResult = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
EP.fromString(`sample-storyboard/sample-scene/sample-app:div/component`),
propertyControlsInfo,
projectContents,
)
expect(actualResult).toEqual(componentDescriptor)
})
Expand Down
29 changes: 25 additions & 4 deletions editor/src/core/property-controls/property-controls-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import type { Styling } from 'utopia-api'
import { StylingOptions } from 'utopia-api'
import { intersection } from '../shared/set-utils'
import { getFilePathMappings } from '../model/project-file-utils'
import { valueDependentCache } from '../shared/memoize'
import * as EP from '../shared/element-path'
import { shallowEqual } from '../shared/equality-utils'

export function propertyControlsForComponentInFile(
componentName: string,
Expand Down Expand Up @@ -135,10 +138,25 @@ export function getPropertyControlsForTarget(
)
}

export function getComponentDescriptorForTarget(
export const getComponentDescriptorForTarget = valueDependentCache(
getComponentDescriptorForTargetInner,
EP.toString,
{
equality: (s1, s2) =>
s1.projectContents === s2.projectContents &&
s1.propertyControlsInfo === s2.propertyControlsInfo,
},
)

function getComponentDescriptorForTargetInner(
{
projectContents,
propertyControlsInfo,
}: {
propertyControlsInfo: PropertyControlsInfo
projectContents: ProjectContentTreeRoot
},
target: ElementPath,
propertyControlsInfo: PropertyControlsInfo,
projectContents: ProjectContentTreeRoot,
): ComponentDescriptor | null {
const filePathMappings = getFilePathMappings(projectContents)
return withUnderlyingTarget(
Expand Down Expand Up @@ -236,7 +254,10 @@ export function getInspectorPreferencesForTargets(
projectContents: ProjectContentTreeRoot,
): InspectorSectionPreference[] {
const inspectorPreferences = targets.map((target) => {
const controls = getComponentDescriptorForTarget(target, propertyControlsInfo, projectContents)
const controls = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
target,
)
if (controls == null || controls.inspector == null) {
return { type: 'all' }
}
Expand Down
6 changes: 5 additions & 1 deletion editor/src/core/shared/memoize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@ export function memoize<T extends Moizeable>(func: T, options?: Partial<Options<
export function valueDependentCache<Value, Input, Result>(
fallback: (value: Value, input: Input) => Result,
inputToString: (input: Input) => string,
options?: {
equality?: (a: Value, b: Value) => boolean
},
): (value: Value, input: Input) => Result {
let cache: { [key: string]: Result } = {}
let lastSeenValue: Value | null = null
const eq = options?.equality ?? ((a, b) => a === b)
return (value: Value, input: Input) => {
const inputAsString = inputToString(input)
if (lastSeenValue == null || lastSeenValue !== value) {
if (lastSeenValue == null || !eq(lastSeenValue, value)) {
// Either this is the first use of the function, or the value has changed.
lastSeenValue = value
const result = fallback(value, input)
Expand Down

0 comments on commit a70170d

Please sign in to comment.