diff --git a/.changeset/grumpy-pens-press.md b/.changeset/grumpy-pens-press.md new file mode 100644 index 0000000000..0ba87ab0c5 --- /dev/null +++ b/.changeset/grumpy-pens-press.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove `WAS_MARKED` flag in favor of `Set` diff --git a/benchmarking/benchmarks/reactivity/index.js b/benchmarking/benchmarks/reactivity/index.js index 3fe9639376..d971b25e9b 100644 --- a/benchmarking/benchmarks/reactivity/index.js +++ b/benchmarking/benchmarks/reactivity/index.js @@ -13,7 +13,7 @@ import { sbench_create_4to1, sbench_create_signals } from './sbench.js'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { create_test } from './util.js'; // This benchmark has been adapted from the js-reactivity-benchmark (https://github.com/milomg/js-reactivity-benchmark) @@ -39,7 +39,8 @@ for (const file of fs.readdirSync(`${dirname}/tests`)) { const name = file.replace('.bench.js', ''); - const module = await import(`${dirname}/tests/${file}`); + const module_url = pathToFileURL(path.join(dirname, 'tests', file)); + const module = await import(module_url.href); const { owned, unowned } = create_test(name, module.default); reactivity_benchmarks.push(owned, unowned); diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 043b50b4b2..578e9988aa 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -44,15 +44,6 @@ export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; export const EFFECT_OFFSCREEN = 1 << 25; -// Flags exclusive to deriveds -/** - * 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). This is a pure performance optimization flag and - * should not be used for any other purpose! - */ -export const WAS_MARKED = 1 << 16; - // Flags used for async export const REACTION_IS_UPDATING = 1 << 21; export const ASYNC = 1 << 22; diff --git a/packages/svelte/src/internal/client/dev/debug.js b/packages/svelte/src/internal/client/dev/debug.js index 83cc510ae2..2513d15c97 100644 --- a/packages/svelte/src/internal/client/dev/debug.js +++ b/packages/svelte/src/internal/client/dev/debug.js @@ -15,7 +15,6 @@ import { MAYBE_DIRTY, RENDER_EFFECT, ROOT_EFFECT, - WAS_MARKED, MANAGED_EFFECT } from '#client/constants'; import { snapshot } from '../../shared/clone.js'; @@ -204,7 +203,6 @@ export function log_reactions(signal) { if ((flags & DIRTY) !== 0) names.push('DIRTY'); if ((flags & MAYBE_DIRTY) !== 0) names.push('MAYBE_DIRTY'); if ((flags & CONNECTED) !== 0) names.push('CONNECTED'); - if ((flags & WAS_MARKED) !== 0) names.push('WAS_MARKED'); if ((flags & INERT) !== 0) names.push('INERT'); if ((flags & DESTROYED) !== 0) names.push('DESTROYED'); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index fd64f3b45d..5a9ac51587 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -9,7 +9,6 @@ import { EFFECT_PRESERVED, STALE_REACTION, ASYNC, - WAS_MARKED, DESTROYED, CLEAN, REACTION_RAN, @@ -364,7 +363,6 @@ export function execute_derived(derived) { stack.push(derived); - derived.f &= ~WAS_MARKED; destroy_derived_effects(derived); value = update_reaction(derived); } finally { @@ -374,7 +372,6 @@ 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 218941ab67..a4647b38ef 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -25,10 +25,7 @@ import { MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, - ASYNC, - WAS_MARKED, - CONNECTED, - REACTION_IS_UPDATING + ASYNC } from '#client/constants'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -171,6 +168,17 @@ export function set(source, value, should_proxy = false) { return internal_set(source, new_value, legacy_updates); } +/** + * A set of signals we have already seen while traversing in mark_reactions. + * Not always set to balance the common case of sources only having a couple + * of (transitive) dependencies (where always creating a Set would be bad for perf) + * with the edge case of extremely deep or wide dependency arrays with cycles. + * @type {Set | null} + */ +var seen = null; +/** Number of transitive dependencies, see {@link seen} for more info */ +var count_deps = 0; + /** * @template V * @param {Source} source @@ -234,7 +242,10 @@ export function internal_set(source, value, updated_during_traversal = null) { // For debugging, in case you want to know which reactions are being scheduled: // log_reactions(source); + seen = null; + count_deps = 0; mark_reactions(source, DIRTY, updated_during_traversal); + seen = null; // It's possible that the current reaction might not have up-to-date dependencies // whilst it's actively running. So in the case of ensuring it registers the reaction @@ -341,6 +352,16 @@ function mark_reactions(signal, status, updated_during_traversal) { var runes = is_runes(); var length = reactions.length; + count_deps += length; + // Activate the `seen` Set if we think from the unusually high number of deps that + // there might be cycles in the graph, to avoid repeated lookups for reactions + if (count_deps > 1000000 && seen === null) seen = new Set(); + + if (seen !== null) { + if (seen.has(signal)) return; + seen.add(signal); + } + for (var i = 0; i < length; i++) { var reaction = reactions[i]; var flags = reaction.f; @@ -364,18 +385,7 @@ function mark_reactions(signal, status, updated_during_traversal) { var derived = /** @type {Derived} */ (reaction); batch_values?.delete(derived); - - if ((flags & WAS_MARKED) === 0) { - // Only connected deriveds being executed outside the update cycle can be reliably unmarked right away - if ( - flags & CONNECTED && - (active_effect === null || (active_effect.f & REACTION_IS_UPDATING) === 0) - ) { - reaction.f |= WAS_MARKED; - } - - mark_reactions(derived, MAYBE_DIRTY, updated_during_traversal); - } + mark_reactions(derived, MAYBE_DIRTY, updated_during_traversal); } else if (not_dirty) { var effect = /** @type {Effect} */ (reaction); diff --git a/packages/svelte/src/internal/client/reactivity/utils.js b/packages/svelte/src/internal/client/reactivity/utils.js index 0d27cb8b84..3280f2372e 100644 --- a/packages/svelte/src/internal/client/reactivity/utils.js +++ b/packages/svelte/src/internal/client/reactivity/utils.js @@ -1,24 +1,7 @@ -/** @import { Derived, Effect, Value } from '#client' */ -import { CLEAN, DERIVED, DIRTY, MAYBE_DIRTY, WAS_MARKED } from '#client/constants'; +/** @import { Effect } from '#client' */ +import { CLEAN, DIRTY, MAYBE_DIRTY } from '#client/constants'; import { set_signal_status } from './status.js'; -/** - * @param {Value[] | null} deps - */ -function clear_marked(deps) { - if (deps === null) return; - - for (const dep of deps) { - if ((dep.f & DERIVED) === 0 || (dep.f & WAS_MARKED) === 0) { - continue; - } - - dep.f ^= WAS_MARKED; - - clear_marked(/** @type {Derived} */ (dep).deps); - } -} - /** * @param {Effect} effect * @param {Set} dirty_effects @@ -31,10 +14,6 @@ export function defer_effect(effect, dirty_effects, maybe_dirty_effects) { maybe_dirty_effects.add(effect); } - // Since we're not executing these effects now, we need to clear any WAS_MARKED flags - // so that other batches can correctly reach these effects during their own traversal - clear_marked(effect.deps); - // mark as clean so they get scheduled if they depend on pending async state set_signal_status(effect, CLEAN); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 8d70eee43c..9c4906d1b5 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -21,7 +21,6 @@ import { REACTION_IS_UPDATING, STALE_REACTION, ERROR_VALUE, - WAS_MARKED, MANAGED_EFFECT, REACTION_RAN } from './constants.js'; @@ -156,10 +155,6 @@ export function is_dirty(reaction) { return true; } - if (flags & DERIVED) { - reaction.f &= ~WAS_MARKED; - } - if ((flags & MAYBE_DIRTY) !== 0) { var dependencies = /** @type {Value[]} */ (reaction.deps); var length = dependencies.length; @@ -388,11 +383,8 @@ function remove_reaction(signal, dependency) { ) { var derived = /** @type {Derived} */ (dependency); - // If we are working with a derived that is owned by an effect, then mark it as being - // disconnected and remove the mark flag, as it cannot be reliably removed otherwise if ((derived.f & CONNECTED) !== 0) { derived.f ^= CONNECTED; - derived.f &= ~WAS_MARKED; } // In a fork it's possible that a derived is executed and gets reactions, then commits, but is