-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
benchmark: add benchmark for coverage #36972
Conversation
@joyeecheung I was thinking this benchmark would be a really good use of |
Subsystem in commit message should be |
d206807
to
1d8a88b
Compare
c5507d4
to
b5eadb0
Compare
b5eadb0
to
efea2d6
Compare
@Trott refactored so that the benchmark is easier to use, it now spawns a subprocess with coverage enabled (_so no need to create a separate bin) 👍 |
efea2d6
to
ed4e294
Compare
This comment has been minimized.
This comment has been minimized.
ed4e294
to
682f4e9
Compare
This comment has been minimized.
This comment has been minimized.
682f4e9
to
ae141a5
Compare
This comment has been minimized.
This comment has been minimized.
ae141a5
to
2823b83
Compare
2823b83
to
56cf8b5
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
PR-URL: #36972 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Landed in 8c9dc4e |
PR-URL: #36972 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #36972 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Working on #36956, I became a bit worried that as we extend code coverage to support more types of branching, performance might start to become unacceptable.
I created this benchmark to experiment:
Node 14.15.4, NODE_V8_COVERAGE, vs., no coverage:
process/coverage.js n=100000 *** -18.94 % ±0.77% ±1.02% ±1.30%
Node 15.6.0, NODE_V8_COVERAGE, vs., no coverage:
process/coverage.js n=100000 *** -17.95 % ±1.08% ±1.43% ±1.83%
Coverage with #36956, vs., no coverage:
process/coverage.js n=100000 *** -19.37 % ±1.36% ±1.79% ±2.30%
To Summarize
In the current release of 14/15, we see that running Node with coverage decreases performance by about 18%.
With the introduction of optional chaining in this benchmark, we see a 1 - 2% additional hit to performance.
Other interesting facts
coverage.js
using Istanbul, vs., NODE_V8_COVERAGE, it was 37.46% slower than no coverage.0.37 % ±1.14% ±1.50% ±1.92%
).Conclusions
CC: @schuay