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

Is estraverse necessary? #32

Open
STRML opened this issue Nov 8, 2019 · 2 comments
Open

Is estraverse necessary? #32

STRML opened this issue Nov 8, 2019 · 2 comments

Comments

@STRML
Copy link

STRML commented Nov 8, 2019

This is the first babel plugin I've seen that simply uses a Program entrypoint then uses estraverse all over (https://github.com/power-assert-js/babel-plugin-espower/blob/4932320d9eecffca0dc38dc7fc2684ffffca1c6c/lib/babel-assertion-visitor.js) rather than use the visitors provided by Babel itself.

This additional/redundant parsing makes estraverse's functions the heaviest in a profile run of one test file, screenshotted below:

Screen Shot 2019-11-08 at 11 33 46 AM

This leads to us finding estraverse code at the bottom of a significant proportion of the flame graph:

Screen Shot 2019-11-08 at 11 36 09 AM

How necessary is this module? Can the plugin be modified to follow babel conventions more closely?

Time with babel-plugin-espower enabled:

env BABEL_DISABLE_CACHE=1 time ./node_modules/.bin/mocha test/spec/fooSpec.js
...
       11.28 real        13.31 user         1.03 sys

With it disabled:

env BABEL_DISABLE_CACHE=1 time ./node_modules/.bin/mocha test/spec/fooSpec.js
...
      7.29 real         8.85 user         0.96 sys
@STRML
Copy link
Author

STRML commented Nov 8, 2019

FWIW, I can get the average time down to 9 sec real just by replacing:

https://github.com/estools/estraverse/blob/54d608c4ce0eb36d9bade685edcc3177e90e9f3c/estraverse.js#L375-L378

with:

this.__keys = visitor.keys || VisitorKeys;

with no loss of functionality.

@twada
Copy link
Member

twada commented Nov 9, 2019

@STRML Thank you for your suggestion!

This is the first babel plugin I've seen that simply uses a Program entrypoint

Because babel-plugin-espower requires massive AST rewrite on leaving nodes (not entering nodes) so cannot coexist with other plugins rewriting nodes on enter. Some babel plugin that also require massive rewrite, such as babel-plugin-istanbul uses Program node as entrypoint and traverses by itself.

then uses estraverse all over rather than use the visitors provided by Babel itself.

Nice point, as the time I wrote babel-plugin-espower babel traverser is feature rich and slower than estraverse so I chose estraverse. But now you found estraverse's functions the heaviest, It's time to reconsider implementation details (maybe babel gets much faster than ever). Thanks a lot.

One more nice suggestion you gave me is the initialization cost of estraverse. There's much room for improvement to make estraverse to not to initialize it on every traverse call.

I'll work on them. Thank you!

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

No branches or pull requests

2 participants