Skip to content

Commit febced5

Browse files
wmertenswmertens
committed
WIP perf(optimizer): make empty functions noopQrls WIP
TODO retain scope captures. The current approach won't work because it strips the code early. TODO handle null qrls in the manifest
1 parent 7c21496 commit febced5

6 files changed

+154
-32
lines changed

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

+23-7
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,32 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
288288
&config.core_module,
289289
);
290290

291+
let mut treeshaker = Treeshaker::new();
292+
291293
// replace const values
292294
if config.mode != EmitMode::Test {
293295
let mut const_replacer =
294296
ConstReplacerVisitor::new(config.is_server, is_dev, &collect);
295297
program.visit_mut_with(&mut const_replacer);
298+
299+
if config.minify != MinifyMode::None {
300+
if !config.is_server {
301+
// remove all side effects from client, step 1
302+
program.visit_mut_with(&mut treeshaker.marker);
303+
}
304+
305+
// simplify & strip unused code before segmenting
306+
program.mutate(&mut simplify::simplifier(
307+
unresolved_mark,
308+
simplify::Config {
309+
dce: simplify::dce::Config {
310+
preserve_imports_with_side_effects: false,
311+
..Default::default()
312+
},
313+
..Default::default()
314+
},
315+
));
316+
}
296317
}
297318

298319
// split into segments
@@ -330,14 +351,8 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
330351
// );
331352
program = program.fold_with(&mut qwik_transform);
332353

333-
let mut treeshaker = Treeshaker::new();
334354
if config.minify != MinifyMode::None {
335-
// remove all side effects from client, step 1
336-
if !config.is_server {
337-
program.visit_mut_with(&mut treeshaker.marker);
338-
}
339-
340-
// simplify & strip unused code
355+
// simplify & strip unused code, again
341356
program.mutate(&mut simplify::simplifier(
342357
unresolved_mark,
343358
simplify::Config {
@@ -349,6 +364,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
349364
},
350365
));
351366
}
367+
352368
if matches!(
353369
config.entry_strategy,
354370
EntryStrategy::Inline | EntryStrategy::Hoist

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

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
source: packages/qwik/src/optimizer/core/src/test.rs
3+
assertion_line: 4275
4+
expression: output
5+
snapshot_kind: text
6+
---
7+
==INPUT==
8+
9+
10+
import { isServer } from '@builder.io/qwik/build';
11+
import { component$ } from '@builder.io/qwik';
12+
export const Cmp0 = component$(() => {
13+
return undefined;
14+
});
15+
export const Cmp1 = component$(() => {
16+
if (!isServer) {
17+
return <div>hello</div>;
18+
}
19+
});
20+
export const Cmp2 = component$(function(_unused) {
21+
if (isServer) {
22+
return;
23+
}
24+
return <div>hello</div>;
25+
});
26+
export const Cmp3 = component$(function() { });
27+
28+
============================= test.tsx ==
29+
30+
import { componentQrl } from "@qwik.dev/core";
31+
import { _noopQrl } from "@qwik.dev/core";
32+
export const Cmp0 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_SEk00MbyDzs"));
33+
export const Cmp1 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_U2t03012214"));
34+
export const Cmp2 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_FnOz26iMYaM"));
35+
export const Cmp3 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_wyGQRm5AyHM"));
36+
37+
38+
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\"}")
39+
== DIAGNOSTICS ==
40+
41+
[]

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

+3-3
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 (
@@ -66,7 +66,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
6666
state
6767
]));
6868
useTaskQrl(/*#__PURE__*/ inlinedQrl(()=>{
69-
// Code
69+
runSomething();
7070
}, "Parent_component_useTask_ngmvcygWux8"));
7171
return /*#__PURE__*/ _jsxSorted("div", null, {
7272
shouldRemove$: /*#__PURE__*/ _noopQrl("Parent_component_div_shouldRemove_EBj69wTX1do", [
@@ -90,7 +90,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
9090
}, "Parent_component_t6Wy3C0Q0XM"));
9191

9292

93-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/components/component.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;AACA,SAAsC,QAAQ,QAAkB,iBAAiB;AAQjF,OAAO,MAAM,uBAAS,sCAAW;IAChC,MAAM,QAAQ,SAAS;QACtB,MAAM;IACP;IAEA,qBAAqB;IACrB;;;IAKA,oCAAS;IACR,OAAO;IACR;IAEA,qBACC,WAAC;QACA,aAAa;;;QACb,QAAQ;;;;sBAER,WAAC;YACA,QAAQ,2BAAE,IAAM,QAAQ,GAAG,CAAC;YAC5B,OAAO,2BAAE;;uBAAM,MAAM,IAAI;;;;;kBAEzB;;AAGJ,oCAAG\"}")
93+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/components/component.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;AACA,SAAsC,QAAQ,QAAkB,iBAAiB;AAQjF,OAAO,MAAM,uBAAS,sCAAW;IAChC,MAAM,QAAQ,SAAS;QACtB,MAAM;IACP;IAEA,qBAAqB;IACrB;;;IAKA,oCAAS;QACR;IACD;IAEA,qBACC,WAAC;QACA,aAAa;;;QACb,QAAQ;;;;sBAER,WAAC;YACA,QAAQ,2BAAE,IAAM,QAAQ,GAAG,CAAC;YAC5B,OAAO,2BAAE;;uBAAM,MAAM,IAAI;;;;;kBAEzB;;AAGJ,oCAAG\"}")
9494
== DIAGNOSTICS ==
9595

9696
[]

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

+17-17
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 (
@@ -93,11 +93,11 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
9393
============================= test.tsx_Parent_component_serverStuff_a_2ca3HLDC7yc.js (ENTRY POINT)==
9494

9595
export const s_2ca3HLDC7yc = ()=>{
96-
// from $(), should not be removed
96+
dontRemoveThisDollar();
9797
};
9898

9999

100-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAqBc;AACX,kCAAkC;AACnC\"}")
100+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAqBc;IACX;AACD\"}")
101101
/*
102102
{
103103
"origin": "test.tsx",
@@ -114,18 +114,18 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
114114
"captures": false,
115115
"loc": [
116116
538,
117-
587
117+
576
118118
]
119119
}
120120
*/
121121
============================= test.tsx_Parent_component_serverStuff_b_client_v9qawr2Inkk.js (ENTRY POINT)==
122122

123123
export const s_v9qawr2Inkk = ()=>{
124-
// from clien$(), should not be removed
124+
dontRemoveThisClient();
125125
};
126126

127127

128-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAwBoB;AACjB,uCAAuC;AACxC\"}")
128+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAwBoB;IACjB;AACD\"}")
129129
/*
130130
{
131131
"origin": "test.tsx",
@@ -141,8 +141,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
141141
"ctxName": "client$",
142142
"captures": false,
143143
"loc": [
144-
610,
145-
664
144+
599,
145+
637
146146
]
147147
}
148148
*/
@@ -190,7 +190,7 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
190190
"captures": false,
191191
"loc": [
192192
279,
193-
835
193+
816
194194
]
195195
}
196196
*/
@@ -215,19 +215,19 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
215215
"ctxName": "onClick$",
216216
"captures": false,
217217
"loc": [
218-
775,
219-
802
218+
756,
219+
783
220220
]
221221
}
222222
*/
223223
============================= test.tsx_Parent_component_useTask_1_P8oRQhHsurk.js (ENTRY POINT)==
224224

225225
export const s_P8oRQhHsurk = ()=>{
226-
// Code
226+
runSomething();
227227
};
228228

229229

230-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAgCU;AACR,OAAO;AACR\"}")
230+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"6BAgCU;IACR;AACD\"}")
231231
/*
232232
{
233233
"origin": "test.tsx",
@@ -243,8 +243,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
243243
"ctxName": "useTask$",
244244
"captures": false,
245245
"loc": [
246-
724,
247-
744
246+
697,
247+
725
248248
]
249249
}
250250
*/

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

+34-4
Original file line numberDiff line numberDiff line change
@@ -1842,18 +1842,18 @@ export const Parent = component$(() => {
18421842
serverStuff$(async () => {
18431843
// should be removed too
18441844
const a = $(() => {
1845-
// from $(), should not be removed
1845+
dontRemoveThisDollar();
18461846
});
18471847
const b = client$(() => {
1848-
// from clien$(), should not be removed
1848+
dontRemoveThisClient();
18491849
});
18501850
return [a,b];
18511851
})
18521852
18531853
serverLoader$(handler);
18541854
18551855
useTask$(() => {
1856-
// Code
1856+
runSomething();
18571857
});
18581858
18591859
return (
@@ -1948,7 +1948,7 @@ export const Parent = component$(() => {
19481948
});
19491949
19501950
useTask$(() => {
1951-
// Code
1951+
runSomething();
19521952
});
19531953
19541954
return (
@@ -4269,6 +4269,36 @@ export const Cmp = component$(() => {
42694269
..TestInput::default()
42704270
});
42714271
}
4272+
4273+
#[test]
4274+
fn empty_fn_to_noop() {
4275+
test_input!(TestInput {
4276+
code: r#"
4277+
import { isServer } from '@builder.io/qwik/build';
4278+
import { component$ } from '@builder.io/qwik';
4279+
export const Cmp0 = component$(() => {
4280+
return undefined;
4281+
});
4282+
export const Cmp1 = component$(() => {
4283+
if (!isServer) {
4284+
return <div>hello</div>;
4285+
}
4286+
});
4287+
export const Cmp2 = component$(function(_unused) {
4288+
if (isServer) {
4289+
return;
4290+
}
4291+
return <div>hello</div>;
4292+
});
4293+
export const Cmp3 = component$(function() { });
4294+
"#
4295+
.to_string(),
4296+
mode: EmitMode::Prod,
4297+
is_server: Some(true),
4298+
..TestInput::default()
4299+
});
4300+
}
4301+
42724302
// TODO(misko): Make this test work by implementing strict serialization.
42734303
// #[test]
42744304
// fn example_of_synchronous_qrl_that_cant_be_serialized() {

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

+36-1
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,7 @@ impl<'a> QwikTransform<'a> {
646646
self.segment_stack.push(symbol_name.clone());
647647
let span = first_arg.span();
648648
let folded = first_arg.fold_with(self);
649+
let is_empty = is_empty_function(&folded);
649650
self.segment_stack.pop();
650651

651652
// Collect local idents
@@ -679,7 +680,7 @@ impl<'a> QwikTransform<'a> {
679680
need_transform: true,
680681
hash,
681682
};
682-
let should_emit = self.should_emit_segment(&segment_data);
683+
let should_emit = !is_empty && self.should_emit_segment(&segment_data);
683684
if should_emit {
684685
for id in &segment_data.local_idents {
685686
if !self.options.global_collect.exports.contains_key(id) {
@@ -2468,3 +2469,37 @@ fn process_node_props(pat: &ast::Pat) -> Vec<IdPlusType> {
24682469

24692470
processed_scope_data
24702471
}
2472+
2473+
/** detect if an expression is a function or arrow function with an empty function body */
2474+
fn is_empty_function(expr: &ast::Expr) -> bool {
2475+
match expr {
2476+
ast::Expr::Fn(ast::FnExpr {
2477+
function: box ast::Function {
2478+
body: Some(ast::BlockStmt { stmts, .. }),
2479+
..
2480+
},
2481+
..
2482+
}) => are_statements_empty(stmts),
2483+
ast::Expr::Arrow(ast::ArrowExpr {
2484+
body: box ast::BlockStmtOrExpr::BlockStmt(block),
2485+
..
2486+
}) => are_statements_empty(&block.stmts),
2487+
_ => false,
2488+
}
2489+
}
2490+
2491+
/** detect if statements are empty or only `return` or `return undefined` */
2492+
fn are_statements_empty(stmts: &[ast::Stmt]) -> bool {
2493+
stmts.is_empty()
2494+
|| (stmts.len() == 1
2495+
&& match &stmts[0] {
2496+
ast::Stmt::Return(ast::ReturnStmt { arg, .. }) => {
2497+
arg.is_none()
2498+
|| match &arg {
2499+
Some(box ast::Expr::Ident(ident)) => ident.sym == js_word!("undefined"),
2500+
_ => false,
2501+
}
2502+
}
2503+
_ => false,
2504+
})
2505+
}

0 commit comments

Comments
 (0)