From 3fe4940a9da82b8e95db7074c0d6171c685fc911 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 27 Feb 2024 14:55:31 +0100 Subject: [PATCH] perf: bail early when traversing non-state (#10654) This has a lot of overhead for large lists, and we can at least diminish in the "no state proxy" case by applying a sensible heuristic: - If the value passed is a state proxy, read it - If not, and if the value is an array, then bail because an array of state proxies is highly unlikely - Traverse the first level of properties of the object and look if these are state, if not bail. State proxies nested further down are highly unlikely, too part of #10637 --- .changeset/chatty-sloths-allow.md | 5 ++++ .../client/visitors/javascript-legacy.js | 2 +- packages/svelte/src/internal/client/render.js | 28 +++++++++++-------- .../svelte/src/internal/client/runtime.js | 23 +++++++++++++++ packages/svelte/src/internal/index.js | 3 +- 5 files changed, 47 insertions(+), 14 deletions(-) create mode 100644 .changeset/chatty-sloths-allow.md diff --git a/.changeset/chatty-sloths-allow.md b/.changeset/chatty-sloths-allow.md new file mode 100644 index 000000000000..09146ea12c8a --- /dev/null +++ b/.changeset/chatty-sloths-allow.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +perf: bail early when traversing non-state diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js index 02f4895ae9d4..9ffeb1df8b67 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js @@ -170,7 +170,7 @@ export const javascript_visitors_legacy = { // If the binding is a prop, we need to deep read it because it could be fine-grained $state // from a runes-component, where mutations don't trigger an update on the prop as a whole. if (name === '$$props' || name === '$$restProps' || binding.kind === 'prop') { - serialized = b.call('$.deep_read', serialized); + serialized = b.call('$.deep_read_state', serialized); } sequence.push(serialized); diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 6cac0c7bad67..fe3db6c0ebf6 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -44,11 +44,11 @@ import { push, pop, current_component_context, - deep_read, get, set, is_signals_recorded, - inspect_fn + inspect_fn, + deep_read_state } from './runtime.js'; import { render_effect, @@ -1950,7 +1950,7 @@ export function action(dom, action, value_fn) { // This works in legacy mode because of mutable_source being updated as a whole, but when using $state // together with actions and mutation, it wouldn't notice the change without a deep read. if (needs_deep_read) { - deep_read(value); + deep_read_state(value); } } else { untrack(() => (payload = action(dom))); @@ -2858,10 +2858,12 @@ export function init() { if (!callbacks) return; // beforeUpdate - pre_effect(() => { - observe_all(context); - callbacks.b.forEach(run); - }); + if (callbacks.b.length) { + pre_effect(() => { + observe_all(context); + callbacks.b.forEach(run); + }); + } // onMount (must run before afterUpdate) user_effect(() => { @@ -2876,10 +2878,12 @@ export function init() { }); // afterUpdate - user_effect(() => { - observe_all(context); - callbacks.a.forEach(run); - }); + if (callbacks.a.length) { + user_effect(() => { + observe_all(context); + callbacks.a.forEach(run); + }); + } } /** @@ -2892,7 +2896,7 @@ function observe_all(context) { for (const signal of context.d) get(signal); } - deep_read(context.s); + deep_read_state(context.s); } /** diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 79eed38d25d1..1aa116076b8a 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1244,6 +1244,29 @@ export function pop(component) { return component || /** @type {T} */ ({}); } +/** + * Possibly traverse an object and read all its properties so that they're all reactive in case this is `$state`. + * Does only check first level of an object for performance reasons (heuristic should be good for 99% of all cases). + * @param {any} value + * @returns {void} + */ +export function deep_read_state(value) { + if (typeof value !== 'object' || !value || value instanceof EventTarget) { + return; + } + + if (STATE_SYMBOL in value) { + deep_read(value); + } else if (!Array.isArray(value)) { + for (let key in value) { + const prop = value[key]; + if (typeof prop === 'object' && prop && STATE_SYMBOL in prop) { + deep_read(prop); + } + } + } +} + /** * Deeply traverse an object and read all its properties * so that they're all reactive in case this is `$state` diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 741391ebc645..1c232cf3f26c 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -18,7 +18,8 @@ export { inspect, unwrap, freeze, - deep_read + deep_read, + deep_read_state } from './client/runtime.js'; export * from './client/dev/ownership.js'; export { await_block as await } from './client/dom/blocks/await.js';