-
Notifications
You must be signed in to change notification settings - Fork 191
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
Fix stripping local debug code so can that dead-code-elimination produces smaller bundles with fewer function calls (part 1) #1606
Conversation
duration phase no difference [-175ms to 29ms] [16:59:30] Generating Benchmark Reports [started]
JSON: /home/runner/work/glimmer-vm/glimmer-vm/tracerbench-results/compare.json PDF: /home/runner/work/glimmer-vm/glimmer-vm/tracerbench-results/artifact-1.pdf HTML: /home/runner/work/glimmer-vm/glimmer-vm/tracerbench-results/artifact-1.html |
…tom babel plugin...
…do a custom babel plugin..." This reverts commit 5cddba2b0a4616350ba4d9da572863e77cad0ffe.
…e can inline the package" This reverts commit 57406f687a7991b8b35c907b1e7483bb40a4d521.
45ce614
to
19303b5
Compare
…uldn't figure out -- I also left some notes about the problem that terser is running in to -- it seems to be related to indirection of identity functions that 'passes' isn't smart enough to figure out We get big wins (side-wise) from sideEffects: false, but things break. This babel plugin is feeling more and more brittle, the more we want to strip.
19303b5
to
affee7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work!
Goals:
inline@glimmer/debug
so we can strip away via dead-code-elimination the calls tocheck(...)
Turns out, this wasn't needed, and we can keep
@glimmer/debug
separate -- terser seems to have figured out now that functions from@glimmer/debug
are not needed in the consuming packages.Of note, in ember's build, the output of
@glimmer/debug
is changed to things like:which is great!
But this keeps the
check
call in source, unless production mode is smart enough to see that the NoopChecker can be removed.Maybe we should move the production checks to the
check
call sites if we determine they remain in production ember apps.There are a number of flags that are meant to be internal to glimmer-vm and not exposed to ember.js.
There are also flags that are meant to be exposed to ember.js (which are swapped out with DEBUG from '@glimmer/env' during the build)
Output size (using dust)
I applied this branch to ember-source 5.10, (which can be tested here) and came out with these production build sizes:
It is 17K smaller overall.
That's still far from the 52KB production vendor growth reported by emberjs/ember.js#20628
In a future PR, I'd like to switch the build to SWC, because SWC's minifier solves the problem we have:
https://play.swc.rs/?version=1.7.24&code=H4sIAAAAAAAAA12OTQqDMBCF957iIS4iFALJ0ssYdYqCNSEmNKl496at1trd%2B%2FnmMZzjbgdH0BZGzUm0uiP0ZCm7%2Bql1g54QWCixwJLzNrkK69FFFi6IFzz%2BiCo7GNW0TP32yZ1GEiBORNyIjILR1uFLUlA3MxJLbAZwDu2d8Q5zr%2F3YoaFPus3UAgISsq5SvGfFEpgoVxRLZLnI3%2Br1odyVYLlMcTpanwuGSssfAQAA&config=H4sIAAAAAAAAA31VPY%2FbMAzd71cEmjsUGTp0vivQIehQ4FZBsShHV1kyRCqXIPB%2FLy07H5fQ2Sw%2BPlLix%2FPpZbVSH9ion6sTf%2FKhNxkhX85swWMkc2CLomMP2GTfk%2Fp2RglHyJmAUE3DhCgyuQUaWYDr7%2Bv1zFAhJYQzY7Z1Pnp3vM3ZpK7PgHhjYyuHLB1Ewq%2F8GcvpcwQol1v7NqUAJj5BtEHtI0ELWQrcpBBMj6D3JgtRxpua7DFJKUawEFjd59SLeLSefIqc8xG1YKxukgUB8hka8nuQaJyLaRH5ecJ7KmxhW9q29vmODXsTiiEhJxxqS%2Fi2QtRd8kjalSiVcAIXajCBc3Hvmd7pDFRyfOR9JB8XevIPgCsQDGI0HUhxq4fjeVpiu6dMHx2PLB0FnOdbemWElouqvXdCZcfKQCYvdTODLQ2MlW2k68zwQvnQW9DgHM%2BKEBo%2FPTU7Kem46MkJAPfXOGmqJkBftnABHxfiCfyLX0nygM0enaHdMorHbpvCkwQd0C7ZJw7cCkrLcGaVOPTLeIkWeDTAii4FK%2FAoArwAlHSoevkwG7weHFG3IW2vMjE7DBcd7kxs7%2Ff9osWZZc6l3H2VWE67SbaEOgEndU1wUlUf6tfvv%2Fr17V1v%2Fry%2BjWJeo6thGKYEL3MS1dVA19%2FIOEGT%2BP9QV6ezzl%2FuqDxuzsz6rOE%2FQnOQmpIGAAA%3D
Terser fails at this:
And terser failing at this is one of the reasons we've had so much dev code in prod