From 5d616d12caf13fb91f7c59c401b3e888dfa356f9 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 14 Sep 2024 22:04:42 -0700 Subject: [PATCH 1/4] perf: optimize non-reactive if statements --- .changeset/khaki-jobs-smoke.md | 5 ++++ .../3-transform/client/visitors/IfBlock.js | 19 +++++++++++-- .../_expected/client/index.svelte.js | 28 +++++++++++++++++++ .../_expected/server/index.svelte.js | 24 ++++++++++++++++ .../index.svelte | 14 ++++++++++ 5 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 .changeset/khaki-jobs-smoke.md create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte diff --git a/.changeset/khaki-jobs-smoke.md b/.changeset/khaki-jobs-smoke.md new file mode 100644 index 000000000000..d72bbab7a0f7 --- /dev/null +++ b/.changeset/khaki-jobs-smoke.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +perf: optimize non-reactive if statements diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index 6f8894c2c83b..174273f1df73 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -8,12 +8,25 @@ import * as b from '../../../../utils/builders.js'; * @param {ComponentContext} context */ export function IfBlock(node, context) { - context.state.template.push_quasi(''); + const { state } = context; const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + // compile to if rather than $.if for non-reactive if statements + // NOTE: this only handles expressions that are simple identifiers and doesn't handle else/elseif yet + if (state.analysis.runes && node.test.type === 'Identifier' && !(node.alternate || node.elseif)) { + const binding = state.scope.owner(node.test.name)?.declarations.get(node.test.name); + if (binding?.kind === 'normal') { + consequent.body.unshift(b.var(b.id('$$anchor'), state.node)); + state.init.push(b.if(node.test, consequent)); + return; + } + } + + state.template.push_quasi(''); + const args = [ - context.state.node, + state.node, b.thunk(/** @type {Expression} */ (context.visit(node.test))), b.arrow([b.id('$$anchor')], consequent) ]; @@ -51,5 +64,5 @@ export function IfBlock(node, context) { args.push(b.literal(true)); } - context.state.init.push(b.stmt(b.call('$.if', ...args))); + state.init.push(b.stmt(b.call('$.if', ...args))); } diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js new file mode 100644 index 000000000000..3524316b0b98 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js @@ -0,0 +1,28 @@ +import "svelte/internal/disclose-version"; +import * as $ from "svelte/internal/client"; + +var root = $.template(` `, 1); + +export default function Non_reactive_control_structures($$anchor) { + const a = true; + let b = true; + var fragment = root(); + var node = $.first_child(fragment); + + if (a) { + var $$anchor = node; + var text = $.text("hello"); + + $.append($$anchor, text); + } + + var node_1 = $.sibling(node, 2); + + $.if(node_1, () => b, ($$anchor) => { + var text_1 = $.text("world"); + + $.append($$anchor, text_1); + }); + + $.append($$anchor, fragment); +} diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js new file mode 100644 index 000000000000..ca209bbf6113 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js @@ -0,0 +1,24 @@ +import * as $ from "svelte/internal/server"; + +export default function Non_reactive_control_structures($$payload) { + const a = true; + let b = true; + + if (a) { + $$payload.out += ""; + $$payload.out += `hello`; + } else { + $$payload.out += ""; + } + + $$payload.out += ` `; + + if (b) { + $$payload.out += ""; + $$payload.out += `world`; + } else { + $$payload.out += ""; + } + + $$payload.out += ``; +} diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte new file mode 100644 index 000000000000..6b92af33f7d8 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte @@ -0,0 +1,14 @@ + + + + +{#if a} + hello +{/if} + +{#if b} + world +{/if} From efd9dbb7563c72dbb977193cdec2900e3d64384a Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sat, 14 Sep 2024 22:04:42 -0700 Subject: [PATCH 2/4] perf: optimize non-reactive if statements --- .changeset/khaki-jobs-smoke.md | 5 +++ .../3-transform/client/visitors/IfBlock.js | 25 +++++++++-- .../_expected/client/index.svelte.js | 42 +++++++++++++++++++ .../_expected/server/index.svelte.js | 34 +++++++++++++++ .../index.svelte | 20 +++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 .changeset/khaki-jobs-smoke.md create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js create mode 100644 packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte diff --git a/.changeset/khaki-jobs-smoke.md b/.changeset/khaki-jobs-smoke.md new file mode 100644 index 000000000000..d72bbab7a0f7 --- /dev/null +++ b/.changeset/khaki-jobs-smoke.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +perf: optimize non-reactive if statements diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index 6f8894c2c83b..65ad27af0414 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -8,12 +8,31 @@ import * as b from '../../../../utils/builders.js'; * @param {ComponentContext} context */ export function IfBlock(node, context) { - context.state.template.push_quasi(''); + const { state } = context; const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + // compile to if rather than $.if for non-reactive if statements + // NOTE: this only handles expressions that are simple identifiers and doesn't handle elseif yet + if (state.analysis.runes && node.test.type === 'Identifier' && !node.elseif) { + const binding = state.scope.owner(node.test.name)?.declarations.get(node.test.name); + if (binding?.kind === 'normal') { + consequent.body.unshift(b.var(b.id('$$anchor'), state.node)); + const alternate = node.alternate + ? /** @type {BlockStatement} */ (context.visit(node.alternate)) + : undefined; + if (alternate) { + alternate.body.unshift(b.var(b.id('$$anchor'), state.node)); + } + state.init.push(b.if(node.test, consequent, alternate)); + return; + } + } + + state.template.push_quasi(''); + const args = [ - context.state.node, + state.node, b.thunk(/** @type {Expression} */ (context.visit(node.test))), b.arrow([b.id('$$anchor')], consequent) ]; @@ -51,5 +70,5 @@ export function IfBlock(node, context) { args.push(b.literal(true)); } - context.state.init.push(b.stmt(b.call('$.if', ...args))); + state.init.push(b.stmt(b.call('$.if', ...args))); } diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js new file mode 100644 index 000000000000..7c4cf77203c4 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js @@ -0,0 +1,42 @@ +import "svelte/internal/disclose-version"; +import * as $ from "svelte/internal/client"; + +var root = $.template(` `, 1); + +export default function Non_reactive_control_structures($$anchor) { + const a = true; + let b = true; + var fragment = root(); + var node = $.first_child(fragment); + + if (a) { + var $$anchor = node; + var text = $.text("hello"); + + $.append($$anchor, text); + } + + var node_1 = $.sibling(node, 2); + + $.if(node_1, () => b, ($$anchor) => { + var text_1 = $.text("world"); + + $.append($$anchor, text_1); + }); + + var node_2 = $.sibling(node_1, 2); + + if (a) { + var $$anchor = node_2; + var text_2 = $.text("hi"); + + $.append($$anchor, text_2); + } else { + var $$anchor = node_2; + var text_3 = $.text("earth"); + + $.append($$anchor, text_3); + } + + $.append($$anchor, fragment); +} diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js new file mode 100644 index 000000000000..c15f13f29ffb --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js @@ -0,0 +1,34 @@ +import * as $ from "svelte/internal/server"; + +export default function Non_reactive_control_structures($$payload) { + const a = true; + let b = true; + + if (a) { + $$payload.out += ""; + $$payload.out += `hello`; + } else { + $$payload.out += ""; + } + + $$payload.out += ` `; + + if (b) { + $$payload.out += ""; + $$payload.out += `world`; + } else { + $$payload.out += ""; + } + + $$payload.out += ` `; + + if (a) { + $$payload.out += ""; + $$payload.out += `hi`; + } else { + $$payload.out += ""; + $$payload.out += `earth`; + } + + $$payload.out += ``; +} diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte new file mode 100644 index 000000000000..a55acc736e60 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/index.svelte @@ -0,0 +1,20 @@ + + + + +{#if a} + hello +{/if} + +{#if b} + world +{/if} + +{#if a} + hi +{:else} + earth +{/if} From 41f0a1212104011498d92f2cbbab3c410c3108e5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 15 Sep 2024 22:19:29 -0400 Subject: [PATCH 3/4] handle more cases --- .../src/compiler/phases/1-parse/state/tag.js | 10 ++++++++-- .../phases/2-analyze/visitors/IfBlock.js | 8 +++++++- .../3-transform/client/visitors/IfBlock.js | 20 +++++++++---------- .../svelte/src/compiler/types/template.d.ts | 4 ++++ .../_expected/server/index.svelte.js | 2 +- .../_expected/client/index.svelte.js | 8 +++++--- .../_expected/server/index.svelte.js | 2 +- 7 files changed, 36 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/state/tag.js b/packages/svelte/src/compiler/phases/1-parse/state/tag.js index de118960ecea..f8d9babecc04 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js @@ -61,7 +61,10 @@ function open(parser) { end: -1, test: read_expression(parser), consequent: create_fragment(), - alternate: null + alternate: null, + metadata: { + expression: create_expression_metadata() + } }); parser.allow_whitespace(); @@ -363,7 +366,10 @@ function next(parser) { elseif: true, test: expression, consequent: create_fragment(), - alternate: null + alternate: null, + metadata: { + expression: create_expression_metadata() + } }); parser.stack.push(child); diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/IfBlock.js index a65771bcfca9..dcdae3587f63 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/IfBlock.js @@ -17,5 +17,11 @@ export function IfBlock(node, context) { mark_subtree_dynamic(context.path); - context.next(); + context.visit(node.test, { + ...context.state, + expression: node.metadata.expression + }); + + context.visit(node.consequent); + if (node.alternate) context.visit(node.alternate); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index 174273f1df73..1a640e8190bc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -12,18 +12,18 @@ export function IfBlock(node, context) { const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + state.template.push_quasi(''); + // compile to if rather than $.if for non-reactive if statements - // NOTE: this only handles expressions that are simple identifiers and doesn't handle else/elseif yet - if (state.analysis.runes && node.test.type === 'Identifier' && !(node.alternate || node.elseif)) { - const binding = state.scope.owner(node.test.name)?.declarations.get(node.test.name); - if (binding?.kind === 'normal') { - consequent.body.unshift(b.var(b.id('$$anchor'), state.node)); - state.init.push(b.if(node.test, consequent)); - return; - } - } + // NOTE: doesn't handle else/elseif yet or mismatches. also, this probably breaks transition locality + if (!node.metadata.expression.has_state && !(node.alternate || node.elseif)) { + state.init.push( + b.stmt(b.call('$.next')), + b.if(node.test, b.block([b.let(b.id('$$anchor'), state.node), ...consequent.body])) + ); - state.template.push_quasi(''); + return; + } const args = [ state.node, diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 9da0320432d4..c4753f791267 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -410,6 +410,10 @@ export namespace AST { test: Expression; consequent: Fragment; alternate: Fragment | null; + /** @internal */ + metadata: { + expression: ExpressionMetadata; + }; } /** An `{#await ...}` block */ diff --git a/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js index 6a9027e41fbb..9dad39d81e9e 100644 --- a/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js @@ -7,4 +7,4 @@ const __DECLARED_ASSET_3__ = "__VITE_ASSET__2AM7_y_g__"; export default function Inline_module_vars($$payload) { $$payload.out += ` `; -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js index 3524316b0b98..b87051108ce3 100644 --- a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/client/index.svelte.js @@ -1,7 +1,7 @@ import "svelte/internal/disclose-version"; import * as $ from "svelte/internal/client"; -var root = $.template(` `, 1); +var root = $.template(` `, 1); export default function Non_reactive_control_structures($$anchor) { const a = true; @@ -9,8 +9,10 @@ export default function Non_reactive_control_structures($$anchor) { var fragment = root(); var node = $.first_child(fragment); + $.next(); + if (a) { - var $$anchor = node; + let $$anchor = node; var text = $.text("hello"); $.append($$anchor, text); @@ -25,4 +27,4 @@ export default function Non_reactive_control_structures($$anchor) { }); $.append($$anchor, fragment); -} +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js index ca209bbf6113..4aad705930aa 100644 --- a/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/non-reactive-control-structures/_expected/server/index.svelte.js @@ -21,4 +21,4 @@ export default function Non_reactive_control_structures($$payload) { } $$payload.out += ``; -} +} \ No newline at end of file From 63cc21c3b1f4670bff5a932a2f6ecc985ef0e03a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 15 Sep 2024 22:33:53 -0400 Subject: [PATCH 4/4] simplify --- .../3-transform/client/visitors/IfBlock.js | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index 43aa63dd6678..c8749cde71ed 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -8,24 +8,19 @@ import * as b from '../../../../utils/builders.js'; * @param {ComponentContext} context */ export function IfBlock(node, context) { - const { state } = context; - const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + const alternate = node.alternate && /** @type {BlockStatement} */ (context.visit(node.alternate)); - state.template.push_quasi(''); + context.state.template.push_quasi(''); // compile to if rather than $.if for non-reactive if statements // NOTE: doesn't handle else/elseif yet or mismatches. also, this probably breaks transition locality if (!node.metadata.expression.has_state && !node.elseif) { - state.init.push( + context.state.init.push( b.block([ - b.let(b.id('$$anchor'), state.node), + b.let(b.id('$$anchor'), context.state.node), b.stmt(b.call('$.next')), - b.if( - node.test, - consequent, - node.alternate ? /** @type {BlockStatement} */ (context.visit(node.alternate)) : undefined - ) + b.if(node.test, consequent, alternate ?? undefined) ]) ); @@ -33,17 +28,13 @@ export function IfBlock(node, context) { } const args = [ - state.node, + context.state.node, b.thunk(/** @type {Expression} */ (context.visit(node.test))), b.arrow([b.id('$$anchor')], consequent) ]; - if (node.alternate || node.elseif) { - args.push( - node.alternate - ? b.arrow([b.id('$$anchor')], /** @type {BlockStatement} */ (context.visit(node.alternate))) - : b.literal(null) - ); + if (alternate || node.elseif) { + args.push(alternate ? b.arrow([b.id('$$anchor')], alternate) : b.literal(null)); } if (node.elseif) { @@ -71,5 +62,5 @@ export function IfBlock(node, context) { args.push(b.literal(true)); } - state.init.push(b.stmt(b.call('$.if', ...args))); + context.state.init.push(b.stmt(b.call('$.if', ...args))); }