consolidate + fix another bug that exists on main

entangle-batches-2
Simon Holthausen 2 months ago
parent 01c124c00b
commit a9aef844ab
No known key found for this signature in database

@ -61,29 +61,18 @@ let last_batch = null;
export let current_batch = null;
/**
* This is needed to avoid overwriting inputs
* The batch whose world is currently applied. This can differ from
* `current_batch`, e.g. while a batch's effects are being flushed (during which
* `current_batch` is `null`, or a new batch created by writes inside effects).
* @type {Batch | null}
*/
export let previous_batch = null;
/**
* When time travelling (i.e. working in one batch, while other batches
* still have ongoing work), we ignore the real values of affected
* signals in favour of their values within the batch.
* Entries are `[value, owner]` tuples `owner` is the live batch whose
* pre-write value we are seeing (so that the reader can be re-run when
* that batch commits), or `null` if the value belongs to the current batch
* @type {Map<Value, [any, Batch | null]> | null}
*/
export let batch_values = null;
export let active_batch = null;
/**
* The batch whose world `batch_values` currently reflects. This can differ from
* `current_batch`, e.g. while a batch's effects are being flushed (during which
* `current_batch` is `null`, or a new batch created by writes inside effects)
* This is needed to avoid overwriting inputs
* @type {Batch | null}
*/
let batch_values_owner = null;
export let previous_batch = null;
/** @type {Effect | null} */
let last_scheduled_effect = null;
@ -174,6 +163,15 @@ export class Batch {
*/
previous = new Map();
/**
* The values visible in this batch's world. Entries are `[value, owner]`
* tuples. `owner` is the live batch whose pre-write value we are seeing,
* or `null` if the value belongs to this batch. For forks this also stores
* world-local derived values between activations.
* @type {Map<Value, [any, Batch | null]> | null}
*/
values = null;
/**
* When the batch is committed (and the DOM is updated), we need to remove old branches
* and append new ones by calling the functions added inside (if/each/key/etc) blocks.
@ -271,16 +269,6 @@ export class Batch {
*/
merged_into = null;
/**
* Deriveds evaluated inside this fork. Forks don't write values through
* this map is the fork's own view of the affected part of the graph, and is
* discarded along with the fork. Committing writes the fork's sources
* through, after which deriveds recompute in the real world.
* `null` unless this batch is (or was) a fork.
* @type {Map<Value, any> | null}
*/
fork_values = null;
/**
* Async and block effects that ran or were proven clean inside this fork.
* The version is that of the latest real-world execution when the effect was
@ -291,8 +279,8 @@ export class Batch {
fork_effects = null;
/**
* Reactions that observed the pre-write world of this batch (via
* `batch_values`) while it was pending. When this batch commits, they
* Reactions that observed the pre-write world of this batch via its active
* overlay while it was pending. When this batch commits, they
* re-run with the real values.
* Lazily initialised for perf reasons
* @type {Set<Reaction> | null}
@ -719,11 +707,13 @@ export class Batch {
other.merged_into = this;
other.#unlink();
// if we're mid-flush, `batch_values` was holding back `other`'s values —
// if we're mid-flush, the active overlay was holding back `other`'s values —
// they are part of this batch's world now, so recompute the overrides
if (batch_values !== null && current_batch === this) {
if (active_batch !== null && active_batch.values !== null && active_batch.resolved() === this) {
this.apply();
}
other.values = null;
}
/**
@ -928,8 +918,9 @@ export class Batch {
if (async_mode_flag) {
// now that this batch is committed, reactions that observed its
// pre-write values (via `batch_values`) re-run with the real ones
// pre-write values (via the active overlay) re-run with the real ones
this.#commit();
this.values = null;
// #commit may have scheduled stale readers into a new batch
next_batch ??= /** @type {Batch | null} */ (/** @type {unknown} */ (current_batch));
@ -1056,10 +1047,10 @@ export class Batch {
capture(source, value) {
this.record_previous(source);
// Don't save errors in `batch_values`, or they won't be thrown in `runtime.js#get`
// Don't save errors in the active overlay, or they won't be thrown in `runtime.js#get`
if ((source.f & ERROR_VALUE) === 0) {
this.current.set(source, value);
batch_values?.set(source, [value, null]);
(active_batch ?? (this.is_fork ? this : null))?.values?.set(source, [value, null]);
}
if (!this.is_fork) {
@ -1073,8 +1064,7 @@ export class Batch {
deactivate() {
current_batch = null;
batch_values = null;
batch_values_owner = null;
active_batch = null;
}
flush() {
@ -1095,8 +1085,7 @@ export class Batch {
is_processing = false;
current_batch = null;
batch_values = null;
batch_values_owner = null;
active_batch = null;
old_values.clear();
@ -1110,6 +1099,7 @@ export class Batch {
discard() {
this.fork_effects = null;
this.values = null;
if (this.#discard_callbacks !== null) {
for (const fn of this.#discard_callbacks) fn(this);
@ -1142,6 +1132,7 @@ export class Batch {
for (const signal of this.current.keys()) {
fork.current.delete(signal);
fork.previous.delete(signal);
fork.values?.delete(signal);
}
}
@ -1295,24 +1286,26 @@ export class Batch {
apply() {
var batch = this.resolved();
active_batch = batch;
if (!async_mode_flag || (!batch.is_fork && batch.#prev === null && batch.#next === null)) {
batch_values = null;
batch_values_owner = null;
batch.values = null;
return;
}
if (batch.is_fork) {
batch.values ??= new Map();
return;
}
/** @type {Map<Value, [any, Batch | null]>} */
var values = (batch_values = new Map());
batch_values_owner = batch;
var values = (batch.values = new Map());
// undo changes belonging to other live batches — aside from our own
// changes, we should only see values that have been committed. Overlapping
// batches are merged unless one is waiting behind a sealed predecessor
for (let other = first_batch; other !== null; other = other.#next) {
// Forks are based on the latest real world, not an independently
// held-back snapshot of it. Other forks remain separate worlds.
if (other === batch || other.is_fork || batch.is_fork) continue;
if (other === batch || other.is_fork) continue;
for (const [source, previous] of other.previous) {
if (!values.has(source)) {
@ -1321,17 +1314,10 @@ export class Batch {
}
}
// our own writes (and, in a fork, deriveds evaluated inside the fork)
// take precedence over everything else
// our own writes take precedence over everything else
for (const [source, value] of batch.current) {
values.set(source, [value, null]);
}
if (batch.fork_values !== null) {
for (const [derived, value] of batch.fork_values) {
values.set(derived, [value, null]);
}
}
}
/**
@ -1563,7 +1549,7 @@ export function claimed_by_other(reaction) {
owner = owner.resolved();
reaction.batch = owner;
return owner.linked && owner !== batch_values_owner ? owner : null;
return owner.linked && owner !== active_batch?.resolved() ? owner : null;
}
/** @type {Source<number>[]} */
@ -1613,13 +1599,13 @@ export function eager(fn) {
if (initial) {
// the first time this runs, we create an eager effect
// that will run eagerly whenever the expression changes
var previous_batch_values = batch_values;
var previous_active_batch = active_batch;
try {
batch_values = null;
active_batch = null;
value = fn();
} finally {
batch_values = previous_batch_values;
active_batch = previous_active_batch;
}
return;
@ -1778,7 +1764,6 @@ export function fork(fn) {
var batch = Batch.ensure();
batch.is_fork = true;
batch.fork_values = new Map();
batch.apply();
var committed = false;
@ -1804,7 +1789,7 @@ export function fork(fn) {
committed = true;
batch.is_fork = false;
batch.fork_values = null;
batch.values = null;
// Write the fork's changes through and invalidate affected deriveds
// and template/user effects, so they recompute with the committed
@ -1863,4 +1848,5 @@ export function fork(fn) {
*/
export function clear() {
first_batch = last_batch = null;
active_batch = null;
}

@ -36,7 +36,7 @@ import { get_error } from '../../shared/dev.js';
import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js';
import { component_context } from '../context.js';
import { UNINITIALIZED } from '../../../constants.js';
import { batch_values, current_batch, previous_batch } from './batch.js';
import { current_batch, previous_batch } from './batch.js';
import { increment_pending, unset_context } from './async.js';
import { deferred, includes, noop } from '../../shared/utils.js';
import { update_derived_status } from './status.js';
@ -397,7 +397,7 @@ export function execute_derived(derived) {
export function update_derived(derived) {
var value = execute_derived(derived);
var batch = current_batch ?? previous_batch;
var fork_values = batch !== null && batch.is_fork ? batch.fork_values : null;
var fork_values = batch !== null && batch.is_fork ? batch.values : null;
if (fork_values !== null && derived.deps !== null) {
// Inside a fork, neither the underlying value nor the status are
@ -406,17 +406,20 @@ export function update_derived(derived) {
// real status keeps describing the real (untouched) value. (Deriveds
// without dependencies never recompute, so they are treated like the
// real world below.)
var previous = fork_values.has(derived) ? fork_values.get(derived) : derived.v;
var override = fork_values.get(derived);
var previous = override === undefined ? derived.v : override[0];
if (
previous === UNINITIALIZED ||
// We cannot rely on raw `derived.equals` here, even if it itself does some logic to
// get the value from current_batch if possible, because in the context of deriveds
// we also do need to check previous_batch (see above).
!derived.equals.call(/** @type {any} */ ({ v: previous }), value)
) {
derived.wv = increment_write_version();
}
fork_values.set(derived, value);
batch_values?.set(derived, [value, null]);
fork_values.set(derived, [value, null]);
return;
}

@ -1,8 +1,16 @@
/** @import { Equals } from '#client' */
/** @import { Equals, Value } from '#client' */
import { active_batch, current_batch } from './batch.js';
/** @param {Value} signal */
function get_value(signal) {
var batch = active_batch ?? (current_batch?.is_fork ? current_batch : null);
var override = batch?.values?.get(signal);
return override === undefined ? signal.v : override[0];
}
/** @type {Equals} */
export function equals(value) {
return value === this.v;
return value === get_value(this);
}
/**
@ -27,5 +35,5 @@ export function not_equal(a, b) {
/** @type {Equals} */
export function safe_equals(value) {
return !safe_not_equal(value, this.v);
return !safe_not_equal(value, get_value(this));
}

@ -36,8 +36,8 @@ import { tag_proxy } from '../dev/tracing.js';
import { get_error } from '../../shared/dev.js';
import { component_context, is_runes } from '../context.js';
import {
active_batch,
Batch,
batch_values,
current_batch,
eager_block_effects,
schedule_effect,
@ -371,8 +371,10 @@ export function mark_reactions(signal, status, updated_during_traversal) {
current_batch?.claim(derived);
// invalidate any world-local memoized values
batch_values?.delete(derived);
current_batch?.fork_values?.delete(derived);
active_batch?.values?.delete(derived);
if (active_batch !== current_batch && current_batch?.is_fork) {
current_batch.values?.delete(derived);
}
if ((flags & WAS_MARKED) === 0) {
// Only connected deriveds being executed outside the update cycle can be reliably unmarked right away

@ -48,8 +48,8 @@ import {
set_dev_stack
} from './context.js';
import {
active_batch,
Batch,
batch_values,
claimed_by_other,
current_batch,
flushSync,
@ -173,7 +173,7 @@ export function is_dirty(reaction) {
for (var i = 0; i < length; i++) {
var dependency = dependencies[i];
if (batch_values !== null && (dependency.f & DERIVED) !== 0) {
if (active_batch !== null && active_batch.values !== null && (dependency.f & DERIVED) !== 0) {
var is_template = (dependency.f & TEMPLATE_EXPRESSION) !== 0;
if (is_template || claimed_by_other(/** @type {Derived} */ (dependency)) !== null) {
@ -686,23 +686,24 @@ export function get(signal) {
// context of the current world, without touching their cached state:
// - deriveds belonging to another live batch's world must not be
// recomputed or have their status reset (the owning batch relies on
// both) — their value in this world follows from `batch_values`
// both) — their value in this world follows from the active overlay
// - dirty template expression deriveds are leaves that can be shared
// between non-overlapping batches, so each world evaluates its own value
/** @type {Batch | null} */
var owner = null;
if (
batch_values !== null &&
active_batch !== null &&
active_batch.values !== null &&
((derived.f & TEMPLATE_EXPRESSION) !== 0
? (derived.f & (DIRTY | MAYBE_DIRTY)) !== 0
: (owner = claimed_by_other(derived)) !== null)
) {
// the world-local value is memoized in `batch_values` (and invalidated
// the world-local value is memoized in the active overlay (and invalidated
// there when dependencies change). Reads are registered with the owner
// batch — when it commits, the reader re-runs with the real values
if (!batch_values.has(derived)) {
batch_values.set(derived, [execute_derived(derived), owner]);
if (!active_batch.values.has(derived)) {
active_batch.values.set(derived, [execute_derived(derived), owner]);
}
} else {
// connect disconnected deriveds if we are reading them inside an effect,
@ -732,8 +733,8 @@ export function get(signal) {
}
}
if (batch_values !== null) {
var override = batch_values.get(signal);
if (active_batch !== null && active_batch.values !== null) {
var override = active_batch.values.get(signal);
if (override !== undefined) {
// if we're seeing another live batch's pre-write world, it must

@ -0,0 +1,17 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
const [create, commit] = target.querySelectorAll('button');
const [p] = target.querySelectorAll('p');
create.click();
await tick();
assert.htmlEqual(p.innerHTML, '0:0');
commit.click();
await tick();
assert.htmlEqual(p.innerHTML, '0:0');
}
});

@ -0,0 +1,20 @@
<script>
import { fork } from 'svelte';
let source = $state(0);
let writable = $derived(source);
let pending;
</script>
<button onclick={() => {
pending = fork(() => {
source = 1;
source = 0;
writable = 1;
writable = 0;
});
}}>fork</button>
<button onclick={() => pending.commit()}>commit</button>
<p>{source}:{writable}</p>
Loading…
Cancel
Save