chore: use internal doubly linked listed for effect tree (#10994)

* chore: use internal doubly linked listed for effect tree

* cleanup

* todo

Author: trueadm

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

@@ -7,10 +7,10 @@ import {
 	set_signal_status,
 	mark_reactions,
 	current_skip_reaction,
-	execute_reaction_fn
+	execute_reaction_fn,
+	destroy_effect_children
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
-import { destroy_effect } from './effects.js';
 
 export let updating_derived = false;
 
@@ -26,13 +26,14 @@ export function derived(fn) {
 
 	/** @type {import('#client').Derived<V>} */
 	const signal = {
-		reactions: null,
 		deps: null,
+		deriveds: null,
 		equals,
 		f: flags,
+		first: null,
 		fn,
-		effects: null,
-		deriveds: null,
+		last: null,
+		reactions: null,
 		v: /** @type {V} */ (null),
 		version: 0
 	};
@@ -70,20 +71,12 @@ export function derived_safe_equal(fn) {
  * @returns {void}
  */
 function destroy_derived_children(signal) {
-	var effects = signal.effects;
-
-	// TODO: should it be possible to create effects in deriveds given they're meant to be pure?
-	if (effects !== null) {
-		signal.effects = null;
-		for (var i = 0; i < effects.length; i += 1) {
-			destroy_effect(effects[i]);
-		}
-	}
+	destroy_effect_children(signal);
 	var deriveds = signal.deriveds;
 
 	if (deriveds !== null) {
 		signal.deriveds = null;
-		for (i = 0; i < deriveds.length; i += 1) {
+		for (var i = 0; i < deriveds.length; i += 1) {
 			destroy_derived(deriveds[i]);
 		}
 	}
@@ -127,7 +120,10 @@ export function destroy_derived(signal) {
 	remove_reactions(signal, 0);
 	set_signal_status(signal, DESTROYED);
 
-	signal.effects =
+	// TODO we need to ensure we remove the derived from any parent derives
+
+	signal.first =
+		signal.last =
 		signal.deps =
 		signal.reactions =
 		// @ts-expect-error `signal.fn` cannot be `null` while the signal is alive

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

@@ -4,7 +4,7 @@ import {
 	current_component_context,
 	current_effect,
 	current_reaction,
-	destroy_children,
+	destroy_effect_children,
 	execute_effect,
 	get,
 	is_flushing_effect,
@@ -21,15 +21,30 @@ import {
 	EFFECT,
 	DESTROYED,
 	INERT,
-	IS_ELSEIF,
 	EFFECT_RAN,
 	BLOCK_EFFECT,
-	ROOT_EFFECT
+	ROOT_EFFECT,
+	IS_ELSEIF
 } from '../constants.js';
 import { set } from './sources.js';
 import { noop } from '../../shared/utils.js';
 import { remove } from '../dom/reconciler.js';
 
+/**
+ * @param {import("#client").Effect} effect
+ * @param {import("#client").Reaction} parent_effect
+ */
+export function push_effect(effect, parent_effect) {
+	var parent_last = parent_effect.last;
+	if (parent_last === null) {
+		parent_effect.last = parent_effect.first = effect;
+	} else {
+		parent_last.next = effect;
+		effect.prev = parent_last;
+		parent_effect.last = effect;
+	}
+}
+
 /**
  * @param {number} type
  * @param {(() => void | (() => void))} fn
@@ -40,23 +55,22 @@ function create_effect(type, fn, sync) {
 	var is_root = (type & ROOT_EFFECT) !== 0;
 	/** @type {import('#client').Effect} */
 	var effect = {
-		parent: is_root ? null : current_effect,
-		dom: null,
+		ctx: current_component_context,
 		deps: null,
+		dom: null,
 		f: type | DIRTY,
+		first: null,
 		fn,
-		effects: null,
+		last: null,
+		next: null,
+		parent: is_root ? null : current_effect,
+		prev: null,
 		teardown: null,
-		ctx: current_component_context,
 		transitions: null
 	};
 
 	if (current_reaction !== null && !is_root) {
-		if (current_reaction.effects === null) {
-			current_reaction.effects = [effect];
-		} else {
-			current_reaction.effects.push(effect);
-		}
+		push_effect(effect, current_reaction);
 	}
 
 	if (sync) {
@@ -221,7 +235,7 @@ export function branch(fn) {
  * @returns {void}
  */
 export function destroy_effect(effect) {
-	destroy_children(effect);
+	destroy_effect_children(effect);
 	remove_reactions(effect, 0);
 	set_signal_status(effect, DESTROYED);
 
@@ -239,15 +253,31 @@ export function destroy_effect(effect) {
 
 	var parent = effect.parent;
 
-	if (parent !== null && (effect.f & BRANCH_EFFECT) !== 0) {
-		var effects = parent.effects;
-		if (effects !== null) {
-			var index = effects.indexOf(effect);
-			effects.splice(index, 1);
+	// 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;
 		}
 	}
 
-	effect.effects =
+	effect.first =
+		effect.last =
+		effect.next =
+		effect.prev =
 		effect.teardown =
 		effect.ctx =
 		effect.dom =
@@ -334,11 +364,14 @@ function pause_children(effect, transitions, local) {
 		}
 	}
 
-	if (effect.effects !== null) {
-		for (const child of effect.effects) {
-			var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
-			pause_children(child, transitions, transparent ? local : false);
-		}
+	var child = effect.first;
+
+	while (child !== null) {
+		var sibling = child.next;
+		var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
+		// TODO we don't need to call pause_children recursively with a linked list in place
+		pause_children(child, transitions, transparent ? local : false);
+		child = sibling;
 	}
 }
 
@@ -365,11 +398,14 @@ function resume_children(effect, local) {
 		execute_effect(effect);
 	}
 
-	if (effect.effects !== null) {
-		for (const child of effect.effects) {
-			var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
-			resume_children(child, transparent ? local : false);
-		}
+	var child = effect.first;
+
+	while (child !== null) {
+		var sibling = child.next;
+		var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
+		// TODO we don't need to call resume_children recursively with a linked list in place
+		resume_children(child, transparent ? local : false);
+		child = sibling;
 	}
 
 	if (effect.transitions !== null) {

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

@@ -21,8 +21,10 @@ export interface Reaction extends Signal {
 	fn: Function;
 	/** Signals that this signal reads from */
 	deps: null | Value[];
-	/** Effects created inside this signal */
-	effects: null | Effect[];
+	/** First child effect created inside this signal */
+	first: null | Effect;
+	/** Last child effect created inside this signal */
+	last: null | Effect;
 }
 
 export interface Derived<V = unknown> extends Value<V>, Reaction {
@@ -43,6 +45,10 @@ export interface Effect extends Reaction {
 	teardown: null | (() => void);
 	/** Transition managers created with `$.transition` */
 	transitions: null | TransitionManager[];
+	/** Next sibling child effect created inside the parent signal */
+	prev: null | Effect;
+	/** Next sibling child effect created inside the parent signal */
+	next: null | Effect;
 }
 
 export interface ValueDebug<V = unknown> extends Value<V> {

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

@@ -26,7 +26,7 @@ import {
 import { flush_tasks } from './dom/task.js';
 import { add_owner } from './dev/ownership.js';
 import { mutate, set, source } from './reactivity/sources.js';
-import { destroy_derived, update_derived } from './reactivity/deriveds.js';
+import { update_derived } from './reactivity/deriveds.js';
 
 const FLUSH_MICROTASK = 0;
 const FLUSH_SYNC = 1;
@@ -349,17 +349,18 @@ export function remove_reactions(signal, start_index) {
 }
 
 /**
- * @param {import('./types.js').Effect} signal
+ * @param {import('./types.js').Reaction} signal
  * @returns {void}
  */
-export function destroy_children(signal) {
-	var effects = signal.effects;
-
-	if (effects !== null) {
-		signal.effects = null;
-		for (var i = 0; i < effects.length; i += 1) {
-			destroy_effect(effects[i]);
-		}
+export function destroy_effect_children(signal) {
+	let effect = signal.first;
+	signal.first = null;
+	signal.last = null;
+	var sibling;
+	while (effect !== null) {
+		sibling = effect.next;
+		destroy_effect(effect);
+		effect = sibling;
 	}
 }
 
@@ -386,7 +387,7 @@ export function execute_effect(effect) {
 
 	try {
 		if ((flags & BLOCK_EFFECT) === 0) {
-			destroy_children(effect);
+			destroy_effect_children(effect);
 		}
 
 		effect.teardown?.();
@@ -499,13 +500,12 @@ export function schedule_effect(signal) {
  * @returns {void}
  */
 function recursively_process_effects(effect, filter_flags, shallow, collected_user) {
-	var effects = effect.effects;
-	if (effects === null) return;
-
+	var current_child = effect.first;
 	var user = [];
 
-	for (var i = 0; i < effects.length; i++) {
-		var child = effects[i];
+	while (current_child !== null) {
+		var child = current_child;
+		current_child = child.next;
 		var flags = child.f;
 		var is_inactive = (flags & (DESTROYED | INERT)) !== 0;
 		if (is_inactive) continue;
@@ -521,16 +521,19 @@ function recursively_process_effects(effect, filter_flags, shallow, collected_us
 		if ((flags & RENDER_EFFECT) !== 0) {
 			if (is_branch) {
 				if (shallow) continue;
+				// TODO we don't need to call recursively_process_effects recursively with a linked list in place
 				recursively_process_effects(child, filter_flags, false, collected_user);
 			} else {
 				if (check_dirtiness(child)) {
 					execute_effect(child);
 				}
+				// TODO we don't need to call recursively_process_effects recursively with a linked list in place
 				recursively_process_effects(child, filter_flags, false, collected_user);
 			}
 		} else if ((flags & EFFECT) !== 0) {
 			if (is_branch || is_clean) {
 				if (shallow) continue;
+				// TODO we don't need to call recursively_process_effects recursively with a linked list in place
 				recursively_process_effects(child, filter_flags, false, collected_user);
 			} else {
 				user.push(child);
@@ -544,7 +547,8 @@ function recursively_process_effects(effect, filter_flags, shallow, collected_us
 		}
 
 		if (!shallow) {
-			for (i = 0; i < user.length; i++) {
+			for (var i = 0; i < user.length; i++) {
+				// TODO we don't need to call recursively_process_effects recursively with a linked list in place
 				recursively_process_effects(user[i], filter_flags, false, collected_user);
 			}
 		}
@@ -571,7 +575,7 @@ function flush_nested_effects(effect, filter_flags, shallow = false) {
 
 	try {
 		// When working with custom elements, the root effects might not have a root
-		if (effect.effects === null && (effect.f & BRANCH_EFFECT) === 0) {
+		if (effect.first === null && (effect.f & BRANCH_EFFECT) === 0) {
 			flush_queued_effects([effect]);
 		} else {
 			recursively_process_effects(effect, filter_flags, shallow, user_effects);