perf: avoid Rc refcount overhead in DomSlot chain traversal#4088
perf: avoid Rc refcount overhead in DomSlot chain traversal#4088Madoshakalaka wants to merge 2 commits intomasterfrom
Conversation
Traverse the DynamicDomSlot chain using raw pointers instead of Rc::clone/drop per hop. The chain is transitively kept alive by the borrowed &self, so raw pointer access is sound.
|
Visit the preview URL for this PR (updated for commit c2baa85): https://yew-rs-api--pr4088-perf-domslot-path-co-hpigyof4.web.app (expires Mon, 06 Apr 2026 08:36:23 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Benchmark - coreYew MasterPull Request |
Size ComparisonDetails
✅ None of the examples has changed their size significantly. |
Benchmark - SSRYew MasterDetails
Pull RequestDetails
|
|
benchmarks looking good 0454fc7#commitcomment-180899917 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
To clarify, it helps with the performance but doesn't actually address the original comment. The 𝒪(𝑁²) comes from the chain structure itself: child[0] chains through 𝑁-1 hops, child[1] through 𝑁-2, etc. To eliminate this, I explored a possibility in #4090 briefly but couldn't make it work correctly |
|
What you would need is a (self-balancing) tree structure, such as an AVL or Splay (not sure what the best here is - depends on usage I suppose). You need to be able to split and merge the tree (when you reassign a node in the middle) and find the right-most node starting from any node in the middle. Worst case could be O(log n) and most likely even close to O(1) if you splay correctly. I never did get it to work either though when I tried. Something about the intrusive nature of parent pointers in trees and the balancing operations needed and I didn't find an existing impl that fit to not have to invent it here. EDIT: "worst case" should probably be "amortized" in the above, a true online worst case structure would be more complicated, see e.g. Scapegoat tree or Multi-Splay tree. It really does get hairy! |
| let cell = unsafe { &*ptr }; | ||
| let slot_ref = cell.borrow(); | ||
| match &slot_ref.variant { | ||
| DomSlotVariant::Node(ref n) => break f(n.as_ref()), |
There was a problem hiding this comment.
Calling the arbitrary function f invalidates the safety claim made above, since this f can indeed - in theory - access any node on the chain by capturing a reference to any of the Rc's therein.
Don't get me wrong, I don't immediately see any code doing this in this file. In practice, we do call gloo::console::error - which can call out to javascript - we run code of a tracing subscriber, we might panic and we might run some drop glue that I'm unaware of. If not now, then perhaps after a refactor. In any case the current API here is NOT safe.
The traversal is fine, but I think in general the refcount increase of the last Rc - which needs to exist and contain a borrowed cell during the call to f - needs to happen. Or, you could clone the Node and call by value.
|
Actually, the current structure might not actually form a simple chain, but a tree. You can tell multiple |
2 participants
This addresses a TODO left by @WorldSEnder
with_next_siblingis called on everyDomSlot::insert, which backs every DOM node insertion.For a
BListof N component children processed right-to-left, the total chain hops across allinsertions is 1 + 2 + ... + N = O(N^2). Reducing the constant factor per hop directly improves
large-list creation and reconciliation.