From c39805d027b6230f54255d006ec00da192069f4b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 10 Jun 2024 09:32:11 -0400 Subject: [PATCH] feat: detach inert effects (#11955) * feat: detach inert effects * simplify * avoid adding inert effects to the tree * partial fix * fix * DRY out * optimise * comments * explicit comparisons --- .changeset/ten-teachers-travel.md | 5 ++ .../src/internal/client/dom/blocks/await.js | 6 +- .../client/dom/blocks/svelte-element.js | 8 ++ .../src/internal/client/reactivity/effects.js | 76 +++++++++++-------- .../svelte/src/internal/client/runtime.js | 22 +++++- packages/svelte/tests/signals/test.ts | 3 +- 6 files changed, 84 insertions(+), 36 deletions(-) create mode 100644 .changeset/ten-teachers-travel.md diff --git a/.changeset/ten-teachers-travel.md b/.changeset/ten-teachers-travel.md new file mode 100644 index 0000000000..e6b73aed56 --- /dev/null +++ b/.changeset/ten-teachers-travel.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: detach inert effects diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index a570cc6bca..8956631a6a 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -1,4 +1,4 @@ -import { is_promise } from '../../../shared/utils.js'; +import { is_promise, noop } from '../../../shared/utils.js'; import { current_component_context, flush_sync, @@ -113,5 +113,9 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { then_effect = branch(() => then_fn(anchor, input)); } } + + // Inert effects are proactively detached from the effect tree. Returning a noop + // teardown function is an easy way to ensure that this is not discarded + return noop; }); } 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 8e5dd9e5a8..651c5fd7ca 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -14,6 +14,7 @@ import { current_component_context, current_effect } from '../../runtime.js'; import { DEV } from 'esm-env'; import { is_array } from '../../utils.js'; import { push_template_node } from '../template.js'; +import { noop } from '../../../shared/utils.js'; /** * @param {import('#client').Effect} effect @@ -151,6 +152,9 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio push_template_node(element, parent_effect); } } + + // See below + return noop; }); } @@ -159,5 +163,9 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio set_should_intro(true); set_current_each_item(previous_each_item); + + // Inert effects are proactively detached from the effect tree. Returning a noop + // teardown function is an easy way to ensure that this is not discarded + return noop; }); } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 4ae4ce2870..356986dce9 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -96,7 +96,30 @@ function create_effect(type, fn, sync) { effect.component_function = dev_current_component_function; } - if (current_reaction !== null && !is_root) { + if (sync) { + var previously_flushing_effect = is_flushing_effect; + + try { + set_is_flushing_effect(true); + execute_effect(effect); + effect.f |= EFFECT_RAN; + } finally { + set_is_flushing_effect(previously_flushing_effect); + } + } else if (fn !== null) { + schedule_effect(effect); + } + + // if an effect has no dependencies, no DOM and no teardown function, + // don't bother adding it to the effect tree + var inert = + sync && + effect.deps === null && + effect.first === null && + effect.dom === null && + effect.teardown === null; + + if (!inert && current_reaction !== null && !is_root) { var flags = current_reaction.f; if ((flags & DERIVED) !== 0) { if ((flags & UNOWNED) !== 0) { @@ -112,20 +135,6 @@ function create_effect(type, fn, sync) { push_effect(effect, current_reaction); } - if (sync) { - var previously_flushing_effect = is_flushing_effect; - - try { - set_is_flushing_effect(true); - execute_effect(effect); - effect.f |= EFFECT_RAN; - } finally { - set_is_flushing_effect(previously_flushing_effect); - } - } else if (fn !== null) { - schedule_effect(effect); - } - return effect; } @@ -348,23 +357,7 @@ export function destroy_effect(effect, remove_dom = true) { // If the parent doesn't have any children, then skip this work altogether if (parent !== null && (effect.f & BRANCH_EFFECT) !== 0 && parent.first !== null) { - var previous = effect.prev; - var next = effect.next; - if (previous !== null) { - if (next !== null) { - previous.next = next; - next.prev = previous; - } else { - previous.next = null; - parent.last = previous; - } - } else if (next !== null) { - next.prev = null; - parent.first = next; - } else { - parent.first = null; - parent.last = null; - } + unlink_effect(effect); } // `first` and `child` are nulled out in destroy_effect_children @@ -379,6 +372,25 @@ export function destroy_effect(effect, remove_dom = true) { null; } +/** + * Detach an effect from the effect tree, freeing up memory and + * reducing the amount of work that happens on subsequent traversals + * @param {import('#client').Effect} effect + */ +export function unlink_effect(effect) { + var parent = effect.parent; + var prev = effect.prev; + var next = effect.next; + + if (prev !== null) prev.next = next; + if (next !== null) next.prev = prev; + + if (parent !== null) { + if (parent.first === effect) parent.first = next; + if (parent.last === effect) parent.last = prev; + } +} + /** * When a block effect is removed, we don't immediately destroy it or yank it * out of the DOM, because it might have transitions. Instead, we 'pause' it. diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index fc6cc43eb3..8b9ca1c64b 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -7,7 +7,12 @@ import { object_freeze } from './utils.js'; import { snapshot } from './proxy.js'; -import { destroy_effect, effect, execute_effect_teardown } from './reactivity/effects.js'; +import { + destroy_effect, + effect, + execute_effect_teardown, + unlink_effect +} from './reactivity/effects.js'; import { EFFECT, RENDER_EFFECT, @@ -603,6 +608,21 @@ function flush_queued_effects(effects) { if ((effect.f & (DESTROYED | INERT)) === 0 && check_dirtiness(effect)) { execute_effect(effect); + + // Effects with no dependencies or teardown do not get added to the effect tree. + // Deferred effects (e.g. `$effect(...)`) _are_ added to the tree because we + // don't know if we need to keep them until they are executed. Doing the check + // here (rather than in `execute_effect`) allows us to skip the work for + // immediate effects. + if (effect.deps === null && effect.first === null && effect.dom === null) { + if (effect.teardown === null) { + // remove this effect from the graph + unlink_effect(effect); + } else { + // keep the effect in the graph, but free up some memory + effect.fn = null; + } + } } } } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 0bf85a6f87..2c681fd41f 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -2,14 +2,13 @@ import { describe, assert, it } from 'vitest'; import { flushSync } from '../../src/index-client'; import * as $ from '../../src/internal/client/runtime'; import { - destroy_effect, effect, effect_root, render_effect, user_effect } from '../../src/internal/client/reactivity/effects'; import { source, set } from '../../src/internal/client/reactivity/sources'; -import type { Derived, Effect, Value } from '../../src/internal/client/types'; +import type { Derived, Value } from '../../src/internal/client/types'; import { proxy } from '../../src/internal/client/proxy'; import { derived } from '../../src/internal/client/reactivity/deriveds';