restore stale_readers dedupe

Without it, a reaction that reads N stale values queues N re-runs, each of which flushes the
later batch — quadratic work (seconds for a 2000-item each block). The new test is a timing
guard: the output is identical either way.
async-another-try-pt-1-prune
Rich Harris 3 days ago
parent 4fef105eaa
commit 7a0b430ec5
No known key found for this signature in database

@ -143,6 +143,21 @@ export class Batch {
return (this.#stale_effects ??= new Map());
}
/**
* @type {Set<Reaction> | null}
*/
#stale_readers = null;
/**
* Reactions that, while running in an earlier batch, read a value that this batch holds
* a newer version of, and that are therefore scheduled to re-run in this batch. Used to
* avoid scheduling the same reaction multiple times when it reads more than one such value.
* Lazily initialized for performance reasons.
*/
get stale_readers() {
return (this.#stale_readers ??= new Set());
}
/** @type {Batch | null} */
prev = null;

@ -771,18 +771,27 @@ function get_batch_value(signal, first_time) {
(active_effect.f & REACTION_IS_UPDATING) !== 0) ||
(active_effect !== null && (active_effect.f & ASYNC) !== 0);
if (reader !== null && reactive) {
if (
reader !== null &&
reactive &&
// a reaction can read several stale values in one run — only schedule the re-run once
!batch.stale_readers.has(reader)
) {
batch.stale_readers.add(reader);
if (current.is_eager) {
// Reactions of eager effects with at least one non-eager effect (which we know is true here due to the stale_sources check above)
// need to rerun because they could do something like `$state.eager(count) !== count`.
if (reader.deps?.some((dep) => dep.f & EAGER_EFFECT)) {
batch.oncommit(() => {
batch.stale_readers.delete(reader);
Batch.ensure();
invalidate(reader, DIRTY);
});
}
} else {
queue_micro_task(() => {
batch.stale_readers.delete(reader);
const b = batch.activate();
invalidate(reader, MAYBE_DIRTY);
b.flush();

@ -0,0 +1,39 @@
import { tick } from 'svelte';
import { test } from '../../test';
const buttons = '<button>show</button><button>items</button><button>shift</button>';
/** @param {number} offset */
const list = (offset) => Array.from({ length: 2000 }, (_, i) => `${i + offset},`).join('');
// When the branch is created in B1, the each block reads all 2000 items, each of which B2 holds
// a newer value of. The each block must be scheduled to re-run in B2 once, not once per stale
// value — otherwise B2 is flushed 2000 times (each flush being O(size of B2)), which takes seconds
export default test({
mode: ['client'],
async test({ assert, target }) {
await tick();
const [show, items, shift] = target.querySelectorAll('button');
// B1: show = true -> pending
show.click();
await tick();
// B2: increment all items -> pending
items.click();
await tick();
// B1 resolves -> the branch is created, reading 2000 values that are stale relative to B2
const start = performance.now();
shift.click();
await tick();
const duration = performance.now() - start;
assert.ok(duration < 1000, `took ${duration}ms`);
assert.htmlEqual(target.innerHTML, `${buttons}<p>0</p><div>${list(0)}</div>`);
// B2 resolves
shift.click();
await tick();
assert.htmlEqual(target.innerHTML, `${buttons}<p>1</p><div>${list(1)}</div>`);
}
});

@ -0,0 +1,23 @@
<script>
let show = $state(false);
let items = $state(Array.from({ length: 2000 }, (_, i) => i));
const deferred = [];
function delay(v) {
if (!v) return v;
return new Promise((resolve) => deferred.push(() => resolve(v)));
}
</script>
<button onclick={() => (show = true)}>show</button>
<button onclick={() => items.forEach((_, i) => items[i]++)}>items</button>
<button onclick={() => deferred.shift()?.()}>shift</button>
<p>{await delay(items[0])}</p>
{#if await delay(show)}
<div>
{#each items as item}{item},{/each}
</div>
{/if}
Loading…
Cancel
Save