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

Make hoist transform ~35% faster #4826

Closed
wants to merge 2 commits into from
Closed

Make hoist transform ~35% faster #4826

wants to merge 2 commits into from

Conversation

devongovett
Copy link
Member

Depends on #4815.

Screen Shot 2020-06-29 at 5 36 43 PM

This refactors the hoist transform in a similar way to how #4815 did for the linker, replacing @babel/traverse with a custom implementation, and using a custom scope tracker. It's ~35% faster to build the asset graph on the esbuild benchmark after this transform. Looking at a profile, actual transformation is no longer the bottleneck.

The hoist transform now happens in a single pass over the AST, rather than waiting for babel's scope tracker to do a first pass before our transform. Now, we collect scope information as we traverse the AST using a custom scope implementation which tracks bindings and references. All renaming of variables happens at the when exiting each scope, once all references are known.

For any transforms that need scope information, the traversal function now allows you to return a function from your visitor. This marks the node as needing to be revisited at the end of the traversal. Once the whole AST has been traversed, the functions for nodes to be revisited are executed, and any nodes that are returned are inserted into the AST at that point.

@height
Copy link

height bot commented Jun 30, 2020

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants