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

xilem_web: Optimize modifiers (allocs mostly), and fix hydrating SVG elements #620

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

Philipp-M
Copy link
Contributor

With the risk of wasting time with micro-optimizations...

Honestly the diff bigger and more complex than I would like it to be for the effect that this change does, so I'm sorry to reviewers.
FWIW, I tested all the examples with the feature "hydration" disabled and enabled.

In effect this reduces the amount of allocs and the size of allocs of all the modifiers (since Vec allocs at least 4 elements) by:

  • Adding a size hint in modifier views, to give the actual element props a hint how much memory is needed in the relevant modifier, so that only one allocation occurs when the actual modifier is added/created.
  • Using bitflags encoded in the start_idx to avoid extra 4 bytes for just two booleans in each modifier, and also use u16 instead of usize to save another 4 bytes.
  • Avoiding updated_modifiers allocations when building the element (with the CREATING bitflag).
  • Reduce allocations when rebuilding, by skipping non-changed modifiers (before this, Cow was cloned, which for example with a String as an attribute value resulted in an additional allocation).

This minimally increases the wasm binary size (though I'm pretty sure in a constant fashion, roughly 500bytes compressed) in exchange for faster creation of elements and less memory usage (in the 10k js-framework-bench roughly 10% less memory usage, and 7-8% faster).

I've also noticed that all the kurbo SVG views weren't hydrated yet, which I think results in buggy behavior when using it e.g. in a Templated view, this is kind of drive-by fix (which I didn't want to separate because of additional work...)

Copy link
Contributor

@flosse flosse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit that I didn't understand all of the content, but at least I didn't notice any obvious mistakes.

@flosse flosse added the web label Oct 1, 2024
@Philipp-M
Copy link
Contributor Author

Thanks, yeah it's probably not straight-forward, I hope to simplify parts of that in a future PR though, maybe even generalize this (or another) kind of modifier-system (maybe even in xilem_core), as I noticed that xilem native could also benefit from it (see also #591 (comment)).

@Philipp-M Philipp-M added this pull request to the merge queue Oct 1, 2024
Merged via the queue into linebender:main with commit 88ac491 Oct 1, 2024
17 checks passed
@Philipp-M Philipp-M deleted the xilem_web-optimize-modifiers branch October 1, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants