chore: only call `mark_reactions` when updating a source (#12272)

* only call mark_reactions when updating a source

* move mark_reactions to where it belongs, remove force_schedule stuff

* tweak

* move inspect_effects to where it is used

* this is wrong

* remove obsolete comment

* tweak

* reconnect on get

* try marking unowneds as clean

* tidy up

* tidy up

* simplify

* typo

* don't bother caching length

* fix lint

---------

Co-authored-by: Dominic Gannaway <dg@domgan.com>
pull/12278/head
Rich Harris 6 months ago committed by GitHub
parent d57491f2ce
commit 15873ab09b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -4,7 +4,6 @@ import {
current_effect, current_effect,
remove_reactions, remove_reactions,
set_signal_status, set_signal_status,
mark_reactions,
current_skip_reaction, current_skip_reaction,
update_reaction, update_reaction,
destroy_effect_children, destroy_effect_children,
@ -39,7 +38,7 @@ export function derived(fn) {
}; };
if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) { if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) {
var current_derived = /** @type {import('#client').Derived<V>} */ (current_reaction); var current_derived = /** @type {import('#client').Derived} */ (current_reaction);
if (current_derived.deriveds === null) { if (current_derived.deriveds === null) {
current_derived.deriveds = [signal]; current_derived.deriveds = [signal];
} else { } else {
@ -100,7 +99,6 @@ export function update_derived(derived) {
if (!derived.equals(value)) { if (!derived.equals(value)) {
derived.v = value; derived.v = value;
derived.version = increment_version(); derived.version = increment_version();
mark_reactions(derived, DIRTY, false);
} }
} }

@ -94,7 +94,8 @@ function create_effect(type, fn, sync, push = true) {
parent: is_root ? null : current_effect, parent: is_root ? null : current_effect,
prev: null, prev: null,
teardown: null, teardown: null,
transitions: null transitions: null,
version: 0
}; };
if (DEV) { if (DEV) {

@ -7,20 +7,28 @@ import {
current_untracked_writes, current_untracked_writes,
get, get,
is_runes, is_runes,
mark_reactions,
schedule_effect, schedule_effect,
set_current_untracked_writes, set_current_untracked_writes,
set_signal_status, set_signal_status,
untrack, untrack,
increment_version, increment_version,
update_effect, update_effect
inspect_effects
} from '../runtime.js'; } from '../runtime.js';
import { equals, safe_equals } from './equality.js'; import { equals, safe_equals } from './equality.js';
import { CLEAN, DERIVED, DIRTY, BRANCH_EFFECT } from '../constants.js'; import {
CLEAN,
DERIVED,
DIRTY,
BRANCH_EFFECT,
INSPECT_EFFECT,
UNOWNED,
MAYBE_DIRTY
} from '../constants.js';
import { UNINITIALIZED } from '../../../constants.js'; import { UNINITIALIZED } from '../../../constants.js';
import * as e from '../errors.js'; import * as e from '../errors.js';
let inspect_effects = new Set();
/** /**
* @template V * @template V
* @param {V} v * @param {V} v
@ -91,7 +99,7 @@ export function set(source, value) {
source.v = value; source.v = value;
source.version = increment_version(); source.version = increment_version();
mark_reactions(source, DIRTY, true); mark_reactions(source, DIRTY);
// If the current signal is running for the first time, it won't have any // If the current signal is running for the first time, it won't have any
// reactions as we only allocate and assign the reactions after the signal // reactions as we only allocate and assign the reactions after the signal
@ -132,3 +140,44 @@ export function set(source, value) {
return value; return value;
} }
/**
* @param {import('#client').Value} signal
* @param {number} status should be DIRTY or MAYBE_DIRTY
* @returns {void}
*/
function mark_reactions(signal, status) {
var reactions = signal.reactions;
if (reactions === null) return;
var runes = is_runes();
var length = reactions.length;
for (var i = 0; i < length; i++) {
var reaction = reactions[i];
var flags = reaction.f;
// Skip any effects that are already dirty
if ((flags & DIRTY) !== 0) continue;
// In legacy mode, skip the current effect to prevent infinite loops
if (!runes && reaction === current_effect) continue;
// Inspect effects need to run immediately, so that the stack trace makes sense
if (DEV && (flags & INSPECT_EFFECT) !== 0) {
inspect_effects.add(reaction);
continue;
}
set_signal_status(reaction, status);
// If the signal a) was previously clean or b) is an unowned derived, then mark it
if ((flags & (CLEAN | UNOWNED)) !== 0) {
if ((flags & DERIVED) !== 0) {
mark_reactions(/** @type {import('#client').Derived} */ (reaction), MAYBE_DIRTY);
} else {
schedule_effect(/** @type {import('#client').Effect} */ (reaction));
}
}
}
}

@ -3,6 +3,8 @@ import type { ComponentContext, Dom, Equals, TemplateNode, TransitionManager } f
export interface Signal { export interface Signal {
/** Flags bitmask */ /** Flags bitmask */
f: number; f: number;
/** Write version */
version: number;
} }
export interface Value<V = unknown> extends Signal { export interface Value<V = unknown> extends Signal {
@ -12,8 +14,6 @@ export interface Value<V = unknown> extends Signal {
equals: Equals; equals: Equals;
/** The latest value for this signal */ /** The latest value for this signal */
v: V; v: V;
/** Write version */
version: number;
} }
export interface Reaction extends Signal { export interface Reaction extends Signal {

@ -29,8 +29,7 @@ import {
ROOT_EFFECT, ROOT_EFFECT,
LEGACY_DERIVED_PROP, LEGACY_DERIVED_PROP,
DISCONNECTED, DISCONNECTED,
STATE_FROZEN_SYMBOL, STATE_FROZEN_SYMBOL
INSPECT_EFFECT
} from './constants.js'; } from './constants.js';
import { flush_tasks } from './dom/task.js'; import { flush_tasks } from './dom/task.js';
import { add_owner } from './dev/ownership.js'; import { add_owner } from './dev/ownership.js';
@ -63,8 +62,6 @@ export function set_is_destroying_effect(value) {
is_destroying_effect = value; is_destroying_effect = value;
} }
export let inspect_effects = new Set();
// Handle effect queues // Handle effect queues
/** @type {import('#client').Effect[]} */ /** @type {import('#client').Effect[]} */
@ -164,77 +161,44 @@ export function is_runes() {
*/ */
export function check_dirtiness(reaction) { export function check_dirtiness(reaction) {
var flags = reaction.f; var flags = reaction.f;
var is_dirty = (flags & DIRTY) !== 0;
if (is_dirty) { if ((flags & DIRTY) !== 0) {
return true; return true;
} }
var is_unowned = (flags & UNOWNED) !== 0;
var is_disconnected = (flags & DISCONNECTED) !== 0;
if ((flags & MAYBE_DIRTY) !== 0) { if ((flags & MAYBE_DIRTY) !== 0) {
var dependencies = reaction.deps; var dependencies = reaction.deps;
if (dependencies !== null) { if (dependencies !== null) {
var length = dependencies.length; var is_unowned = (flags & UNOWNED) !== 0;
var reactions;
for (var i = 0; i < length; i++) { for (var i = 0; i < dependencies.length; i++) {
var dependency = dependencies[i]; var dependency = dependencies[i];
if (!is_dirty && check_dirtiness(/** @type {import('#client').Derived} */ (dependency))) { if (check_dirtiness(/** @type {import('#client').Derived} */ (dependency))) {
update_derived(/** @type {import('#client').Derived} **/ (dependency)); update_derived(/** @type {import('#client').Derived} */ (dependency));
} }
if ((reaction.f & DIRTY) !== 0) { if (dependency.version > reaction.version) {
// `reaction` might now be dirty, as a result of calling `update_derived`
return true; return true;
} }
if (is_unowned) { if (is_unowned) {
// If we're working with an unowned derived signal, then we need to check // TODO is there a more logical place to do this work?
// if our dependency write version is higher. If it is then we can assume
// that state has changed to a newer version and thus this unowned signal
// is also dirty.
if (dependency.version > /** @type {import('#client').Derived} */ (reaction).version) {
return true;
}
if (!current_skip_reaction && !dependency?.reactions?.includes(reaction)) { if (!current_skip_reaction && !dependency?.reactions?.includes(reaction)) {
// If we are working with an unowned signal as part of an effect (due to !current_skip_reaction) // If we are working with an unowned signal as part of an effect (due to !current_skip_reaction)
// and the version hasn't changed, we still need to check that this reaction // and the version hasn't changed, we still need to check that this reaction
// if linked to the dependency source otherwise future updates will not be caught. // if linked to the dependency source otherwise future updates will not be caught.
(dependency.reactions ??= []).push(reaction); (dependency.reactions ??= []).push(reaction);
} }
} else if (is_disconnected) {
// It might be that the derived was was dereferenced from its dependencies but has now come alive again.
// In thise case, we need to re-attach it to the graph and mark it dirty if any of its dependencies have
// changed since.
if (dependency.version > /** @type {import('#client').Derived} */ (reaction).version) {
is_dirty = true;
}
reactions = dependency.reactions;
if (reactions === null) {
dependency.reactions = [reaction];
} else if (!reactions.includes(reaction)) {
reactions.push(reaction);
}
} }
} }
} }
// Unowned signals are always maybe dirty, as we instead check their dependency versions.
if (!is_unowned) {
set_signal_status(reaction, CLEAN); set_signal_status(reaction, CLEAN);
} }
if (is_disconnected) {
reaction.f ^= DISCONNECTED;
}
}
return is_dirty; return false;
} }
/** /**
@ -490,6 +454,8 @@ export function update_effect(effect) {
execute_effect_teardown(effect); execute_effect_teardown(effect);
var teardown = update_reaction(effect); var teardown = update_reaction(effect);
effect.teardown = typeof teardown === 'function' ? teardown : null; effect.teardown = typeof teardown === 'function' ? teardown : null;
effect.version = current_version;
} catch (error) { } catch (error) {
handle_error(/** @type {Error} */ (error), effect, current_component_context); handle_error(/** @type {Error} */ (error), effect, current_component_context);
} finally { } finally {
@ -798,11 +764,31 @@ export function get(signal) {
} }
} }
if ( if ((flags & DERIVED) !== 0) {
(flags & DERIVED) !== 0 && var derived = /** @type {import('#client').Derived} */ (signal);
check_dirtiness(/** @type {import('#client').Derived} */ (signal))
) { if (check_dirtiness(derived)) {
update_derived(/** @type {import('#client').Derived} **/ (signal)); update_derived(derived);
}
if ((flags & DISCONNECTED) !== 0) {
// reconnect to the graph
deps = derived.deps;
if (deps !== null) {
for (var i = 0; i < deps.length; i++) {
var dep = deps[i];
var reactions = dep.reactions;
if (reactions === null) {
dep.reactions = [derived];
} else if (!reactions.includes(derived)) {
reactions.push(derived);
}
}
}
derived.f ^= DISCONNECTED;
}
} }
return signal.v; return signal.v;
@ -845,52 +831,6 @@ export function invalidate_inner_signals(fn) {
} }
} }
/**
* @param {import('#client').Value} signal
* @param {number} status should be DIRTY or MAYBE_DIRTY
* @param {boolean} force_schedule
* @returns {void}
*/
export function mark_reactions(signal, status, force_schedule) {
var reactions = signal.reactions;
if (reactions === null) return;
var runes = is_runes();
var length = reactions.length;
for (var i = 0; i < length; i++) {
var reaction = reactions[i];
var flags = reaction.f;
if (DEV && (flags & INSPECT_EFFECT) !== 0) {
inspect_effects.add(reaction);
continue;
}
// We skip any effects that are already dirty. Additionally, we also
// skip if the reaction is the same as the current effect (except if we're not in runes or we
// are in force schedule mode).
if ((flags & DIRTY) !== 0 || ((!force_schedule || !runes) && reaction === current_effect)) {
continue;
}
set_signal_status(reaction, status);
// If the signal a) was previously clean or b) is an unowned derived, then mark it
if ((flags & (CLEAN | UNOWNED)) !== 0) {
if ((flags & DERIVED) !== 0) {
mark_reactions(
/** @type {import('#client').Derived} */ (reaction),
MAYBE_DIRTY,
force_schedule
);
} else {
schedule_effect(/** @type {import('#client').Effect} */ (reaction));
}
}
}
}
/** /**
* Use `untrack` to prevent something from being treated as an `$effect`/`$derived` dependency. * Use `untrack` to prevent something from being treated as an `$effect`/`$derived` dependency.
* *

Loading…
Cancel
Save