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

Dev performance reduced by add_owner_to_object calls when using a $state.raw and a context created by a function #15072

Open
thes01 opened this issue Jan 21, 2025 · 2 comments

Comments

@thes01
Copy link

thes01 commented Jan 21, 2025

Describe the bug

This issue happens in the development mode only. I believe this bug is somewhat related to #14491, respectively to the fix #14533, which helps with this case for classes, but not functions.

Steps to reproduce:

  • set a context that is a POJO with a $state.raw field (e.g. by using a factory function)
  • fill the $state.raw with something sufficiently deep
  • have multiple child components mount and call getContext to retrieve the context (same symptom as in the referenced PR)

Reproduction

https://svelte.dev/playground/ab1f18a235f44df28eae50837df5bc1d?version=5.19.0

The Svelte REPL apparently cannot build the component in a dev mode, therefore you need to run it in localhost. What I see in the devtool performance monitor is something like this:

Image

Looking at the getContext implementation, it is understandable why this happens. The add_owner_to_object simply recurses into the fields of the state. I hope this could be addressed so that people who don't use classes (for whatever reason) aren't surprised by this.

Logs

System Info

System:
    OS: Windows 11 10.0.26100
    CPU: (16) x64 AMD Ryzen 7 PRO 6850U with Radeon Graphics     
    Memory: 3.64 GB / 14.76 GB
  Binaries:
    Node: 22.9.0 - ~\AppData\Local\fnm_multishells\23524_1737450727692\node.EXE
    Yarn: 1.22.22 - ~\AppData\Local\fnm_multishells\23524_1737450727692\yarn.CMD
    npm: 10.9.0 - ~\AppData\Local\fnm_multishells\23524_1737450727692\npm.CMD
  Browsers:
    Edge: Chromium (131.0.2903.146)
    Internet Explorer: 11.0.26100.1882

Severity

annoyance

@thes01
Copy link
Author

thes01 commented Jan 21, 2025

We've also seen the add_owner_to_object happen when using useThrelte from Threlte 8. I looked up the implementation and it also uses the factory function pattern, where nothing prevents the deep recursion.

Image

@dummdidumm
Copy link
Member

The problem with this is that getContext calls add_owner so that mutations coming from stuff passed through getContext are fine. Since it's $state.raw the algorithm doesn't stop traversal at any point, going through the whole object which is costly - and then it does that for each entry.

Possible solutions:

  • do widening at setContext ("everything is allowed for this thing"). Could mean false negatives when you also pass it as a prop that you don't bind but that's likely rare. Could mean worse perf in other places because it's not lazy (theoretically you can never do getContext, but again that's likely also rare)
  • do something with WeakMap to save traversed trees that didn't turn up any $state to add ownership to. Results in less runs at the cost of potentialyl higher memory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants