perf: Improve scope information collection performance #16923
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58295 |
| } else { | ||
| // @ts-expect-error Expression produces too complex union | ||
| visitor[type] = fns; | ||
| } |
There was a problem hiding this comment.
This is a bug fix, previously it shallow copied fns to multiple places, but it just happened not to be modified. After we added the definition of BlockScoped it was modified later, causing the visitor to be wrong. Here we use mergePair to do a deep clone.
liuxingbaoyu
force-pushed
the
traverseForScope
branch
from
33191e9 to
0e08681
Compare
liuxingbaoyu
added
pkg: traverse (scope)
PR: Performance 🏃♀️
There was a problem hiding this comment.
I think the performance improvement comes from the fact that we avoided creation of traversal context in the simplified traversal. We can investigate if we can share the traversal context used by the sub-traverseNode calls with the root context of the node path where we issued path.traverse() from. If it is possible, it will benefit all Babel plugins.
| @@ -133,8 +133,8 @@ export function stop(this: NodePath) { | |||
| this._traverseFlags |= SHOULD_SKIP | SHOULD_STOP; | |||
| } | |||
|
|
|||
| export function setScope(this: NodePath) { | |||
| if (this.opts?.noScope) return; | |||
| export function setScope(this: NodePath, ignoreNoScope?: boolean) { | |||
There was a problem hiding this comment.
| export function setScope(this: NodePath, ignoreNoScope?: boolean) { | |
| export function setScope(this: NodePath, force?: boolean) { |
setScope is (unfortunately) part of the public API. What do you think abuot adding an internal
function _forceSetScope(path: NodePath) {}instead?
There was a problem hiding this comment.
That's all good to me. :)
I implemented a more complete traversal that fully shares a traversal context, but unfortunately the performance didn't improve. |
|
I believe this PR is possibly causing some problems, e.g. emberjs/ember.js#20806 🤔 I was able to pin stuff down to the 7.26.3 release of |
|
@mydea Thanks for the report! I will take a look. |
Fixes #1, Fixes #2Here we introduced a simplified traversal, which improved the overall performance by about 10%. (Well, I don’t understand why it brings such a big performance improvement. I tried more later but failed. 🤦♂️)