-
Notifications
You must be signed in to change notification settings - Fork 13
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
Use zero-based numeric indices for much faster instrumented code #22
Conversation
Closes istanbuljs-archived-repos#21. Makes instrumented code run much faster by making V8 switch to “fast” mode when accessing stats objects.
CLA is valid! |
@bcoe Travis passes, so this ready to go. |
@bcoe @gotwarlost anything I could help with to get this merged? A 15x speedup seems like something that's worth prioritizing. :) |
this is amazing! ❤️ |
@DmitriiAbramov @mourner I see no reason not to land this; but since I haven't heard back from @gotwarlost, I will be testing quite throughly; I'll start by publishing this to a |
@mourner I've published these changes to the
|
Oh this is awesome! Is anyone interested in trying this out on https://github.com/facebook/jest itself? :) (run jest with --coverage). |
Thanks @mourner, I would love to try this out to confirm my issue in gotwarlost/istanbul#556 is gone. Unfortunately I currently rely on https://github.com/karma-runner/karma-coverage which hasn't updated to this library yet (but I'm hoping it happens soon or I can open a PR to do it for them). |
@cpojer i tested it on jest itself, but we don't really instrument much code, so there isn't much we can optimize :) |
@bcoe thanks! The new version works perfectly in our project:
|
So sorry about my lack of response (this is getting old :( ) I see no reason off the top of my head why this should not be merged. 👍 |
ah, it is merged already. Great. |
Closes #21. Makes instrumented code run much faster by making V8 use “fast” mode when accessing stats objects, which wasn't triggered before because of non-0-based indices.
Here are the timings from mapbox-gl-js render suite:
So, basically the performance overhead is 15 times smaller, down from 320% to 20%.
cc @bcoe @mramato