Reduce linear search on list traversing #12302
Conversation
JLHwung
added
pkg: traverse
PR: Performance 🏃♀️
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3a365bf:
|
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/31812/ |
|
This is awesome 😄 |
|
@nicolo-ribaudo @kaicataldo Thanks! I converted this PR to draft since it is breaking TDZ implementation, which I am investigating. |
JLHwung
force-pushed
the
fix-11527
branch
2 times, most recently
from
092c2ba
to
91ecd58
Compare
| visited.push(path.node); | ||
| const { node } = path; | ||
| if (visited.has(node)) continue; | ||
| if (node) visited.add(node); |
There was a problem hiding this comment.
Note that when path.node is null, the current behaviour is to visit this path once but skip other nullish node paths since null can be hold in array. In this PR they will all be visited. This could be a breaking change in a way but I think the equality of two paths whose path.node is null, is not well-defined.
The usage of WeakSet is intentional, so we don't leak memory when path.node is GCed.
| if (!pathCache.has(parent)) { | ||
| let paths = pathCache.get(parent); | ||
| if (!paths) { | ||
| paths = new Map(); |
There was a problem hiding this comment.
We can't use a WeakMap here because we iterate on pathCache in https://github.com/babel/babel/pull/12302/files#diff-e406da86b11f8d4846c6ffb3bf527e1c0fa230888127f90bb8978c16a9bd025aR166
There was a problem hiding this comment.
We can create an IterableWeakMap, falling back to a normal Map on Node.js <13
There was a problem hiding this comment.
Oh the IterableWeakMap is not a builtin object nor supposed to be a public Node.js API, I suggest we hold off a bit since WeakRef (relied by IterableWeakMap) is still stage 3 and @babel/traverse is heavily used.
github-actions
bot
added
the
outdated
Checkout this branch, build Babel and run https://gist.github.com/robmcl4/080bf388c95de3b2761307b39d6f895a
The output on my local machine is attached here for reference
Current main
This PR
In other words, traversing an ArrayLiteral of 4096 elements can be up to 16x faster. The execution time also looks linear.
Note that this issue is not limited to ArrayLiteral, I can reproduce it on any list-style AST structures: e.g. Program, ObjectLiteral, BlockStatement, etc. Although it is uncommon that an array literal may contain 4096 elements, a Program/BlockStatement, especially bundled, could contain thousands of statements as their list children.