Skip to content

Commit f6c783a

Browse files
wmertenswmertens
committed
perf(optimizer): make empty functions noopQrls WIP
TODO retain scope captures. The current approach won't work because it strips the code early.
1 parent 52b8ddb commit f6c783a

File tree

6 files changed

+152
-32
lines changed

6 files changed

+152
-32
lines changed

‎packages/qwik/src/optimizer/core/src/parse.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,32 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
309309
&config.core_module,
310310
);
311311

312+
let mut treeshaker = Treeshaker::new();
313+
312314
// replace const values
313315
if config.mode != EmitMode::Test {
314316
let mut const_replacer =
315317
ConstReplacerVisitor::new(config.is_server, is_dev, &collect);
316318
program.visit_mut_with(&mut const_replacer);
319+
320+
if config.minify != MinifyMode::None {
321+
if !config.is_server {
322+
// remove all side effects from client, step 1
323+
program.visit_mut_with(&mut treeshaker.marker);
324+
}
325+
326+
// simplify & strip unused code before segmenting
327+
program.mutate(&mut simplify::simplifier(
328+
unresolved_mark,
329+
simplify::Config {
330+
dce: simplify::dce::Config {
331+
preserve_imports_with_side_effects: false,
332+
..Default::default()
333+
},
334+
..Default::default()
335+
},
336+
));
337+
}
317338
}
318339

319340
// split into segments
@@ -337,14 +358,8 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
337358
});
338359
program = program.fold_with(&mut qwik_transform);
339360

340-
let mut treeshaker = Treeshaker::new();
341361
if config.minify != MinifyMode::None {
342-
// remove all side effects from client, step 1
343-
if !config.is_server {
344-
program.visit_mut_with(&mut treeshaker.marker);
345-
}
346-
347-
// simplify & strip unused code
362+
// simplify & strip unused code, again
348363
program.mutate(&mut simplify::simplifier(
349364
unresolved_mark,
350365
simplify::Config {
@@ -356,6 +371,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
356371
},
357372
));
358373
}
374+
359375
if matches!(
360376
config.entry_strategy,
361377
EntryStrategy::Inline | EntryStrategy::Hoist

‎packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__empty_fn_to_noop.snap

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
source: packages/qwik/src/optimizer/core/src/test.rs
3+
assertion_line: 3565
4+
expression: output
5+
---
6+
==INPUT==
7+
8+
9+
import { isServer } from '@builder.io/qwik/build';
10+
import { component$ } from '@builder.io/qwik';
11+
export const Cmp0 = component$(() => {
12+
return undefined;
13+
});
14+
export const Cmp1 = component$(() => {
15+
if (!isServer) {
16+
return <div>hello</div>;
17+
}
18+
});
19+
export const Cmp2 = component$(function(_unused) {
20+
if (isServer) {
21+
return;
22+
}
23+
return <div>hello</div>;
24+
});
25+
export const Cmp3 = component$(function() { });
26+
27+
============================= test.tsx ==
28+
29+
import { componentQrl } from "@builder.io/qwik";
30+
import { _noopQrl } from "@builder.io/qwik";
31+
export const Cmp0 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_SEk00MbyDzs"));
32+
export const Cmp1 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_U2t03012214"));
33+
export const Cmp2 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_FnOz26iMYaM"));
34+
export const Cmp3 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_wyGQRm5AyHM"));
35+
36+
37+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;AAGE,OAAO,MAAM,qBAAO,sDAEhB;AACJ,OAAO,MAAM,qBAAO,sDAIjB;AACH,OAAO,MAAM,qBAAO,sDAKjB;AACH,OAAO,MAAM,qBAAO,sDAA2B\"}")
38+
== DIAGNOSTICS ==
39+
40+
[]

‎packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_strip_client_code.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export const Parent = component$(() => {
2727
});
2828

2929
useTask$(() => {
30-
// Code
30+
runSomething();
3131
});
3232

3333
return (
@@ -68,7 +68,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
6868
state
6969
]));
7070
useTaskQrl(/*#__PURE__*/ inlinedQrl(()=>{
71-
// Code
71+
runSomething();
7272
}, "Parent_component_useTask_ngmvcygWux8"));
7373
return /*#__PURE__*/ _jsxQ("div", null, {
7474
shouldRemove$: /*#__PURE__*/ _noopQrl("Parent_component_div_shouldRemove_EBj69wTX1do", [
@@ -98,7 +98,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
9898
}, "Parent_component_t6Wy3C0Q0XM"));
9999

100100

101-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/components/component.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;;;AACA,SAAsC,QAAQ,QAAkB,mBAAmB;AAQnF,OAAO,MAAM,uBAAS,sCAAW;IAC7B,MAAM,QAAQ,SAAS;QACnB,MAAM;IACV;IAEA,qBAAqB;IACrB;;;IAKA,oCAAS;IACL,OAAO;IACX;IAEA,qBACI,MAAC;QACG,aAAa;;;QACb,QAAQ;;;;sBAER,MAAC;YACG,QAAQ,2BAAE,IAAM,QAAQ,GAAG,CAAC;YAC5B,OAAO,2BAAE;;uBAAM,MAAM,IAAI;;;;;gBADzB,QAAQ;gBACR,OAAO;;;wBAEV,GAAM,IAAI;;;;AAGvB,oCAAG\"}")
101+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/components/component.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;;;AACA,SAAsC,QAAQ,QAAkB,mBAAmB;AAQnF,OAAO,MAAM,uBAAS,sCAAW;IAC7B,MAAM,QAAQ,SAAS;QACnB,MAAM;IACV;IAEA,qBAAqB;IACrB;;;IAKA,oCAAS;QACL;IACJ;IAEA,qBACI,MAAC;QACG,aAAa;;;QACb,QAAQ;;;;sBAER,MAAC;YACG,QAAQ,2BAAE,IAAM,QAAQ,GAAG,CAAC;YAC5B,OAAO,2BAAE;;uBAAM,MAAM,IAAI;;;;;gBADzB,QAAQ;gBACR,OAAO;;;wBAEV,GAAM,IAAI;;;;AAGvB,oCAAG\"}")
102102
== DIAGNOSTICS ==
103103

104104
[]

‎packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_strip_server_code.snap

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,18 @@ export const Parent = component$(() => {
2828
serverStuff$(async () => {
2929
// should be removed too
3030
const a = $(() => {
31-
// from $(), should not be removed
31+
dontRemoveThisDollar();
3232
});
3333
const b = client$(() => {
34-
// from clien$(), should not be removed
34+
dontRemoveThisClient();
3535
});
3636
return [a,b];
3737
})
3838

3939
serverLoader$(handler);
4040

4141
useTask$(() => {
42-
// Code
42+
runSomething();
4343
});
4444

4545
return (
@@ -96,12 +96,12 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
9696
============================= test.tsx_Parent_component_serverStuff_a_2ca3HLDC7yc.js (ENTRY POINT)==
9797

9898
export const Parent_component_serverStuff_a_2ca3HLDC7yc = ()=>{
99-
// from $(), should not be removed
99+
dontRemoveThisDollar();
100100
};
101101
export { _hW } from "@builder.io/qwik";
102102

103103

104-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"0DAqBoB;AACR,kCAAkC;AACtC\"}")
104+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"0DAqBoB;IACR;AACJ\"}")
105105
/*
106106
{
107107
"origin": "test.tsx",
@@ -118,18 +118,18 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
118118
"captures": false,
119119
"loc": [
120120
602,
121-
666
121+
655
122122
]
123123
}
124124
*/
125125
============================= test.tsx_Parent_component_serverStuff_b_client_v9qawr2Inkk.js (ENTRY POINT)==
126126

127127
export const Parent_component_serverStuff_b_client_v9qawr2Inkk = ()=>{
128-
// from clien$(), should not be removed
128+
dontRemoveThisClient();
129129
};
130130

131131

132-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"iEAwB0B;AACd,uCAAuC;AAC3C\"}")
132+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"iEAwB0B;IACd;AACJ\"}")
133133
/*
134134
{
135135
"origin": "test.tsx",
@@ -145,8 +145,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
145145
"ctxName": "client$",
146146
"captures": false,
147147
"loc": [
148-
695,
149-
764
148+
684,
149+
737
150150
]
151151
}
152152
*/
@@ -196,7 +196,7 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
196196
"captures": false,
197197
"loc": [
198198
289,
199-
986
199+
967
200200
]
201201
}
202202
*/
@@ -221,20 +221,20 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
221221
"ctxName": "onClick$",
222222
"captures": false,
223223
"loc": [
224-
908,
225-
935
224+
889,
225+
916
226226
]
227227
}
228228
*/
229229
============================= test.tsx_Parent_component_useTask_1_P8oRQhHsurk.js (ENTRY POINT)==
230230

231231
export const Parent_component_useTask_1_P8oRQhHsurk = ()=>{
232-
// Code
232+
runSomething();
233233
};
234234
export { _hW } from "@builder.io/qwik";
235235

236236

237-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"sDAgCa;AACL,OAAO;AACX\"}")
237+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"sDAgCa;IACL;AACJ\"}")
238238
/*
239239
{
240240
"origin": "test.tsx",
@@ -250,8 +250,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
250250
"ctxName": "useTask$",
251251
"captures": false,
252252
"loc": [
253-
839,
254-
868
253+
812,
254+
849
255255
]
256256
}
257257
*/

‎packages/qwik/src/optimizer/core/src/test.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,18 +1732,18 @@ export const Parent = component$(() => {
17321732
serverStuff$(async () => {
17331733
// should be removed too
17341734
const a = $(() => {
1735-
// from $(), should not be removed
1735+
dontRemoveThisDollar();
17361736
});
17371737
const b = client$(() => {
1738-
// from clien$(), should not be removed
1738+
dontRemoveThisClient();
17391739
});
17401740
return [a,b];
17411741
})
17421742
17431743
serverLoader$(handler);
17441744
17451745
useTask$(() => {
1746-
// Code
1746+
runSomething();
17471747
});
17481748
17491749
return (
@@ -1837,7 +1837,7 @@ export const Parent = component$(() => {
18371837
});
18381838
18391839
useTask$(() => {
1840-
// Code
1840+
runSomething();
18411841
});
18421842
18431843
return (
@@ -3600,6 +3600,35 @@ fn impure_template_fns() {
36003600
});
36013601
}
36023602

3603+
#[test]
3604+
fn empty_fn_to_noop() {
3605+
test_input!(TestInput {
3606+
code: r#"
3607+
import { isServer } from '@builder.io/qwik/build';
3608+
import { component$ } from '@builder.io/qwik';
3609+
export const Cmp0 = component$(() => {
3610+
return undefined;
3611+
});
3612+
export const Cmp1 = component$(() => {
3613+
if (!isServer) {
3614+
return <div>hello</div>;
3615+
}
3616+
});
3617+
export const Cmp2 = component$(function(_unused) {
3618+
if (isServer) {
3619+
return;
3620+
}
3621+
return <div>hello</div>;
3622+
});
3623+
export const Cmp3 = component$(function() { });
3624+
"#
3625+
.to_string(),
3626+
mode: EmitMode::Prod,
3627+
is_server: Some(true),
3628+
..TestInput::default()
3629+
});
3630+
}
3631+
36033632
// TODO(misko): Make this test work by implementing strict serialization.
36043633
// #[test]
36053634
// fn example_of_synchronous_qrl_that_cant_be_serialized() {

‎packages/qwik/src/optimizer/core/src/transform.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,7 @@ impl<'a> QwikTransform<'a> {
628628
self.segment_stack.push(symbol_name.clone());
629629
let span = first_arg.span();
630630
let folded = first_arg.fold_with(self);
631+
let is_empty = is_empty_function(&folded);
631632
self.segment_stack.pop();
632633

633634
// Collect local idents
@@ -661,7 +662,7 @@ impl<'a> QwikTransform<'a> {
661662
need_transform: true,
662663
hash,
663664
};
664-
let should_emit = self.should_emit_segment(&segment_data);
665+
let should_emit = !is_empty && self.should_emit_segment(&segment_data);
665666
if should_emit {
666667
for id in &segment_data.local_idents {
667668
if !self.options.global_collect.exports.contains_key(id) {
@@ -2446,3 +2447,37 @@ fn is_text_only(node: &str) -> bool {
24462447
"text" | "textarea" | "title" | "option" | "script" | "style" | "noscript"
24472448
)
24482449
}
2450+
2451+
/** detect if an expression is a function or arrow function with an empty function body */
2452+
fn is_empty_function(expr: &ast::Expr) -> bool {
2453+
match expr {
2454+
ast::Expr::Fn(ast::FnExpr {
2455+
function: box ast::Function {
2456+
body: Some(ast::BlockStmt { stmts, .. }),
2457+
..
2458+
},
2459+
..
2460+
}) => are_statements_empty(stmts),
2461+
ast::Expr::Arrow(ast::ArrowExpr {
2462+
body: box ast::BlockStmtOrExpr::BlockStmt(block),
2463+
..
2464+
}) => are_statements_empty(&block.stmts),
2465+
_ => false,
2466+
}
2467+
}
2468+
2469+
/** detect if statements are empty or only `return` or `return undefined` */
2470+
fn are_statements_empty(stmts: &[ast::Stmt]) -> bool {
2471+
stmts.is_empty()
2472+
|| (stmts.len() == 1
2473+
&& match &stmts[0] {
2474+
ast::Stmt::Return(ast::ReturnStmt { arg, .. }) => {
2475+
arg.is_none()
2476+
|| match &arg {
2477+
Some(box ast::Expr::Ident(ident)) => ident.sym == js_word!("undefined"),
2478+
_ => false,
2479+
}
2480+
}
2481+
_ => false,
2482+
})
2483+
}

0 commit comments

Comments
 (0)