From 16117d9b692728c0d5b5288d8e3be7b7211e2afc Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Thu, 23 Oct 2025 20:29:21 +0200 Subject: [PATCH] perf: skip repeatedly traversing the same derived (#17016) * perf: skip repeatedly traversing the same derived We introduce a new flag which marks a derived during the mark_reaction phase and lift it during the execution phase. * fix This introduces a new flag which marks a derived as visited during the mark_reaction phase and lifts it during the dirty-check/execution phase (both are necessary because not all deriveds are necessarily executed, but they're always checked). We could've used a Set on mark_reactions which would've been simpler to reason about, alas it was performing consistenly worse on the kairo_mux_unowned / kairo_mux_owned benchmarks (it kinda makes sense generally, creating and filling a set is more expensive than juggling flags). Closes #16658, which showcases are particularly gnarly signal graph where many deriveds point to the same deriveds, where we would be traversing the same reactions over and over if we didn't do this. I also added a similar measure (this time a set/map is unavoidable/makes more sense) for mark_effects, hopefully this helps for #16990 --- .changeset/slimy-gifts-cough.md | 5 +++ .../svelte/src/internal/client/constants.js | 15 +++++++-- .../src/internal/client/reactivity/batch.js | 32 +++++++++++++++---- .../internal/client/reactivity/deriveds.js | 5 ++- .../src/internal/client/reactivity/sources.js | 8 +++-- .../svelte/src/internal/client/runtime.js | 7 +++- 6 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 .changeset/slimy-gifts-cough.md diff --git a/.changeset/slimy-gifts-cough.md b/.changeset/slimy-gifts-cough.md new file mode 100644 index 0000000000..b6c9897648 --- /dev/null +++ b/.changeset/slimy-gifts-cough.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +perf: skip repeatedly traversing the same derived diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 50a7a21ae8..1f35add2a8 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -1,3 +1,4 @@ +// General flags export const DERIVED = 1 << 1; export const EFFECT = 1 << 2; export const RENDER_EFFECT = 1 << 3; @@ -5,13 +6,13 @@ export const BLOCK_EFFECT = 1 << 4; export const BRANCH_EFFECT = 1 << 5; export const ROOT_EFFECT = 1 << 6; export const BOUNDARY_EFFECT = 1 << 7; -export const UNOWNED = 1 << 8; -export const DISCONNECTED = 1 << 9; export const CLEAN = 1 << 10; export const DIRTY = 1 << 11; export const MAYBE_DIRTY = 1 << 12; export const INERT = 1 << 13; export const DESTROYED = 1 << 14; + +// Flags exclusive to effects export const EFFECT_RAN = 1 << 15; /** 'Transparent' effects do not create a transition boundary */ export const EFFECT_TRANSPARENT = 1 << 16; @@ -20,6 +21,16 @@ export const HEAD_EFFECT = 1 << 18; export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; +// Flags exclusive to deriveds +export const UNOWNED = 1 << 8; +export const DISCONNECTED = 1 << 9; +/** + * Tells that we marked this derived and its reactions as visited during the "mark as (maybe) dirty"-phase. + * Will be lifted during execution of the derived and during checking its dirty state (both are necessary + * because a derived might be checked but not executed). + */ +export const WAS_MARKED = 1 << 15; + // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; export const ASYNC = 1 << 22; diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 91635bd5d2..2cef562ac9 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -377,8 +377,12 @@ export class Batch { // Re-run async/block effects that depend on distinct values changed in both batches const others = [...batch.current.keys()].filter((s) => !this.current.has(s)); if (others.length > 0) { + /** @type {Set} */ + const marked = new Set(); + /** @type {Map} */ + const checked = new Map(); for (const source of sources) { - mark_effects(source, others); + mark_effects(source, others, marked, checked); } if (queued_root_effects.length > 0) { @@ -688,15 +692,24 @@ function flush_queued_effects(effects) { * these effects can re-run after another batch has been committed * @param {Value} value * @param {Source[]} sources + * @param {Set} marked + * @param {Map} checked */ -function mark_effects(value, sources) { +function mark_effects(value, sources, marked, checked) { + if (marked.has(value)) return; + marked.add(value); + if (value.reactions !== null) { for (const reaction of value.reactions) { const flags = reaction.f; if ((flags & DERIVED) !== 0) { - mark_effects(/** @type {Derived} */ (reaction), sources); - } else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0 && depends_on(reaction, sources)) { + mark_effects(/** @type {Derived} */ (reaction), sources, marked, checked); + } else if ( + (flags & (ASYNC | BLOCK_EFFECT)) !== 0 && + (flags & DIRTY) === 0 && // we may have scheduled this one already + depends_on(reaction, sources, checked) + ) { set_signal_status(reaction, DIRTY); schedule_effect(/** @type {Effect} */ (reaction)); } @@ -707,20 +720,27 @@ function mark_effects(value, sources) { /** * @param {Reaction} reaction * @param {Source[]} sources + * @param {Map} checked */ -function depends_on(reaction, sources) { +function depends_on(reaction, sources, checked) { + const depends = checked.get(reaction); + if (depends !== undefined) return depends; + if (reaction.deps !== null) { for (const dep of reaction.deps) { if (sources.includes(dep)) { return true; } - if ((dep.f & DERIVED) !== 0 && depends_on(/** @type {Derived} */ (dep), sources)) { + if ((dep.f & DERIVED) !== 0 && depends_on(/** @type {Derived} */ (dep), sources, checked)) { + checked.set(/** @type {Derived} */ (dep), true); return true; } } } + checked.set(reaction, false); + return false; } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 06ae0f6d7a..5a3dee4b7f 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -10,7 +10,8 @@ import { MAYBE_DIRTY, STALE_REACTION, UNOWNED, - ASYNC + ASYNC, + WAS_MARKED } from '#client/constants'; import { active_reaction, @@ -326,6 +327,7 @@ export function execute_derived(derived) { stack.push(derived); + derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { @@ -335,6 +337,7 @@ export function execute_derived(derived) { } } else { try { + derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index c5dcff9cfb..2fe8c4f75d 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -27,7 +27,8 @@ import { MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, - ASYNC + ASYNC, + WAS_MARKED } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -332,7 +333,10 @@ function mark_reactions(signal, status) { } if ((flags & DERIVED) !== 0) { - mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); + if ((flags & WAS_MARKED) === 0) { + reaction.f |= WAS_MARKED; + mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); + } } else if (not_dirty) { if ((flags & BLOCK_EFFECT) !== 0) { if (eager_block_effects !== null) { diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a146659bf6..2e6f05b4b1 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -20,7 +20,8 @@ import { DISCONNECTED, REACTION_IS_UPDATING, STALE_REACTION, - ERROR_VALUE + ERROR_VALUE, + WAS_MARKED } from './constants.js'; import { old_values } from './reactivity/sources.js'; import { @@ -161,6 +162,10 @@ export function is_dirty(reaction) { var dependencies = reaction.deps; var is_unowned = (flags & UNOWNED) !== 0; + if (flags & DERIVED) { + reaction.f &= ~WAS_MARKED; + } + if (dependencies !== null) { var i; var dependency;