-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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 O(n^2) getLetReferences – 40% faster on large flat files #7028
Conversation
`this.blockPath.get("body")` constructs an array of paths corresponding to each node in `blocks.body` so takes O(n) time if n is that length. We were re-constructing that array on each iteration, so the entire loop was O(n^2). On files with many statements in a single block (such as Rollup-generated bundles), this takes a large portion of time. In particular, this makes transforming react-dom.development.js about 40% faster. Not that you should be transforming our bundle with Babel. Test Plan: Make an HTML file with these three lines and watch it in the Chrome Performance tab to see timings (on my machine: 2.9s before, 1.6s after): ``` <!DOCTYPE html> <script src="https://unpkg.com/babel-standalone@7.0.0-beta.3/babel.js"></script> <script type="text/babel" src="https://unpkg.com/react-dom@16.2.0/umd/react-dom.development.js"></script> ```
7be71dc
to
6a7223a
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/6187/ |
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.
Gr8 finding. Caching <3
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.
Good catch! Thanks @sophiebits
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.
Thank you very much, Sophie! 🎉
I'm going ahead and merging this. Thanks @sophiebits |
this.blockPath.get("body")
constructs an array of paths corresponding to each node inblocks.body
so takes O(n) time if n is that length. We were re-constructing that array on each iteration, so the entire loop was O(n^2).On files with many statements in a single block (such as Rollup-generated bundles), this takes a large portion of time. In particular, this makes transforming react-dom.development.js about 40% faster because the bundle has 755 statements in one block. Not that you should be transforming our bundle with Babel.
Test Plan:
Make an HTML file with these three lines and watch it in the Chrome Performance tab to see timings (on my machine: 2.9s before, 1.6s after):