chore: add comments to update_reaction and get (#12271)

* add some explanatory comments

* rename variables

* var

* this appears to be unnecessary?

* more renaming

* surely we only care about array.length?

* no need to cache this, it isnt faster AFAICT

* comment

* remove unnecessary casting

* again

Author: Rich-Harris

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

@@ -2,7 +2,7 @@ import { DEV } from 'esm-env';
 import {
 	current_component_context,
 	current_reaction,
-	current_dependencies,
+	new_deps,
 	current_effect,
 	current_untracked_writes,
 	get,
@@ -109,7 +109,7 @@ export function set(source, value) {
 			(current_effect.f & CLEAN) !== 0 &&
 			(current_effect.f & BRANCH_EFFECT) === 0
 		) {
-			if (current_dependencies !== null && current_dependencies.includes(source)) {
+			if (new_deps !== null && new_deps.includes(source)) {
 				set_signal_status(current_effect, DIRTY);
 				schedule_effect(current_effect);
 			} else {

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

@@ -89,9 +89,16 @@ export function set_current_effect(effect) {
 	current_effect = effect;
 }
 
-/** @type {null | import('#client').Value[]} */
-export let current_dependencies = null;
-let current_dependencies_index = 0;
+/**
+ * The dependencies of the reaction that is currently being executed. In many cases,
+ * the dependencies are unchanged between runs, and so this will be `null` unless
+ * and until a new dependency is accessed — we track this via `skipped_deps`
+ * @type {null | import('#client').Value[]}
+ */
+export let new_deps = null;
+
+let skipped_deps = 0;
+
 /**
  * Tracks writes that the effect it's executed in doesn't listen to yet,
  * so that the dependency can be added to the effect later on if it then reads it
@@ -296,63 +303,56 @@ function handle_error(error, effect, component_context) {
  * @returns {V}
  */
 export function update_reaction(reaction) {
-	var previous_dependencies = current_dependencies;
-	var previous_dependencies_index = current_dependencies_index;
+	var previous_deps = new_deps;
+	var previous_skipped_deps = skipped_deps;
 	var previous_untracked_writes = current_untracked_writes;
 	var previous_reaction = current_reaction;
 	var previous_skip_reaction = current_skip_reaction;
 
-	current_dependencies = /** @type {null | import('#client').Value[]} */ (null);
-	current_dependencies_index = 0;
+	new_deps = /** @type {null | import('#client').Value[]} */ (null);
+	skipped_deps = 0;
 	current_untracked_writes = null;
 	current_reaction = (reaction.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null;
 	current_skip_reaction = !is_flushing_effect && (reaction.f & UNOWNED) !== 0;
 
 	try {
 		var result = /** @type {Function} */ (0, reaction.fn)();
-		var dependencies = /** @type {import('#client').Value<unknown>[]} **/ (reaction.deps);
+		var deps = reaction.deps;
 
-		if (current_dependencies !== null) {
+		if (new_deps !== null) {
 			var dependency;
 			var i;
 
-			if (dependencies !== null) {
-				var deps_length = dependencies.length;
-
+			if (deps !== null) {
 				/** All dependencies of the reaction, including those tracked on the previous run */
-				var array =
-					current_dependencies_index === 0
-						? current_dependencies
-						: dependencies.slice(0, current_dependencies_index).concat(current_dependencies);
+				var array = skipped_deps === 0 ? new_deps : deps.slice(0, skipped_deps).concat(new_deps);
 
 				// If we have more than 16 elements in the array then use a Set for faster performance
 				// TODO: evaluate if we should always just use a Set or not here?
-				var set =
-					array.length > 16 && deps_length - current_dependencies_index > 1 ? new Set(array) : null;
+				var set = array.length > 16 ? new Set(array) : null;
 
-				for (i = current_dependencies_index; i < deps_length; i++) {
-					dependency = dependencies[i];
+				// Remove dependencies that should no longer be tracked
+				for (i = skipped_deps; i < deps.length; i++) {
+					dependency = deps[i];
 
 					if (set !== null ? !set.has(dependency) : !array.includes(dependency)) {
 						remove_reaction(reaction, dependency);
 					}
 				}
 			}
 
-			if (dependencies !== null && current_dependencies_index > 0) {
-				dependencies.length = current_dependencies_index + current_dependencies.length;
-				for (i = 0; i < current_dependencies.length; i++) {
-					dependencies[current_dependencies_index + i] = current_dependencies[i];
+			if (deps !== null && skipped_deps > 0) {
+				deps.length = skipped_deps + new_deps.length;
+				for (i = 0; i < new_deps.length; i++) {
+					deps[skipped_deps + i] = new_deps[i];
 				}
 			} else {
-				reaction.deps = /** @type {import('#client').Value<V>[]} **/ (
-					dependencies = current_dependencies
-				);
+				reaction.deps = deps = new_deps;
 			}
 
 			if (!current_skip_reaction) {
-				for (i = current_dependencies_index; i < dependencies.length; i++) {
-					dependency = dependencies[i];
+				for (i = skipped_deps; i < deps.length; i++) {
+					dependency = deps[i];
 					var reactions = dependency.reactions;
 
 					if (reactions === null) {
@@ -365,15 +365,15 @@ export function update_reaction(reaction) {
 					}
 				}
 			}
-		} else if (dependencies !== null && current_dependencies_index < dependencies.length) {
-			remove_reactions(reaction, current_dependencies_index);
-			dependencies.length = current_dependencies_index;
+		} else if (deps !== null && skipped_deps < deps.length) {
+			remove_reactions(reaction, skipped_deps);
+			deps.length = skipped_deps;
 		}
 
 		return result;
 	} finally {
-		current_dependencies = previous_dependencies;
-		current_dependencies_index = previous_dependencies_index;
+		new_deps = previous_deps;
+		skipped_deps = previous_skipped_deps;
 		current_untracked_writes = previous_untracked_writes;
 		current_reaction = previous_reaction;
 		current_skip_reaction = previous_skip_reaction;
@@ -755,7 +755,8 @@ export async function tick() {
  * @returns {V}
  */
 export function get(signal) {
-	const flags = signal.f;
+	var flags = signal.f;
+
 	if ((flags & DESTROYED) !== 0) {
 		return signal.v;
 	}
@@ -766,26 +767,25 @@ export function get(signal) {
 
 	// Register the dependency on the current reaction signal.
 	if (current_reaction !== null) {
-		const unowned = (current_reaction.f & UNOWNED) !== 0;
-		const dependencies = current_reaction.deps;
-		if (
-			current_dependencies === null &&
-			dependencies !== null &&
-			dependencies[current_dependencies_index] === signal &&
-			!(unowned && current_effect !== null)
-		) {
-			current_dependencies_index++;
-		} else if (
-			dependencies === null ||
-			current_dependencies_index === 0 ||
-			dependencies[current_dependencies_index - 1] !== signal
-		) {
-			if (current_dependencies === null) {
-				current_dependencies = [signal];
-			} else if (current_dependencies[current_dependencies.length - 1] !== signal) {
-				current_dependencies.push(signal);
+		var deps = current_reaction.deps;
+
+		// 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++;
+		}
+
+		// Otherwise, create or push to `new_deps`, but only if this
+		// dependency wasn't the last one that was accessed
+		else if (deps === null || skipped_deps === 0 || deps[skipped_deps - 1] !== signal) {
+			if (new_deps === null) {
+				new_deps = [signal];
+			} else if (new_deps[new_deps.length - 1] !== signal) {
+				new_deps.push(signal);
 			}
 		}
+
 		if (
 			current_untracked_writes !== null &&
 			current_effect !== null &&