-
Notifications
You must be signed in to change notification settings - Fork 120
Implement incremental style traversal with dirty descendants tracking #332
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
Implement incremental style traversal with dirty descendants tracking #332
Conversation
nicoburns
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joeleaver Thanks for the PR. This is a nice find!
I've tested this and I've noticed that:
- In cases where it helps, it can make a big difference.
- In some cases it makes no difference at all.
I've been testing with https://en.wikipedia.org/wiki/United_States (just incr https://en.wikipedia.org/wiki/United_States from the repo root (using https://github.com/casey/just)) and window size large enough to show the "desktop" version of the site (this uses a --release build of Blitz).
With main:
- Moving the mouse over text in the "main body" of the article causes style passes of ~3-5ms
- Moving the mouse from the "main body" into the "gutter" to the side causes style passes of up to ~50ms
With this PR:
- The time for style passes when moving the mouse over text in the "main body" of the article is reduced to ~100µs
- The time for style passes when moving the mouse from the "main body" into the "gutter" to the side remains ~50ms
So it seems like a good improvement, but there are still slow cases. Also of note: in the newly fast cases, the overall "resolve" (style/layout) time is now dominated by the damage propagation pass! Something to improve upon in future
I'd be interested to know what you're testing with where you are getting ~20-30ms style passes (and are you testing with debug or release builds?). https://en.wikipedia.org/wiki/United_States is a very large page (~20k DOM nodes), and I wouldn't expect style passes to be that slow for a typical view in an app which I'd expect to me much simpler.
Finally, I think there are several additional places where we will need to set dirty_descedants to true. Basically everywhere where we manipulate the DOM tree and are setting a RestyleHint in mutator.rs. This kind of fine-grained dirty tracking is always tricky to get right, but I think if we handle those cases then we can probably land this.
The Blitz TodoMVC example can be used to test this ( |
e66a459 to
500afe5
Compare
Add proper dirty_descendants flag tracking to enable Stylo's incremental style traversal to skip unchanged subtrees. Previously, has_dirty_descendants() always returned true, causing the style traversal to visit every node in the DOM on every resolve() call. Changes: - Add dirty_descendants: AtomicBool field to Node struct - Implement has_dirty_descendants(), set_dirty_descendants(), unset_dirty_descendants() methods on Node - Add mark_ancestors_dirty() to propagate dirty flags up the tree - Update TElement trait implementation to use real dirty flag values - Call mark_ancestors_dirty() when set_restyle_hint() is called - Clear dirty_descendants flags after style resolution (with incremental feature) This optimization allows the style traversal to skip entire subtrees that haven't changed, significantly reducing style resolution time for incremental updates. For example, hovering over a single element no longer requires restyling the entire DOM. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This adds mark_ancestors_dirty() calls to all DOM mutation methods that set RestyleHint or damage, ensuring the style traversal visits modified subtrees. Without these calls, the style traversal may skip nodes with pending RestyleHint/damage because it uses dirty_descendants flags to determine which subtrees to visit. This caused issues like: - DOM mutations via set_attribute/set_node_text/clear_attribute not resulting in visual updates during continuous operations (e.g. slider dragging) - New elements added to the DOM being initially unstyled until some other event triggered a restyle Methods updated: - set_node_text() - set_attribute() - clear_attribute() - remove_node() - remove_and_drop_node() - remove_and_drop_all_children() - add_children_to_parent() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
28df861 to
2d25cdb
Compare
Summary
This PR implements proper dirty descendants flag tracking to enable Stylo's incremental style traversal to skip unchanged subtrees.
Previously,
has_dirty_descendants()always returnedtrue, causing the style traversal to visit every node in the DOM on everyresolve()call.Changes
Core Infrastructure (
node.rs,stylo.rs,resolve.rs):dirty_descendants: AtomicBoolfield to Node structhas_dirty_descendants(),set_dirty_descendants(),unset_dirty_descendants()methodsmark_ancestors_dirty()to propagate dirty flags up the treemark_ancestors_dirty()whenset_restyle_hint()is calleddirty_descendantsflags after style resolution (with incremental feature)Mutation Methods (
mutator.rs):Added
mark_ancestors_dirty()calls to all DOM mutation methods that set RestyleHint or damage:set_node_text()- afterinsert_damage()set_attribute()- after setting hint/damage on node and parentclear_attribute()- after setting hint/damageremove_node()- on parent when removing childremove_and_drop_node()- on parent when in documentremove_and_drop_all_children()- on parent when in documentadd_children_to_parent()- on both new parent and old parent when in documentWhy All Mutation Methods Need This
The Stylo style traversal uses the
dirty_descendantsflag on ancestor nodes to determine which subtrees need restyling. When a node needs restyling, the traversal walks down from the root but only descends into subtrees where an ancestor hasdirty_descendants = true.Without
mark_ancestors_dirty()calls in the mutation methods, DOM changes viaset_attribute,set_node_text, tree mutations, etc. would set the correct damage/hint flags on nodes, but the traversal would skip those nodes entirely because their ancestors didn't havedirty_descendantsset.This caused issues like:
Performance Results
Before (always visiting all nodes):
After (with dirty descendants tracking):
Note: As @nicoburns observed, some cases (like moving from main body to gutter on Wikipedia) still show ~50ms style passes. This appears to be related to sibling/pseudo-class selector invalidation triggering large restyle scopes, which is a separate optimization opportunity.
Test Plan
🤖 Generated with Claude Code