Skip to content

Commit

Permalink
perf(optimizer): make empty functions noopQrls
Browse files Browse the repository at this point in the history
  • Loading branch information
@wmertens
wmertens committed Sep 8, 2024
1 parent f6164fa commit 31dc690
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 32 deletions.
30 changes: 23 additions & 7 deletions packages/qwik/src/optimizer/core/src/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,32 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
&config.core_module,
);

let mut treeshaker = Treeshaker::new();

// replace const values
if config.mode != EmitMode::Test {
let mut const_replacer =
ConstReplacerVisitor::new(config.is_server, is_dev, &collect);
program.visit_mut_with(&mut const_replacer);

if config.minify != MinifyMode::None {
if !config.is_server {
// remove all side effects from client, step 1
program.visit_mut_with(&mut treeshaker.marker);
}

// simplify & strip unused code before segmenting
program = program.fold_with(&mut simplify::simplifier(
unresolved_mark,
simplify::Config {
dce: simplify::dce::Config {
preserve_imports_with_side_effects: false,
..Default::default()
},
..Default::default()
},
));
}
}

// split into segments
Expand All @@ -338,14 +359,8 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
});
program = program.fold_with(&mut qwik_transform);

let mut treeshaker = Treeshaker::new();
if config.minify != MinifyMode::None {
// remove all side effects from client, step 1
if !config.is_server {
program.visit_mut_with(&mut treeshaker.marker);
}

// simplify & strip unused code
// simplify & strip unused code, again
program = program.fold_with(&mut simplify::simplifier(
unresolved_mark,
simplify::Config {
Expand All @@ -357,6 +372,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
},
));
}

if matches!(
config.entry_strategy,
EntryStrategy::Inline | EntryStrategy::Hoist
Expand Down
40 changes: 40 additions & 0 deletions packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__empty_fn_to_noop.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
source: packages/qwik/src/optimizer/core/src/test.rs
assertion_line: 3565
expression: output
---
==INPUT==


import { isServer } from '@builder.io/qwik/build';
import { component$ } from '@builder.io/qwik';
export const Cmp0 = component$(() => {
return undefined;
});
export const Cmp1 = component$(() => {
if (!isServer) {
return <div>hello</div>;
}
});
export const Cmp2 = component$(function(_unused) {
if (isServer) {
return;
}
return <div>hello</div>;
});
export const Cmp3 = component$(function() { });

============================= test.tsx ==

import { componentQrl } from "@builder.io/qwik";
import { _noopQrl } from "@builder.io/qwik";
export const Cmp0 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_SEk00MbyDzs"));
export const Cmp1 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_U2t03012214"));
export const Cmp2 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_FnOz26iMYaM"));
export const Cmp3 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ _noopQrl("s_wyGQRm5AyHM"));


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\"}")
== DIAGNOSTICS ==

[]
6 changes: 3 additions & 3 deletions ...ges/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_strip_client_code.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const Parent = component$(() => {
});

useTask$(() => {
// Code
runSomething();
});

return (
Expand Down Expand Up @@ -67,7 +67,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
state
]));
useTaskQrl(/*#__PURE__*/ inlinedQrl(()=>{
// Code
runSomething();
}, "Parent_component_useTask_ngmvcygWux8"));
return /*#__PURE__*/ _jsxQ("div", null, {
shouldRemove$: /*#__PURE__*/ _noopQrl("Parent_component_div_shouldRemove_EBj69wTX1do", [
Expand Down Expand Up @@ -97,7 +97,7 @@ export const Parent = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(()=>{
}, "Parent_component_t6Wy3C0Q0XM"));


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\"}")
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\"}")
== DIAGNOSTICS ==

[]
34 changes: 17 additions & 17 deletions ...ges/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_strip_server_code.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ export const Parent = component$(() => {
serverStuff$(async () => {
// should be removed too
const a = $(() => {
// from $(), should not be removed
dontRemoveThisDollar();
});
const b = client$(() => {
// from clien$(), should not be removed
dontRemoveThisClient();
});
return [a,b];
})

serverLoader$(handler);

useTask$(() => {
// Code
runSomething();
});

return (
Expand Down Expand Up @@ -95,12 +95,12 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
============================= parent_component_serverstuff_a_2ca3hldc7yc.js (ENTRY POINT)==

export const Parent_component_serverStuff_a_2ca3HLDC7yc = ()=>{
// from $(), should not be removed
dontRemoveThisDollar();
};
export { _hW } from "@builder.io/qwik";


Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"0DAqBoB;AACR,kCAAkC;AACtC\"}")
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"0DAqBoB;IACR;AACJ\"}")
/*
{
"origin": "test.tsx",
Expand All @@ -117,18 +117,18 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
"captures": false,
"loc": [
602,
666
655
]
}
*/
============================= parent_component_serverstuff_b_client_v9qawr2inkk.js (ENTRY POINT)==

export const Parent_component_serverStuff_b_client_v9qawr2Inkk = ()=>{
// from clien$(), should not be removed
dontRemoveThisClient();
};


Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"iEAwB0B;AACd,uCAAuC;AAC3C\"}")
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"iEAwB0B;IACd;AACJ\"}")
/*
{
"origin": "test.tsx",
Expand All @@ -144,8 +144,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
"ctxName": "client$",
"captures": false,
"loc": [
695,
764
684,
737
]
}
*/
Expand Down Expand Up @@ -195,7 +195,7 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
"captures": false,
"loc": [
289,
986
967
]
}
*/
Expand All @@ -220,20 +220,20 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
"ctxName": "onClick$",
"captures": false,
"loc": [
908,
935
889,
916
]
}
*/
============================= parent_component_usetask_1_p8orqhhsurk.js (ENTRY POINT)==

export const Parent_component_useTask_1_P8oRQhHsurk = ()=>{
// Code
runSomething();
};
export { _hW } from "@builder.io/qwik";


Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"sDAgCa;AACL,OAAO;AACX\"}")
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"sDAgCa;IACL;AACJ\"}")
/*
{
"origin": "test.tsx",
Expand All @@ -249,8 +249,8 @@ Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"ma
"ctxName": "useTask$",
"captures": false,
"loc": [
839,
868
812,
849
]
}
*/
Expand Down
37 changes: 33 additions & 4 deletions packages/qwik/src/optimizer/core/src/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1731,18 +1731,18 @@ export const Parent = component$(() => {
serverStuff$(async () => {
// should be removed too
const a = $(() => {
// from $(), should not be removed
dontRemoveThisDollar();
});
const b = client$(() => {
// from clien$(), should not be removed
dontRemoveThisClient();
});
return [a,b];
})
serverLoader$(handler);
useTask$(() => {
// Code
runSomething();
});
return (
Expand Down Expand Up @@ -1836,7 +1836,7 @@ export const Parent = component$(() => {
});
useTask$(() => {
// Code
runSomething();
});
return (
Expand Down Expand Up @@ -3560,6 +3560,35 @@ export const Counter = component$(() => {
});
}

#[test]
fn empty_fn_to_noop() {
test_input!(TestInput {
code: r#"
import { isServer } from '@builder.io/qwik/build';
import { component$ } from '@builder.io/qwik';
export const Cmp0 = component$(() => {
return undefined;
});
export const Cmp1 = component$(() => {
if (!isServer) {
return <div>hello</div>;
}
});
export const Cmp2 = component$(function(_unused) {
if (isServer) {
return;
}
return <div>hello</div>;
});
export const Cmp3 = component$(function() { });
"#
.to_string(),
mode: EmitMode::Prod,
is_server: Some(true),
..TestInput::default()
});
}

// TODO(misko): Make this test work by implementing strict serialization.
// #[test]
// fn example_of_synchronous_qrl_that_cant_be_serialized() {
Expand Down
37 changes: 36 additions & 1 deletion packages/qwik/src/optimizer/core/src/transform.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ impl<'a> QwikTransform<'a> {
self.segment_stack.push(symbol_name.clone());
let span = first_arg.span();
let folded = first_arg.fold_with(self);
let is_empty = is_empty_function(&folded);
self.segment_stack.pop();

// Collect local idents
Expand Down Expand Up @@ -654,7 +655,7 @@ impl<'a> QwikTransform<'a> {
need_transform: true,
hash,
};
let should_emit = self.should_emit_segment(&segment_data);
let should_emit = !is_empty && self.should_emit_segment(&segment_data);
if should_emit {
for id in &segment_data.local_idents {
if !self.options.global_collect.exports.contains_key(id) {
Expand Down Expand Up @@ -2413,3 +2414,37 @@ fn is_text_only(node: &str) -> bool {
"text" | "textarea" | "title" | "option" | "script" | "style" | "noscript"
)
}

/** detect if an expression is a function or arrow function with an empty function body */
fn is_empty_function(expr: &ast::Expr) -> bool {
match expr {
ast::Expr::Fn(ast::FnExpr {
function: box ast::Function {
body: Some(ast::BlockStmt { stmts, .. }),
..
},
..
}) => are_statements_empty(&stmts),

Check failure on line 2427 in packages/qwik/src/optimizer/core/src/transform.rs

View workflow job for this annotation

GitHub Actions / Build optimizer x86_64-unknown-linux-gnu 

this expression creates a reference which is immediately dereferenced by the compiler
ast::Expr::Arrow(ast::ArrowExpr {
body: box ast::BlockStmtOrExpr::BlockStmt(block),
..
}) => are_statements_empty(&block.stmts),
_ => false,
}
}

/** detect if statements are empty or only `return` or `return undefined` */
fn are_statements_empty(stmts: &[ast::Stmt]) -> bool {
dbg!(stmts).is_empty()
|| (stmts.len() == 1
&& match &stmts[0] {
ast::Stmt::Return(ast::ReturnStmt { arg, .. }) => {
arg.is_none()
|| match &arg {
Some(box ast::Expr::Ident(ident)) => ident.sym == js_word!("undefined"),
_ => false,
}
}
_ => false,
})
}

0 comments on commit 31dc690

Please sign in to comment.