From da4509c96e68811294b1a3d55973f383361520ae Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 30 May 2024 01:32:10 +0100 Subject: [PATCH 1/6] fix: improve controlled each block cleanup performance --- .changeset/gentle-ties-fetch.md | 5 +++++ .../svelte/src/internal/client/dom/blocks/each.js | 11 +++++++---- .../src/internal/client/dom/blocks/svelte-element.js | 2 +- .../svelte/src/internal/client/reactivity/deriveds.js | 2 +- .../svelte/src/internal/client/reactivity/effects.js | 11 ++++++----- packages/svelte/src/internal/client/runtime.js | 7 ++++--- 6 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 .changeset/gentle-ties-fetch.md diff --git a/.changeset/gentle-ties-fetch.md b/.changeset/gentle-ties-fetch.md new file mode 100644 index 000000000000..830278581134 --- /dev/null +++ b/.changeset/gentle-ties-fetch.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improve controlled each block cleanup performance diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 8c59e3aebab3..48ea57f91433 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -68,17 +68,20 @@ function pause_effects(items, controlled_anchor, callback) { pause_children(items[i].e, transitions, true); } + var is_controlled = length > 0 && transitions.length === 0 && controlled_anchor !== null; // If we have a controlled anchor, it means that the each block is inside a single // DOM element, so we can apply a fast-path for clearing the contents of the element. - if (length > 0 && transitions.length === 0 && controlled_anchor !== null) { - var parent_node = /** @type {Element} */ (controlled_anchor.parentNode); + if (is_controlled) { + var parent_node = /** @type {Element} */ ( + /** @type {Element} */ (controlled_anchor).parentNode + ); clear_text_content(parent_node); - parent_node.append(controlled_anchor); + parent_node.append(/** @type {Element} */ (controlled_anchor)); } run_out_transitions(transitions, () => { for (var i = 0; i < length; i++) { - destroy_effect(items[i].e); + destroy_effect(items[i].e, is_controlled); } if (callback !== undefined) callback(); diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index df607fe54a58..a57d2cc519c4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -95,7 +95,7 @@ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, locat resume_effect(effect); } else { // tag is changing — destroy immediately, render contents without intro transitions - destroy_effect(effect); + destroy_effect(effect, false); set_should_intro(false); } } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 6ebd945301d1..b22feafaa650 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -71,7 +71,7 @@ export function derived_safe_equal(fn) { * @returns {void} */ function destroy_derived_children(signal) { - destroy_effect_children(signal); + destroy_effect_children(signal, false); var deriveds = signal.deriveds; if (deriveds !== null) { diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 6411e8109af7..9c401e02b50c 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -198,7 +198,7 @@ export function user_pre_effect(fn) { export function effect_root(fn) { const effect = create_effect(ROOT_EFFECT, fn, true); return () => { - destroy_effect(effect); + destroy_effect(effect, false); }; } @@ -311,16 +311,17 @@ export function execute_effect_teardown(effect) { /** * @param {import('#client').Effect} effect + * @param {boolean} skip_remove_dom * @returns {void} */ -export function destroy_effect(effect) { +export function destroy_effect(effect, skip_remove_dom) { var dom = effect.dom; - if (dom !== null) { + if (dom !== null && !skip_remove_dom) { remove(dom); } - destroy_effect_children(effect); + destroy_effect_children(effect, skip_remove_dom); remove_reactions(effect, 0); set_signal_status(effect, DESTROYED); @@ -384,7 +385,7 @@ export function pause_effect(effect, callback) { pause_children(effect, transitions, true); run_out_transitions(transitions, () => { - destroy_effect(effect); + destroy_effect(effect, false); if (callback) callback(); }); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 1f0a0bfdabac..2a6f6270cb21 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -478,16 +478,17 @@ export function remove_reactions(signal, start_index) { /** * @param {import('#client').Reaction} signal + * @param {boolean} skip_remove_dom * @returns {void} */ -export function destroy_effect_children(signal) { +export function destroy_effect_children(signal, skip_remove_dom) { let effect = signal.first; signal.first = null; signal.last = null; var sibling; while (effect !== null) { sibling = effect.next; - destroy_effect(effect); + destroy_effect(effect, skip_remove_dom); effect = sibling; } } @@ -520,7 +521,7 @@ export function execute_effect(effect) { try { if ((flags & BLOCK_EFFECT) === 0) { - destroy_effect_children(effect); + destroy_effect_children(effect, false); } execute_effect_teardown(effect); From d55ef9437fe52145d002d2ecd030b7b138a43f72 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 30 May 2024 08:50:55 +0100 Subject: [PATCH 2/6] missed some --- packages/svelte/src/internal/client/dom/blocks/await.js | 4 ++-- packages/svelte/src/internal/client/dom/blocks/snippet.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index a570cc6bcaf6..8ea0434c537c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -74,7 +74,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { if (pending_fn) { if (pending_effect && (pending_effect.f & INERT) === 0) { - destroy_effect(pending_effect); + destroy_effect(pending_effect, false); } pending_effect = branch(() => pending_fn(anchor)); @@ -107,7 +107,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { if (then_fn) { if (then_effect) { - destroy_effect(then_effect); + destroy_effect(then_effect, false); } then_effect = branch(() => then_fn(anchor, input)); diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index 5bf13a45d8d4..62c9f5e5f6c6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -25,7 +25,7 @@ export function snippet(get_snippet, node, ...args) { if (snippet === (snippet = get_snippet())) return; if (snippet_effect) { - destroy_effect(snippet_effect); + destroy_effect(snippet_effect, false); snippet_effect = null; } From 4ba1d04c9f8defc553d462968e409ac2fd004215 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 30 May 2024 12:09:01 +0100 Subject: [PATCH 3/6] missed some --- packages/svelte/src/internal/client/dev/hmr.js | 2 +- .../svelte/src/internal/client/dom/elements/custom-element.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/hmr.js b/packages/svelte/src/internal/client/dev/hmr.js index 0f79607707fe..89af22edfb59 100644 --- a/packages/svelte/src/internal/client/dev/hmr.js +++ b/packages/svelte/src/internal/client/dev/hmr.js @@ -23,7 +23,7 @@ export function hmr(source) { if (effect) { // @ts-ignore for (var k in instance) delete instance[k]; - destroy_effect(effect); + destroy_effect(effect, false); } effect = branch(() => { diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index ae966651e8c9..076402e337de 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -196,7 +196,7 @@ if (typeof HTMLElement === 'function') { Promise.resolve().then(() => { if (!this.$$cn && this.$$c) { this.$$c.$destroy(); - destroy_effect(this.$$me); + destroy_effect(this.$$me, false); this.$$c = undefined; } }); From e20514424627715406e99d0ce07288657a48eb5a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 31 May 2024 11:25:51 +0100 Subject: [PATCH 4/6] address feedback --- packages/svelte/src/internal/client/dev/hmr.js | 2 +- .../svelte/src/internal/client/dom/blocks/await.js | 4 ++-- .../svelte/src/internal/client/dom/blocks/snippet.js | 2 +- .../src/internal/client/dom/blocks/svelte-element.js | 2 +- .../src/internal/client/dom/elements/custom-element.js | 2 +- .../svelte/src/internal/client/reactivity/deriveds.js | 2 +- .../svelte/src/internal/client/reactivity/effects.js | 10 +++++----- packages/svelte/src/internal/client/runtime.js | 8 ++++---- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/hmr.js b/packages/svelte/src/internal/client/dev/hmr.js index 89af22edfb59..0f79607707fe 100644 --- a/packages/svelte/src/internal/client/dev/hmr.js +++ b/packages/svelte/src/internal/client/dev/hmr.js @@ -23,7 +23,7 @@ export function hmr(source) { if (effect) { // @ts-ignore for (var k in instance) delete instance[k]; - destroy_effect(effect, false); + destroy_effect(effect); } effect = branch(() => { diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 8ea0434c537c..a570cc6bcaf6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -74,7 +74,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { if (pending_fn) { if (pending_effect && (pending_effect.f & INERT) === 0) { - destroy_effect(pending_effect, false); + destroy_effect(pending_effect); } pending_effect = branch(() => pending_fn(anchor)); @@ -107,7 +107,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { if (then_fn) { if (then_effect) { - destroy_effect(then_effect, false); + destroy_effect(then_effect); } then_effect = branch(() => then_fn(anchor, input)); diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index 62c9f5e5f6c6..5bf13a45d8d4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -25,7 +25,7 @@ export function snippet(get_snippet, node, ...args) { if (snippet === (snippet = get_snippet())) return; if (snippet_effect) { - destroy_effect(snippet_effect, false); + destroy_effect(snippet_effect); snippet_effect = null; } diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index a57d2cc519c4..df607fe54a58 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -95,7 +95,7 @@ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, locat resume_effect(effect); } else { // tag is changing — destroy immediately, render contents without intro transitions - destroy_effect(effect, false); + destroy_effect(effect); set_should_intro(false); } } diff --git a/packages/svelte/src/internal/client/dom/elements/custom-element.js b/packages/svelte/src/internal/client/dom/elements/custom-element.js index 076402e337de..ae966651e8c9 100644 --- a/packages/svelte/src/internal/client/dom/elements/custom-element.js +++ b/packages/svelte/src/internal/client/dom/elements/custom-element.js @@ -196,7 +196,7 @@ if (typeof HTMLElement === 'function') { Promise.resolve().then(() => { if (!this.$$cn && this.$$c) { this.$$c.$destroy(); - destroy_effect(this.$$me, false); + destroy_effect(this.$$me); this.$$c = undefined; } }); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index b22feafaa650..6ebd945301d1 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -71,7 +71,7 @@ export function derived_safe_equal(fn) { * @returns {void} */ function destroy_derived_children(signal) { - destroy_effect_children(signal, false); + destroy_effect_children(signal); var deriveds = signal.deriveds; if (deriveds !== null) { diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 9c401e02b50c..6ec6ce05d8a5 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -198,7 +198,7 @@ export function user_pre_effect(fn) { export function effect_root(fn) { const effect = create_effect(ROOT_EFFECT, fn, true); return () => { - destroy_effect(effect, false); + destroy_effect(effect); }; } @@ -311,17 +311,17 @@ export function execute_effect_teardown(effect) { /** * @param {import('#client').Effect} effect - * @param {boolean} skip_remove_dom + * @param {boolean} [remove_dom] * @returns {void} */ -export function destroy_effect(effect, skip_remove_dom) { +export function destroy_effect(effect, remove_dom = true) { var dom = effect.dom; - if (dom !== null && !skip_remove_dom) { + if (dom !== null && remove_dom) { remove(dom); } - destroy_effect_children(effect, skip_remove_dom); + destroy_effect_children(effect, remove_dom); remove_reactions(effect, 0); set_signal_status(effect, DESTROYED); diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2a6f6270cb21..8fe016532da0 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -478,17 +478,17 @@ export function remove_reactions(signal, start_index) { /** * @param {import('#client').Reaction} signal - * @param {boolean} skip_remove_dom + * @param {boolean} [remove_dom] * @returns {void} */ -export function destroy_effect_children(signal, skip_remove_dom) { +export function destroy_effect_children(signal, remove_dom = true) { let effect = signal.first; signal.first = null; signal.last = null; var sibling; while (effect !== null) { sibling = effect.next; - destroy_effect(effect, skip_remove_dom); + destroy_effect(effect, remove_dom); effect = sibling; } } @@ -521,7 +521,7 @@ export function execute_effect(effect) { try { if ((flags & BLOCK_EFFECT) === 0) { - destroy_effect_children(effect, false); + destroy_effect_children(effect); } execute_effect_teardown(effect); From 79fce639b821732d55250291dcff9460bd18592e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 31 May 2024 11:26:31 +0100 Subject: [PATCH 5/6] address feedback --- packages/svelte/src/internal/client/reactivity/effects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 6ec6ce05d8a5..f7db38f3d148 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -385,7 +385,7 @@ export function pause_effect(effect, callback) { pause_children(effect, transitions, true); run_out_transitions(transitions, () => { - destroy_effect(effect, false); + destroy_effect(effect); if (callback) callback(); }); } From a0316e86d80ac3bd41dac345cbfa1df223510177 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 31 May 2024 11:27:00 +0100 Subject: [PATCH 6/6] address feedback --- packages/svelte/src/internal/client/dom/blocks/each.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 48ea57f91433..bef42ea5c4ff 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -81,7 +81,7 @@ function pause_effects(items, controlled_anchor, callback) { run_out_transitions(transitions, () => { for (var i = 0; i < length; i++) { - destroy_effect(items[i].e, is_controlled); + destroy_effect(items[i].e, !is_controlled); } if (callback !== undefined) callback();