fix: perf regression with async mode (#17461)

* perf: use Set for new_deps to avoid O(n) includes check

* fix: only call revive() when batch is no longer deferred

* fix: avoid unnecessary async tracking when blockers are already settled

* add changeset

* only record promises as settled after cleanup is complete

* don't flush when already flushing

* skip settled blockers more aggressively

* batch decrement and pending count updates to reduce flush pressure

* extract new_deps changes to a separate PR

* remove changeset that applies to the other branch

* mark promises as settled as they settle — at the end is too late to do any good

* remove unnecessary microtask

* extract batch changes to separate PR

* this too

* avoid assigning to parameter

* wrong promise added to settled_promises due to reassignment

* WIP

* WIP

* fix

* unused

* oops

* bad import

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/17522/head
David 8 months ago committed by GitHub
parent f9cc2d25b9
commit 7719d0312c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: avoid async overhead for already settled promises

@ -115,9 +115,9 @@ function add_const_declaration(state, id, expression, metadata, bindings) {
const body = after.length === 0 ? assignment : b.block([b.stmt(assignment), ...after]);
if (blockers.length === 1) {
run.thunks.push(b.thunk(/** @type {Expression} */ (blockers[0])));
run.thunks.push(b.thunk(b.member(/** @type {Expression} */ (blockers[0]), 'promise')));
} else if (blockers.length > 0) {
run.thunks.push(b.thunk(b.call('Promise.all', b.array(blockers))));
run.thunks.push(b.thunk(b.call('$.wait', b.array(blockers))));
}
run.thunks.push(b.thunk(body, has_await));

@ -1,4 +1,4 @@
/** @import { TemplateNode, Value } from '#client' */
/** @import { Blocker, TemplateNode, Value } from '#client' */
import { flatten } from '../../reactivity/async.js';
import { Batch, current_batch } from '../../reactivity/batch.js';
import { get } from '../../runtime.js';
@ -14,11 +14,16 @@ import { get_boundary } from './boundary.js';
/**
* @param {TemplateNode} node
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
* @param {Array<() => Promise<any>>} expressions
* @param {(anchor: TemplateNode, ...deriveds: Value[]) => void} fn
*/
export function async(node, blockers = [], expressions = [], fn) {
if (expressions.length === 0 && blockers.every((b) => b.settled)) {
fn(node);
return;
}
var boundary = get_boundary();
var batch = /** @type {Batch} */ (current_batch);
var blocking = boundary.is_rendered();

@ -1,4 +1,4 @@
/** @import { Effect } from '#client' */
/** @import { Blocker, Effect } from '#client' */
import { DEV } from 'esm-env';
import { hydrating, set_hydrating } from '../hydration.js';
import { get_descriptors, get_prototype_of } from '../../../shared/utils.js';
@ -483,7 +483,7 @@ function set_attributes(
* @param {(...expressions: any) => Record<string | symbol, any>} fn
* @param {Array<() => any>} sync
* @param {Array<() => Promise<any>>} async
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
* @param {string} [css_hash]
* @param {boolean} [should_remove_defaults]
* @param {boolean} [skip_warning]

@ -1,4 +1,4 @@
/** @import { Effect, TemplateNode, Value } from '#client' */
/** @import { Blocker, Effect, Value } from '#client' */
import { DESTROYED, STALE_REACTION } from '#client/constants';
import { DEV } from 'esm-env';
import {
@ -27,7 +27,7 @@ import {
import { aborted } from './effects.js';
/**
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
* @param {Array<() => any>} sync
* @param {Array<() => Promise<any>>} async
* @param {(values: Value[]) => any} fn
@ -35,7 +35,10 @@ import { aborted } from './effects.js';
export function flatten(blockers, sync, async, fn) {
const d = is_runes() ? derived : derived_safe_equal;
if (async.length === 0 && blockers.length === 0) {
// Filter out already-settled blockers - no need to wait for them
var pending = blockers.filter((b) => !b.settled);
if (async.length === 0 && pending.length === 0) {
fn(sync.map(d));
return;
}
@ -44,47 +47,52 @@ export function flatten(blockers, sync, async, fn) {
var parent = /** @type {Effect} */ (active_effect);
var restore = capture();
var blocker_promise =
pending.length === 1
? pending[0].promise
: pending.length > 1
? Promise.all(pending.map((b) => b.promise))
: null;
/** @param {Value[]} values */
function finish(values) {
restore();
function run() {
Promise.all(async.map((expression) => async_derived(expression)))
.then((result) => {
restore();
try {
fn(values);
} catch (error) {
if ((parent.f & DESTROYED) === 0) {
invoke_error_boundary(error, parent);
}
}
try {
fn([...sync.map(d), ...result]);
} catch (error) {
// ignore errors in blocks that have already been destroyed
if ((parent.f & DESTROYED) === 0) {
invoke_error_boundary(error, parent);
}
}
batch?.deactivate();
unset_context();
}
batch?.deactivate();
unset_context();
})
.catch((error) => {
invoke_error_boundary(error, parent);
});
// Fast path: blockers but no async expressions
if (async.length === 0) {
/** @type {Promise<any>} */ (blocker_promise).then(() => finish(sync.map(d)));
return;
}
if (blockers.length > 0) {
Promise.all(blockers).then(() => {
restore();
// Full path: has async expressions
function run() {
restore();
Promise.all(async.map((expression) => async_derived(expression)))
.then((result) => finish([...sync.map(d), ...result]))
.catch((error) => invoke_error_boundary(error, parent));
}
try {
return run();
} finally {
batch?.deactivate();
unset_context();
}
});
if (blocker_promise) {
blocker_promise.then(run);
} else {
run();
}
}
/**
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
* @param {(values: Value[]) => any} fn
*/
export function run_after_blockers(blockers, fn) {
@ -239,7 +247,13 @@ export function run(thunks) {
var promise = Promise.resolve(thunks[0]()).catch(handle_error);
var promises = [promise];
/** @type {Blocker} */
var blocker = { promise, settled: false };
var blockers = [blocker];
promise.finally(() => {
blocker.settled = true;
});
for (const fn of thunks.slice(1)) {
promise = promise
@ -255,13 +269,17 @@ export function run(thunks) {
restore();
return fn();
})
.catch(handle_error)
.finally(() => {
unset_context();
current_batch?.deactivate();
});
.catch(handle_error);
const blocker = { promise, settled: false };
blockers.push(blocker);
promises.push(promise);
promise.finally(() => {
blocker.settled = true;
unset_context();
current_batch?.deactivate();
});
}
promise
@ -273,5 +291,12 @@ export function run(thunks) {
batch.decrement(blocking);
});
return promises;
return blockers;
}
/**
* @param {Blocker[]} blockers
*/
export function wait(blockers) {
return Promise.all(blockers.map((b) => b.promise));
}

@ -1,4 +1,4 @@
/** @import { ComponentContext, ComponentContextLegacy, Derived, Effect, TemplateNode, TransitionManager } from '#client' */
/** @import { Blocker, ComponentContext, ComponentContextLegacy, Derived, Effect, TemplateNode, TransitionManager } from '#client' */
import {
is_dirty,
active_effect,
@ -361,7 +361,7 @@ export function render_effect(fn, flags = 0) {
* @param {(...expressions: any) => void | (() => void)} fn
* @param {Array<() => any>} sync
* @param {Array<() => Promise<any>>} async
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
*/
export function template_effect(fn, sync = [], async = [], blockers = []) {
flatten(blockers, sync, async, (values) => {
@ -374,7 +374,7 @@ export function template_effect(fn, sync = [], async = [], blockers = []) {
* @param {(...expressions: any) => void | (() => void)} fn
* @param {Array<() => any>} sync
* @param {Array<() => Promise<any>>} async
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
*/
export function deferred_template_effect(fn, sync = [], async = [], blockers = []) {
var batch = /** @type {Batch} */ (current_batch);

@ -103,3 +103,8 @@ export interface Effect extends Reaction {
export type Source<V = unknown> = Value<V>;
export type MaybeSource<T = unknown> = T | Source<T>;
export interface Blocker {
promise: Promise<any>;
settled: boolean;
}

@ -1,3 +1,4 @@
/** @import { Blocker } from '#client' */
import { dev_current_component_function } from './context.js';
import { is_array } from '../shared/utils.js';
import * as e from './errors.js';
@ -41,7 +42,7 @@ export function validate_each_keys(collection, key_fn) {
/**
* @param {string} binding
* @param {Array<Promise<void>>} blockers
* @param {Blocker[]} blockers
* @param {() => Record<string, any>} get_object
* @param {() => string} get_property
* @param {number} line

Loading…
Cancel
Save