From 66396f391ee109af8420882dd89be658ac6ce9d2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:28:16 -0400 Subject: [PATCH 1/9] fix: only abort effect flushing if it causes an existing effect to be scheduled --- .changeset/clever-months-clap.md | 5 +++++ .../svelte/src/internal/client/reactivity/batch.js | 14 +++++--------- .../src/internal/client/reactivity/sources.js | 6 ++++++ 3 files changed, 16 insertions(+), 9 deletions(-) create mode 100644 .changeset/clever-months-clap.md diff --git a/.changeset/clever-months-clap.md b/.changeset/clever-months-clap.md new file mode 100644 index 000000000000..9566dcf54d66 --- /dev/null +++ b/.changeset/clever-months-clap.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: only abort effect flushing if it causes an existing effect to be scheduled diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 123bc95d163a..f4ba4b54b61f 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -28,7 +28,7 @@ import * as e from '../errors.js'; import { flush_tasks } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { old_values } from './sources.js'; +import { old_values, schedule_version } from './sources.js'; import { unlink_effect } from './effects.js'; import { unset_context } from './async.js'; @@ -598,7 +598,7 @@ function flush_queued_effects(effects) { var effect = effects[i++]; if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) { - var n = current_batch ? current_batch.current.size : 0; + var sv = schedule_version; update_effect(effect); @@ -619,13 +619,9 @@ function flush_queued_effects(effects) { } } - // if state is written in a user effect, abort and re-schedule, lest we run - // effects that should be removed as a result of the state change - if ( - current_batch !== null && - current_batch.current.size > n && - (effect.f & USER_EFFECT) !== 0 - ) { + // if an effect is invalidated by a user effect, abort and re-schedule, lest we + // run effects that should be removed as a result of the state change + if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { break; } } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7fb3135708d3..d2a57e1acfe3 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -299,6 +299,11 @@ export function increment(source) { set(source, source.v + 1); } +/** + * We increment this value when an effect is scheduled as a result of a state change + */ +export let schedule_version = 0; + /** * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY @@ -334,6 +339,7 @@ function mark_reactions(signal, status) { if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); } else if (not_dirty) { + schedule_version += 1; schedule_effect(/** @type {Effect} */ (reaction)); } } From 2048f5dae0168ff53391d66f4a229b523fda23dd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:35:51 -0400 Subject: [PATCH 2/9] tweak comment --- packages/svelte/src/internal/client/reactivity/batch.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f4ba4b54b61f..8bdb12007df5 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -619,8 +619,8 @@ function flush_queued_effects(effects) { } } - // if an effect is invalidated by a user effect, abort and re-schedule, lest we - // run effects that should be removed as a result of the state change + // if a state change in a user effect invalidates a _different_ effect, + // abort and reschedule in case that effect now needs to be destroyed if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { break; } From acc7bb3f7f203b10a09968119e6d8c0159b21d24 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:48:17 -0400 Subject: [PATCH 3/9] add test --- .../samples/effect-loop-3/Child.svelte | 11 +++++++++++ .../samples/effect-loop-3/_config.js | 12 ++++++++++++ .../samples/effect-loop-3/main.svelte | 15 +++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte new file mode 100644 index 000000000000..71588c6ee8c3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte @@ -0,0 +1,11 @@ + + + {@render children()} diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js new file mode 100644 index 000000000000..046c1904322a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js @@ -0,0 +1,12 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [button] = target.querySelectorAll('button'); + + assert.doesNotThrow(() => { + flushSync(() => button.click()); + }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte new file mode 100644 index 000000000000..2b3a17179806 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte @@ -0,0 +1,15 @@ + + + + +{#if show} + {#each { length: 1234 } as i} + {i} + {/each} +{/if} From f89e3557eef4a2d3feb849c0e8cb39e9e8c08b7e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 22:54:01 -0400 Subject: [PATCH 4/9] restrict to block effects --- packages/svelte/src/internal/client/reactivity/sources.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index d2a57e1acfe3..93f4cdaddbe0 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -300,7 +300,8 @@ export function increment(source) { } /** - * We increment this value when an effect is scheduled as a result of a state change + * We increment this value when a block effect is scheduled as a result of a state change, + * as its currently-scheduled child effects may need to be destroyed */ export let schedule_version = 0; @@ -339,7 +340,7 @@ function mark_reactions(signal, status) { if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); } else if (not_dirty) { - schedule_version += 1; + if ((flags & BLOCK_EFFECT) !== 0) schedule_version += 1; schedule_effect(/** @type {Effect} */ (reaction)); } } From 12ceae113d3ce9fe05e876dc1773cf53135fbc32 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 20:17:25 -0400 Subject: [PATCH 5/9] make test fail --- .../tests/runtime-runes/samples/effect-loop-3/Child.svelte | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte index 71588c6ee8c3..9bf4db52d6b4 100644 --- a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte @@ -8,4 +8,6 @@ }); - {@render children()} +{#if inited} + {@render children()} +{/if} From d5ba21f77cae226de3ceb0073e208b8c796ed32b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 20:32:37 -0400 Subject: [PATCH 6/9] collect all effects --- .../svelte/src/internal/client/reactivity/batch.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 8bdb12007df5..b109c792e8b0 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -292,12 +292,12 @@ export class Batch { if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; + } else if ((flags & EFFECT) !== 0) { + this.#effects.push(effect); + } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { + this.#render_effects.push(effect); } else if ((flags & CLEAN) === 0) { - if ((flags & EFFECT) !== 0) { - this.#effects.push(effect); - } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { - this.#render_effects.push(effect); - } else if ((flags & ASYNC) !== 0) { + if ((flags & ASYNC) !== 0) { var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects; effects.push(effect); } else if (is_dirty(effect)) { From c9b2654357b0b4bb107f7236f4d821452a0c03d9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 20:43:51 -0400 Subject: [PATCH 7/9] WIP --- .../src/internal/client/reactivity/batch.js | 17 ++++++++++++++++- .../src/internal/client/reactivity/sources.js | 10 ++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index b109c792e8b0..d569ef26ecb1 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -584,6 +584,9 @@ function infinite_loop_guard() { } } +/** @type {Effect[] | null} */ +export let eager_block_effects = null; + /** * @param {Array} effects * @returns {void} @@ -600,6 +603,8 @@ function flush_queued_effects(effects) { if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) { var sv = schedule_version; + eager_block_effects = []; + update_effect(effect); // Effects with no dependencies or teardown do not get added to the effect tree. @@ -619,14 +624,24 @@ function flush_queued_effects(effects) { } } + if (eager_block_effects.length > 0) { + for (const e of eager_block_effects) { + update_effect(e); + } + + eager_block_effects = []; + } + // if a state change in a user effect invalidates a _different_ effect, // abort and reschedule in case that effect now needs to be destroyed if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { - break; + // break; } } } + eager_block_effects = null; + while (i < length) { schedule_effect(effects[i++]); } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 93f4cdaddbe0..d8a30d580d85 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -33,7 +33,7 @@ import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { get_stack, tag_proxy } from '../dev/tracing.js'; import { component_context, is_runes } from '../context.js'; -import { Batch, schedule_effect } from './batch.js'; +import { Batch, eager_block_effects, schedule_effect } from './batch.js'; import { proxy } from '../proxy.js'; import { execute_derived } from './deriveds.js'; @@ -340,7 +340,13 @@ function mark_reactions(signal, status) { if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); } else if (not_dirty) { - if ((flags & BLOCK_EFFECT) !== 0) schedule_version += 1; + if ((flags & BLOCK_EFFECT) !== 0) { + if (eager_block_effects !== null) { + eager_block_effects.push(/** @type {Effect} */ (reaction)); + } + schedule_version += 1; + } + schedule_effect(/** @type {Effect} */ (reaction)); } } From 5d6b0817478a1ef1a45ba399fd9232bfe00f1dff Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 21:08:47 -0400 Subject: [PATCH 8/9] run rescheduled block effects eagerly --- .../src/internal/client/dom/operations.js | 2 ++ .../src/internal/client/reactivity/batch.js | 17 ++++------------- .../src/internal/client/reactivity/sources.js | 7 ------- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index fb269e47e0fe..abc29a7670cb 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -6,6 +6,7 @@ import { get_descriptor, is_extensible } from '../../shared/utils.js'; import { active_effect } from '../runtime.js'; import { async_mode_flag } from '../../flags/index.js'; import { TEXT_NODE, EFFECT_RAN } from '#client/constants'; +import { eager_block_effects } from '../reactivity/batch.js'; // export these for reference in the compiled code, making global name deduplication unnecessary /** @type {Window} */ @@ -214,6 +215,7 @@ export function clear_text_content(node) { */ export function should_defer_append() { if (!async_mode_flag) return false; + if (eager_block_effects !== null) return false; var flags = /** @type {Effect} */ (active_effect).f; return (flags & EFFECT_RAN) !== 0; diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index d569ef26ecb1..60fa03c56cd2 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -28,7 +28,7 @@ import * as e from '../errors.js'; import { flush_tasks } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { old_values, schedule_version } from './sources.js'; +import { old_values } from './sources.js'; import { unlink_effect } from './effects.js'; import { unset_context } from './async.js'; @@ -601,8 +601,6 @@ function flush_queued_effects(effects) { var effect = effects[i++]; if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) { - var sv = schedule_version; - eager_block_effects = []; update_effect(effect); @@ -625,26 +623,19 @@ function flush_queued_effects(effects) { } if (eager_block_effects.length > 0) { + // TODO this feels incorrect! it gets the tests passing + old_values.clear(); + for (const e of eager_block_effects) { update_effect(e); } eager_block_effects = []; } - - // if a state change in a user effect invalidates a _different_ effect, - // abort and reschedule in case that effect now needs to be destroyed - if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { - // break; - } } } eager_block_effects = null; - - while (i < length) { - schedule_effect(effects[i++]); - } } /** diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index d8a30d580d85..cd0c28016dc5 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -299,12 +299,6 @@ export function increment(source) { set(source, source.v + 1); } -/** - * We increment this value when a block effect is scheduled as a result of a state change, - * as its currently-scheduled child effects may need to be destroyed - */ -export let schedule_version = 0; - /** * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY @@ -344,7 +338,6 @@ function mark_reactions(signal, status) { if (eager_block_effects !== null) { eager_block_effects.push(/** @type {Effect} */ (reaction)); } - schedule_version += 1; } schedule_effect(/** @type {Effect} */ (reaction)); From e7cc0e0d6cc6bb80c462da92b2c4a34b6bd26bff Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sun, 17 Aug 2025 14:30:50 +0200 Subject: [PATCH 9/9] Update .changeset/clever-months-clap.md --- .changeset/clever-months-clap.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/clever-months-clap.md b/.changeset/clever-months-clap.md index 9566dcf54d66..4a1b69e824a6 100644 --- a/.changeset/clever-months-clap.md +++ b/.changeset/clever-months-clap.md @@ -2,4 +2,4 @@ 'svelte': patch --- -fix: only abort effect flushing if it causes an existing effect to be scheduled +perf: run blocks eagerly during flush instead of aborting