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

Author: dummdidumm

.changeset/slimy-gifts-cough.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+perf: skip repeatedly traversing the same derived

packages/svelte/src/internal/client/constants.js

@@ -1,17 +1,18 @@
+// General flags
 export const DERIVED = 1 << 1;
 export const EFFECT = 1 << 2;
 export const RENDER_EFFECT = 1 << 3;
 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;

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<Value>} */
+					const marked = new Set();
+					/** @type {Map<Reaction, boolean>} */
+					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<Value>} marked
+ * @param {Map<Reaction, boolean>} 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<Reaction, boolean>} 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;
 }
 

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 {

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) {

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;