-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Only selectively deoptimize call parameters #4892
Conversation
✅ Deploy Preview for rollupjs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#selective-parameter-deoptimization or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #4892 +/- ##
==========================================
+ Coverage 98.98% 99.01% +0.03%
==========================================
Files 219 221 +2
Lines 7968 8025 +57
Branches 2194 2209 +15
==========================================
+ Hits 7887 7946 +59
Misses 26 26
+ Partials 55 53 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
2cb3c8a
to
e69c628
Compare
While this seems to work, it has abysmal performance as evidenced by the core-js test. A better approach might be only deoptimize top-level parts.
e69c628
to
ea44dcb
Compare
I tested this on several larger projects and both performance and output seem to be fine. Will release this soon. |
5020c98
to
7a17845
Compare
This PR has been released as part of rollup@3.19.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
Before, any call to a function would deoptimize all call parameters meaning that they become "unknown", even if the function was pure.
This PR changes the logic to actually track what mutations happen with a function parameter and apply only the corresponding deoptimizations to the call parameters.
There are some limitations as the initial naive implementation had terrible performance: Deoptimizations are only tracked one level deep. If a nested property is reassigned, i.e.
then this is treated as if the top-level property, i.e.
arg.a
, was reassigned instead. This also applies to property accesses because we do not track if deeper properties are setters/getters. So in the example, just the linearg.a.b
would deoptimizearg.a
.But I think this should be good enough for most purposes and is already a huge improvement over the previous "deoptimize everything" approach.
I also applied this optimization to known global functions: Now all known global functions except
Object.freeze
,Object.defineProperty
andObject.defineProperties
employ this logic and no longer deoptimize their arguments.