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
pull/11970/head
Rich Harris 7 months ago committed by GitHub
parent 3bfb302a63
commit c39805d027
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
feat: detach inert effects

@ -1,4 +1,4 @@
import { is_promise } from '../../../shared/utils.js'; import { is_promise, noop } from '../../../shared/utils.js';
import { import {
current_component_context, current_component_context,
flush_sync, 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)); 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;
}); });
} }

@ -14,6 +14,7 @@ import { current_component_context, current_effect } from '../../runtime.js';
import { DEV } from 'esm-env'; import { DEV } from 'esm-env';
import { is_array } from '../../utils.js'; import { is_array } from '../../utils.js';
import { push_template_node } from '../template.js'; import { push_template_node } from '../template.js';
import { noop } from '../../../shared/utils.js';
/** /**
* @param {import('#client').Effect} effect * @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); 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_should_intro(true);
set_current_each_item(previous_each_item); 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;
}); });
} }

@ -96,7 +96,30 @@ function create_effect(type, fn, sync) {
effect.component_function = dev_current_component_function; 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; var flags = current_reaction.f;
if ((flags & DERIVED) !== 0) { if ((flags & DERIVED) !== 0) {
if ((flags & UNOWNED) !== 0) { if ((flags & UNOWNED) !== 0) {
@ -112,20 +135,6 @@ function create_effect(type, fn, sync) {
push_effect(effect, current_reaction); 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; 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 the parent doesn't have any children, then skip this work altogether
if (parent !== null && (effect.f & BRANCH_EFFECT) !== 0 && parent.first !== null) { if (parent !== null && (effect.f & BRANCH_EFFECT) !== 0 && parent.first !== null) {
var previous = effect.prev; unlink_effect(effect);
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;
}
} }
// `first` and `child` are nulled out in destroy_effect_children // `first` and `child` are nulled out in destroy_effect_children
@ -379,6 +372,25 @@ export function destroy_effect(effect, remove_dom = true) {
null; 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 * 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. * out of the DOM, because it might have transitions. Instead, we 'pause' it.

@ -7,7 +7,12 @@ import {
object_freeze object_freeze
} from './utils.js'; } from './utils.js';
import { snapshot } from './proxy.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 { import {
EFFECT, EFFECT,
RENDER_EFFECT, RENDER_EFFECT,
@ -603,6 +608,21 @@ function flush_queued_effects(effects) {
if ((effect.f & (DESTROYED | INERT)) === 0 && check_dirtiness(effect)) { if ((effect.f & (DESTROYED | INERT)) === 0 && check_dirtiness(effect)) {
execute_effect(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;
}
}
} }
} }
} }

@ -2,14 +2,13 @@ import { describe, assert, it } from 'vitest';
import { flushSync } from '../../src/index-client'; import { flushSync } from '../../src/index-client';
import * as $ from '../../src/internal/client/runtime'; import * as $ from '../../src/internal/client/runtime';
import { import {
destroy_effect,
effect, effect,
effect_root, effect_root,
render_effect, render_effect,
user_effect user_effect
} from '../../src/internal/client/reactivity/effects'; } from '../../src/internal/client/reactivity/effects';
import { source, set } from '../../src/internal/client/reactivity/sources'; 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 { proxy } from '../../src/internal/client/proxy';
import { derived } from '../../src/internal/client/reactivity/deriveds'; import { derived } from '../../src/internal/client/reactivity/deriveds';

Loading…
Cancel
Save