power-assert-js / babel-plugin-espower

Babel plugin for power-assert
MIT License
93 stars 6 forks source link

Is estraverse necessary? #32

Open STRML opened 4 years ago

STRML commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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!