fix: yield when 16ms has passed and no dom mutating tasks are pending#4033
fix: yield when 16ms has passed and no dom mutating tasks are pending#4033Madoshakalaka merged 4 commits intomasterfrom
Conversation
|
Visit the preview URL for this PR (updated for commit 46ce0d8): https://yew-rs-api--pr4033-fix-periodic-yield-kmvhcr0x.web.app (expires Sat, 14 Mar 2026 15:05:09 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Benchmark - coreYew MasterPull Request |
Size ComparisonDetails
|
Benchmark - SSRYew MasterDetails
Pull RequestDetails
|
|
@Ddystopia do you wanna try the |
Is it? I'm on board with doing this yield for user-defined and external tasks. But for anything inside of yew, yielding at an arbitrary point between updating a component and its children, or other point with the scheduler partially finished, means there could be an event handling interrupting it. The event handling part is currently unaware of this intermediate state and will run arbitrary used code that might interact with DOM at that point. |
packages/yew/src/scheduler.rs
Outdated
| } | ||
|
|
||
| fn run_scheduler(mut queue: Vec<super::QueueEntry>) { | ||
| let mut deadline = js_sys::Date::now() + YIELD_DEADLINE_MS; |
There was a problem hiding this comment.
an alternative is the Performance API which has μs resolution (as opposed to ms) and monotonicity, neither of which we really need.
real, how about yields only when the render queues are empty? |
I'm specifically talking about the closure that is used as the event handler which will get run immediately. Any change to a component including a re-render would go through the scheduler and run later. The event handler itself of course can not be delayed, since it might cancel the event which the browser must know about when the event handler returns. |
Even without yielding some actions trigger a forced reflow and dom changes, like setting a width to the node and then reading client rect. |
I can't run master due to some stale dependencies 😅 |
Yes, but I'm talking about a handler that expects the DOM to be in a state consistent with the last rendered vdom/component state, which might not be the case if it we yield while updating the node's children but after installing the handler itself. |
|
@WorldSEnder I'm thinking of only yielding when no DOM-mutating work is pending. Tracing through every
These touch the DOM (or are always paired with something that does). Not safe to yield while any of these have pending items.
Lifecycle callbacks or state updates that don't directly mutate the DOM. Safe to have pending at yield time. This means a single render cascade that exceeds 50ms still blocks (inherent limitation, can't split a parent/child render tree), but between independent update cycles we yield as intended. Thoughts? |
|
Sounds good to me. Any specific reason to use 50ms here, btw? I'd expected something close to either 30 or 60 fps so to speak (~33m and ~16ms respectively) but that's just a first naive thought. |
|
It's from the article https://web.dev/articles/optimize-long-tasks?utm_source=devtools
But since we are not yielding consistently, whatever we set will be an lower bound. 33ms or 16ms makes more sense I think. |
Madoshakalaka
force-pushed
the
fix/periodic-yield
branch
from
b802e59 to
86c2d0f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Isn't the issue that the test is wrong and expects everything to be settled after |
I was thinking of that actually. will #2679 help? |
That was an attempt at that, but abondoned for being too complex and also trying to integrate with playwright and actual e2e testing frameworks which was much more complicated than I first thought. |
Replaces the async/await yield mechanism (JsFuture + Promise) with a synchronous callback approach (Closure::once_into_js + setTimeout). This avoids pulling in the JsFuture state machine and web_sys::window() binding, cutting the wasm size overhead roughly in half (~0.9KB vs ~1.8KB).
Yield to the browser only when no DOM-mutating work (destroy, create, render_first, render, render_priority) is pending, so event handlers that fire during the yield never see a partially-rendered tree. Also lower yield deadline from 50ms to 16ms (~60fps) and gate start_now() to non-wasm/test targets where it is actually used.
Madoshakalaka
force-pushed
the
fix/periodic-yield
branch
from
86c2d0f to
62f78d7
Compare
Madoshakalaka
force-pushed
the
fix/periodic-yield
branch
from
5923b5e to
46ce0d8
Compare
|
rebased onto master and it seems the flush function worked wonders |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Madoshakalaka
changed the title
3 participants
Fixes #4032.
This makes the scheduler in the browser environment yield every ~16 ms
Concerns
- DOM may be partially updated when the browser paints during a yield. This is arguably better than the status quo, where the browser cannot paint at all until the entire batch completes, causing the page to appear frozen.task.run(). This is an inherent limitation -- individual component renders that are themselves expensive would need to be optimized at the component level.