Skip to content

Commit

Permalink
feat: detach inert effects (#11955)
Browse files Browse the repository at this point in the history
* feat: detach inert effects

* simplify

* avoid adding inert effects to the tree

* partial fix

* fix

* DRY out

* optimise

* comments

* explicit comparisons
  • Loading branch information
Rich-Harris authored Jun 10, 2024
1 parent 3bfb302 commit c39805d
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 36 deletions.
5 changes: 5 additions & 0 deletions .changeset/ten-teachers-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: detach inert effects
6 changes: 5 additions & 1 deletion packages/svelte/src/internal/client/dom/blocks/await.js
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
});
}

Expand All @@ -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;
});
}
76 changes: 44 additions & 32 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
22 changes: 21 additions & 1 deletion packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
}
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions packages/svelte/tests/signals/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down

0 comments on commit c39805d

Please sign in to comment.