-
-
Notifications
You must be signed in to change notification settings - Fork 724
feat(codegen): keep arrow function PIFEs #12353
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
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
CodSpeed Instrumentation Performance ReportMerging #12353 will not alter performanceComparing Summary
|
|
@oxc-project/core Does this approach make sense? If it does, I'll add |
|
I'm wondering if the additional I assume this alternative approach would also work:
Current state of play appears to be: // PIFEs retained in codegen, functions unwrapped in minifier. Good!
(() => alert(1))();
(() => { alert(2); })();
// PIFEs retained in codegen and minifier,
// but not optimal minification - could unwrap functions
!function() { alert(3); }();
!function() { alert(4); }(), function() { alert(5); }();
// PIFE lost. Bad!
f1 = (function() { alert(6); });
f2 = (() => alert(7));
f3 = (() => { alert(8); });So, unless I'm missing something, it looks like the last 3 cases ( Personally I think it might be preferable not to introduce a |
Parentheses are not supposed to have meanings ... minifier is free to discard it, or it will break. Parser also has a |
Let me think about this. |
|
Yeah, I mainly went with this approach considering |
|
My thoughts:
|
Merge activity
|
PIFE is the abbreviation of "Possibly-Invoked Function Expressions". It is a function expression wrapped with a parenthesized expression. PIFEs annotate functions that are likely to be invoked eagerly. When v8 encounters such expressions, it compiles them eagerly (rather than compiling it later). See [v8's blog post](https://v8.dev/blog/preparser#pife) for more details. The blog post only mentions regular FunctionExpressions, but ArrowFunctions are also supported. The cases that will be eagerly compiled are: - when `!` comes before function literals ([code](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L3730-L3733)) (e.g. `!function(){}`) - when a function expression is wrapped with parenthesis ([code](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L2243-L2248)) (e.g. `(function () {})`, `(async function () {})`) - when an arrow function is wrapped with parenthesis ([code1](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L2173-L2174), [code2](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L2232-L2259), [code3](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L3297-L3298)) (e.g. `console.log((() => {}))`) - when `()` or ``` `` ``` (tagged templates) comes after function literals (only in some cases) ([code1](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L3987-L3992), [code2](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L4344-L4348)) (e.g. `~function(){}()`, ```~function(){}`` ```) - when [explicit compile hints](https://v8.dev/blog/explicit-compile-hints) are used ([code1](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser.cc#L2717-L2720), [code2](https://github.com/v8/v8/blob/328f6c467b940f322544567740c9c871064d045c/src/parsing/parser-base.h#L5070-L5073)) (e.g. `//# allFunctionsCalledOnLoad`) Keeping PIFEs as-is in the code generation will unlock optimizations like rolldown/rolldown#5319. _Note: this PR only implements arrow function PIFEs for now._
e67a11e to
0920e98
Compare
This upgrade adds * oxc-project/oxc#12353 * oxc-project/oxc#12373 * oxc-project/oxc#12393 * oxc-project/oxc#12394 * oxc-project/oxc#12398 * oxc-project/oxc-sourcemap#86 We should expect a small performance improvement when sourcemap is enabled.
This PR changes the generated code to use PIFE for callbacks passed to `__esmMin` wrapper. This improved the start up runtime performance by 2.1x in the case I tested. I tested this change on https://github.com/rolldown/benchmarks/tree/main/apps/10000 with the following config: ```ts import { defineConfig } from "vite"; export default defineConfig({ build: { rollupOptions: { output: { advancedChunks: { groups: [ { name: 'some' } ] } } } }, experimental: { enableNativePlugin: true, }, esbuild: false, }); ``` I also updated the HTML to measure the render time. ```html <!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8" /> <link rel="icon" type="image/svg+xml" href="/vite.svg" /> <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <title>Vite + React</title> </head> <body> <div id="root"></div> <script type="module"> globalThis.start = performance.now() </script> <script type="module" src="./src/index.jsx"></script> <script type="module"> const end = performance.now() console.log('render time:', end - globalThis.start) </script> </body> </html> ``` The results were (average of 5 times): | | before | after | |-------------------| ---------- | ------ | | dev (no minify, hmr=true) | 295.3 ms | 139.7 ms | | build (minified) | 190.1 ms | 97.2 ms | I tested this on Chrome 138.0.7204.101. Also note that this change increases the output code size slightly: **Before**: 6,338.19 kB (gzip: 1,871.28 kB) **After**: 6,376.20 kB (gzip: 1,874.64 kB) requires oxc-project/oxc#12353 --- **Note: The same optimization can be applied to `__commonJSMin`, `__esm`, `__commonJS`, `createEsmInitializer`.**
PIFE is the abbreviation of "Possibly-Invoked Function Expressions". It is a function expression wrapped with a parenthesized expression.
PIFEs annotate functions that are likely to be invoked eagerly. When v8 encounters such expressions, it compiles them eagerly (rather than compiling it later). See v8's blog post for more details.
The blog post only mentions regular FunctionExpressions, but ArrowFunctions are also supported. The cases that will be eagerly compiled are:
!comes before function literals (code) (e.g.!function(){})(function () {}),(async function () {}))console.log((() => {})))()or``(tagged templates) comes after function literals (only in some cases) (code1, code2) (e.g.~function(){}(),~function(){}``)//# allFunctionsCalledOnLoad)Keeping PIFEs as-is in the code generation will unlock optimizations like rolldown/rolldown#5319.
Support by other engines
Note: this PR only implements arrow function PIFEs for now.