Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: replace empty functions with noopQrl #6871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions packages/qwik/src/optimizer/core/src/parse.rs
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
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 ==

[]
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 ==

[]
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
============================= test.tsx_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
]
}
*/
============================= test.tsx_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
]
}
*/
============================= test.tsx_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
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 @@ -3592,6 +3592,35 @@ fn impure_template_fns() {
});
}

#[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
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,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 @@ -657,7 +658,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 @@ -2430,3 +2431,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),
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,
})
}
Loading