chore: improve signal performance by reducing duplicate deps (#14945)

* chore: expand benchmark iterations

* chore: improve signal performance by reducing duplicate deps

* feedback

* oops

.changeset/tame-students-retire.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+chore: improve signal performance by reducing duplicate deps

packages/svelte/src/internal/client/dev/tracing.js

@@ -42,15 +42,12 @@ function log_entry(signal, entry) {
 
 	const type = (signal.f & DERIVED) !== 0 ? '$derived' : '$state';
 	const current_reaction = /** @type {Reaction} */ (active_reaction);
-	const status =
-		signal.version > current_reaction.version || current_reaction.version === 0 ? 'dirty' : 'clean';
+	const dirty = signal.wv > current_reaction.wv || current_reaction.wv === 0;
 
 	// eslint-disable-next-line no-console
 	console.groupCollapsed(
 		`%c${type}`,
-		status !== 'clean'
-			? 'color: CornflowerBlue; font-weight: bold'
-			: 'color: grey; font-weight: bold',
+		dirty ? 'color: CornflowerBlue; font-weight: bold' : 'color: grey; font-weight: bold',
 		typeof value === 'object' && value !== null && STATE_SYMBOL in value
 			? snapshot(value, true)
 			: value

packages/svelte/src/internal/client/reactivity/deriveds.js

@@ -16,7 +16,7 @@ import {
 	set_signal_status,
 	skip_reaction,
 	update_reaction,
-	increment_version,
+	increment_write_version,
 	set_active_effect,
 	component_context
 } from '../runtime.js';
@@ -58,8 +58,9 @@ export function derived(fn) {
 		f: flags,
 		fn,
 		reactions: null,
+		rv: 0,
 		v: /** @type {V} */ (null),
-		version: 0,
+		wv: 0,
 		parent: parent_derived ?? active_effect
 	};
 
@@ -182,7 +183,7 @@ export function update_derived(derived) {
 
 	if (!derived.equals(value)) {
 		derived.v = value;
-		derived.version = increment_version();
+		derived.wv = increment_write_version();
 	}
 }
 

packages/svelte/src/internal/client/reactivity/effects.js

@@ -110,7 +110,7 @@ function create_effect(type, fn, sync, push = true) {
 		prev: null,
 		teardown: null,
 		transitions: null,
-		version: 0
+		wv: 0
 	};
 
 	if (DEV) {

packages/svelte/src/internal/client/reactivity/sources.js

@@ -12,7 +12,7 @@ import {
 	set_untracked_writes,
 	set_signal_status,
 	untrack,
-	increment_version,
+	increment_write_version,
 	update_effect,
 	derived_sources,
 	set_derived_sources,
@@ -57,7 +57,8 @@ export function source(v, stack) {
 		v,
 		reactions: null,
 		equals,
-		version: 0
+		rv: 0,
+		wv: 0
 	};
 
 	if (DEV && tracing_mode_flag) {
@@ -169,7 +170,7 @@ export function internal_set(source, value) {
 	if (!source.equals(value)) {
 		var old_value = source.v;
 		source.v = value;
-		source.version = increment_version();
+		source.wv = increment_write_version();
 
 		if (DEV && tracing_mode_flag) {
 			source.updated = get_stack('UpdatedAt');

packages/svelte/src/internal/client/reactivity/types.d.ts

@@ -4,14 +4,16 @@ export interface Signal {
 	/** Flags bitmask */
 	f: number;
 	/** Write version */
-	version: number;
+	wv: number;
 }
 
 export interface Value<V = unknown> extends Signal {
-	/** Signals that read from this signal */
-	reactions: null | Reaction[];
 	/** Equality function */
 	equals: Equals;
+	/** Signals that read from this signal */
+	reactions: null | Reaction[];
+	/** Read version */
+	rv: number;
 	/** The latest value for this signal */
 	v: V;
 	/** Dev only */

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

@@ -127,8 +127,14 @@ export function set_untracked_writes(value) {
 	untracked_writes = value;
 }
 
-/** @type {number} Used by sources and deriveds for handling updates to unowned deriveds it starts from 1 to differentiate between a created effect and a run one for tracing */
-let current_version = 1;
+/**
+ * @type {number} Used by sources and deriveds for handling updates.
+ * Version starts from 1 so that unowned deriveds differentiate between a created effect and a run one for tracing
+ **/
+let write_version = 1;
+
+/** @type {number} Used to version each read of a source of derived to avoid duplicating depedencies inside a reaction */
+let read_version = 0;
 
 // If we are working with a get() chain that has no active container,
 // to prevent memory leaks, we skip adding the reaction.
@@ -168,8 +174,8 @@ export function set_dev_current_component_function(fn) {
 	dev_current_component_function = fn;
 }
 
-export function increment_version() {
-	return ++current_version;
+export function increment_write_version() {
+	return ++write_version;
 }
 
 /** @returns {boolean} */
@@ -226,7 +232,7 @@ export function check_dirtiness(reaction) {
 					update_derived(/** @type {Derived} */ (dependency));
 				}
 
-				if (dependency.version > reaction.version) {
+				if (dependency.wv > reaction.wv) {
 					return true;
 				}
 			}
@@ -398,6 +404,7 @@ export function update_reaction(reaction) {
 	skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0;
 	derived_sources = null;
 	component_context = reaction.ctx;
+	read_version++;
 
 	try {
 		var result = /** @type {Function} */ (0, reaction.fn)();
@@ -528,7 +535,7 @@ export function update_effect(effect) {
 		execute_effect_teardown(effect);
 		var teardown = update_reaction(effect);
 		effect.teardown = typeof teardown === 'function' ? teardown : null;
-		effect.version = current_version;
+		effect.wv = write_version;
 
 		var deps = effect.deps;
 
@@ -540,7 +547,7 @@ export function update_effect(effect) {
 			for (let i = 0; i < deps.length; i++) {
 				var dep = deps[i];
 				if (dep.trace_need_increase) {
-					dep.version = increment_version();
+					dep.wv = increment_write_version();
 					dep.trace_need_increase = undefined;
 					dep.trace_v = undefined;
 				}
@@ -880,27 +887,29 @@ export function get(signal) {
 			e.state_unsafe_local_read();
 		}
 		var deps = active_reaction.deps;
+		if (signal.rv < read_version) {
+			signal.rv = read_version;
+			// If the signal is accessing the same dependencies in the same
+			// order as it did last time, increment `skipped_deps`
+			// rather than updating `new_deps`, which creates GC cost
+			if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
+				skipped_deps++;
+			} else if (new_deps === null) {
+				new_deps = [signal];
+			} else {
+				new_deps.push(signal);
+			}
 
-		// If the signal is accessing the same dependencies in the same
-		// order as it did last time, increment `skipped_deps`
-		// rather than updating `new_deps`, which creates GC cost
-		if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
-			skipped_deps++;
-		} else if (new_deps === null) {
-			new_deps = [signal];
-		} else {
-			new_deps.push(signal);
-		}
-
-		if (
-			untracked_writes !== null &&
-			active_effect !== null &&
-			(active_effect.f & CLEAN) !== 0 &&
-			(active_effect.f & BRANCH_EFFECT) === 0 &&
-			untracked_writes.includes(signal)
-		) {
-			set_signal_status(active_effect, DIRTY);
-			schedule_effect(active_effect);
+			if (
+				untracked_writes !== null &&
+				active_effect !== null &&
+				(active_effect.f & CLEAN) !== 0 &&
+				(active_effect.f & BRANCH_EFFECT) === 0 &&
+				untracked_writes.includes(signal)
+			) {
+				set_signal_status(active_effect, DIRTY);
+				schedule_effect(active_effect);
+			}
 		}
 	} else if (is_derived && /** @type {Derived} */ (signal).deps === null) {
 		var derived = /** @type {Derived} */ (signal);

packages/svelte/tests/signals/test.ts

@@ -296,6 +296,7 @@ describe('signals', () => {
 		const destroy = effect_root(() => {
 			user_effect(() => {
 				log.push($.get(calc));
+				$.get(calc);
 			});
 		});
 
@@ -306,7 +307,7 @@ describe('signals', () => {
 			flushSync(() => set(count, 4));
 			flushSync(() => set(count, 0));
 			// Ensure we're not leaking consumers
-			assert.deepEqual(count.reactions?.length, 2);
+			assert.deepEqual(count.reactions?.length, 1);
 			assert.deepEqual(calc.reactions?.length, 1);
 			assert.deepEqual(log, [0, 2, 'limit', 0]);
 			destroy();