-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Breadth First Subscriber Resolution #3219
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
Conversation
This PR is way too slow. My app's page maxes out the CPU & does not load. I'll leave it here for reference, however it looks like the naive approach is no bueno. |
The latest commit performs significantly better than the first. This PR should be ready to go. I'll be testing it using https://github.com/btakita/svelte/tree/gh-3191-hotfix |
It looks like this has no diamond dependencies consolidation for asynchronous derived stores, right? But is it even a big deal? Should diamond dependencies cause one update even for async deriveds? |
I've always had the understanding that the observers need to resolve their own diamond dependency state transitions by using guard conditionals. Breadth first subscriber resolution effectively enables automatic diamond dependency resolution for synchronous derived stores. Re: asynchronous stores, it would be nice to have diamond dependency resolution. Perhaps the derived([a, b], async ([$a, $b], set, invalidate) => {
invalidate()
const [$a_, $b_] = await Promise.all([a_fn($a), b_fn($b)])
set($a_ + $b_)
}) Calling the |
One could also do multi-step async state-machine flows with the derived([a, b], async ([$a, $b], set, invalidate) => {
invalidate()
const $a_ = await a_fn($a)
set({ step: 1, $a_ })
invalidate()
const $b_ = await b_fn($b)
set({ step: 2, $a_, $b_ }
}) |
Derived store reruns subscribers if it's value has not changed when synced. All invalidators of subscribers are run on a derived store when invalidated. See sveltejs#2955
Optimize performance by using subscriber_queue.
Thank you. Took me a long time to understand what was going on here, so I'm going to leave some notes to preserve my current state of enlightenment: The root issue here was that invalidating a This depth-first approach to invalidation also meant that invalidation was happening more frequently than necessary — if B depends on A and C depends on B, then changing A will invalidate B and C, then updating B will cause C to be invalidated again, harmlessly but pointlessly. Switching to a breadth-first approach fixes that; we only invalidate derived stores whose inputs have in fact changed. (Thanks for bearing with me during this episode of 'Rich learns algorithms' — I could have saved myself some mental effort if I'd just seen @mrkishi's comment sooner.) It does introduce a theoretical downside, in that this aspect of I made a small change — we can now return the I'm not certain if we want to handle the async case — it feels like adding a fair bit of complexity for somewhat incremental gains. At that point it might be better to reach for something more powerful than Svelte's built-in stuff. But we can discuss that on #3233. |
fixes #3191
All invalidators of subscribers are run on a derived store when invalidated.
See #2955