Skip to content

Svelte 5 performance of style and class directives #12652

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

Closed
peterkogo opened this issue Jul 29, 2024 · 0 comments · Fixed by #12654
Closed

Svelte 5 performance of style and class directives #12652

peterkogo opened this issue Jul 29, 2024 · 0 comments · Fixed by #12654
Milestone

Comments

@peterkogo
Copy link

Describe the bug

I am having a hard time optimizing a library that renders many components to be on par with its Svelte 4 version.
I have identified style and class directives playing a substantial part in this.

This REPL illustrates what I would like to achieve: Render many components based on a frozen state of an array of objects (using $state.frozen(array) for performance purposes) and each with a couple of attributes that effect styling and classes.

The issue

  1. All style and class directives are rerun regardless if the respective values change
  2. Setting a style directive calls getProperty, not only setProperty
  3. Might be unrelated: Update function of actions rerun on every update

Style and class directives

Here is a REPL demonstrating this issue: Check the flame graph in the performance tab of your browser after hitting 'change frozen stuff' a couple of times. Nothing is changing inside the immutable objects, but most time is spent on toggling classes, as well as reading and setting style properties that haven't changed.

setProperty calls

Additionally, in comparison to Svelte 4 simply calling setProperty when updating a style directive, Svelte 5 calls getProperty before. This more than doubles the time that is being spent on setting styles. I can imagine this provides some benefits, I would like to mention it, nonetheless, because it has quite the substantial impact for my use case. Think: rendering draggable elements with a transform.

Actions are updated on every re-render

I'm happy to create a separate issue for this if it turns out to be unrelated.

For possibly similar reasons, another issue is contributing to my performance problems.

Take this simple action

function someAction(domNode, id) {
  return {
    update(id) {
         console.log('new id', id)
     }
   }
}

The update function is rerun on every re-render even though the id does not change. See this REPL for a reproduction. (Hit the button and observe console output)

Notes

I realize this is an excessive number of elements — however, I run into this in production with mere hundreds of more complex components, and it worsens depending on the number of style and class directives. In particular, this heavily effects draggable elements with a number of optional class directives, where updates should run 60 to 120 times a second. Plus, Svelte 4 isn't even breaking a sweat in the same situation. (REPL if anyone wants to play around)

I would like to stress that this issue isn't some kind of micro-benchmark. It substantially impacts the Svelte 5 version of Svelte Flow. For comparison: dragging 600 nodes & edges takes 10ms in Svelte 4 and around 45ms in Svelte 5. The latter is completely rewritten in Svelte 5 & runs in Runes mode.

Reproduction

See above

Logs

No response

System Info

System:
    OS: macOS 14.5
    CPU: (11) arm64 Apple M3 Pro
    Memory: 153.52 MB / 18.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v20.10.0/bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: 9.4.0 - ~/.nvm/versions/node/v20.10.0/bin/pnpm
    bun: 1.1.12 - ~/.bun/bin/bun
  Browsers:
    Chrome: 126.0.6478.185
    Safari: 17.5
  npmPackages:
    rollup: ^3.23.0 => 3.23.0

Severity

annoyance

@dummdidumm dummdidumm added this to the 5.0 milestone Jul 29, 2024
dummdidumm added a commit that referenced this issue Jul 29, 2024
- check if we really need to add/remove the class (calling `includes` first is cheaper than always setting/removing it)
- check if we really need to update a style (calling `getPropertyValue/setProperty` is expensive)
- check if we should call the action's update function (this is not only a perf tweak but also a correctness fix)

closes #12652
dummdidumm added a commit that referenced this issue Jul 29, 2024
- check if we really need to add/remove the class (calling `includes` first is cheaper than always setting/removing it)
- check if we really need to update a style (calling `getPropertyValue/setProperty` is expensive)
- check if we should call the action's update function (this is not only a perf tweak but also a correctness fix)

closes #12652
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

Successfully merging a pull request may close this issue.

2 participants