Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canvas Performance: severe problems with dom-walker, MutationObserver and ResizeObserver #6067

Closed
balazsbajorics opened this issue Jul 11, 2024 · 3 comments
Assignees
Labels
Blocked Canvas Affects the visual editor Nontrivial Requires invention / discovery to solve Performance

Comments

@balazsbajorics
Copy link
Contributor

balazsbajorics commented Jul 11, 2024

When dragging the border radius slider of the PromiseCard component, we can see egregious performance:

Image

The entire render would be bad enough at 55ms, but we accidentally run it twice because an activating MutationObserver and ResizeObserver.

These observers do not create accidental re-renders during on-canvas interactions, which is where we tested them, so that's good.

BUT the entire system is not fine grained and were designed in a more naive time:
1: we invalidate the entire scene, not just the element
2: it's not controlled when should this dom-walker run happen then
3: we should either rely on the observers OR on the canvas interaction, but not both
4: the dom-walker should never walk entire subtrees during interactions

Possible ideas for building a better solution:

  • Rheese proposed that we should store in the jsxMetadata a flag for the metadata staleness.
  • we could try a model where we more bravely let a lot of metadata remain stale during interactions: 99% of canvas interactions only need to know metadata of the same few elements: the selected element, its parent, siblings, children, and what is under the mouse. We could cut a ton of CPU cycles spent on creating and maintaining the metadata for hundreds of other elements
  • the ReconstructJsxMetadata is pretty expensive, we should refactor the metadata so that we only need to cheaply join the SpyMetadata and the DomMetadata, but without the need to create a bunch of new objects. Today the spyMetadata creates an entire fake object for specialSizeMeasurements etc, all of this is just putting load on the garbage collector.
  • Move jsxMetadata out of EditorState, possibly into the editorStore root, so we can more clearly treat it as its own thing.
  • With React flushSync in the mix, it seems like we -could- create a new approach that relies purely on the mutation observer, but this needs investigation
  • the actual dom-walker run time and performance should be investigated too.

** TLDR end goal is that during an interaction, we really only call getComputedStyle for a handful of elements, and spend almost zero time on merging large jsxMetadata trees etc. ideally ALL OF the metadata update should take 0.1-0.2ms per frame instead of 7-10ms of today**

@balazsbajorics balazsbajorics added Canvas Affects the visual editor Performance Nontrivial Requires invention / discovery to solve labels Jul 11, 2024
@balazsbajorics balazsbajorics changed the title Canvas Performance: severe problems with dom-walker MutationObserver and ResizeObserver Canvas Performance: severe problems with dom-walker, MutationObserver and ResizeObserver Jul 11, 2024
@balazsbajorics balazsbajorics self-assigned this Jul 15, 2024
@Rheeseyb
Copy link
Contributor

Rheeseyb commented Sep 3, 2024

@balazsbajorics Is this ticket still relevant?

@maltenuhn
Copy link
Member

Note: for labels scrubbing, the performance (at least in vanilla project) is now more than adequate (prob result of #6196 and other performance improvements)

@maltenuhn
Copy link
Member

Turns out this actually done not blocked; closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Canvas Affects the visual editor Nontrivial Requires invention / discovery to solve Performance
Projects
None yet
Development

No branches or pull requests

3 participants