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

fix: upgrade to newer versions of source-map, signal-exit, and instrument #389

Merged
merged 4 commits into from
Sep 13, 2016

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Sep 13, 2016

a few somewhat frightening dependencies being upgraded, we should test thoroughly.

@mourner mind taking this for a spin, it's what actually lands your performance fixes (along with some performance fixes I snuck into #388, yargs should now execute much less frequently).

@mourner
Copy link
Contributor

mourner commented Sep 13, 2016

@bcoe I'll try this out, thanks!

@mourner
Copy link
Contributor

mourner commented Sep 13, 2016

@bcoe this branch works great for me.

@bcoe
Copy link
Member Author

bcoe commented Sep 13, 2016

@mourner seeing any significant performance gains, vs. latest?

@mourner
Copy link
Contributor

mourner commented Sep 13, 2016

@bcoe seeing the same performance gains I described in istanbuljs-archived-repos/istanbul-lib-instrument#22

@bcoe bcoe force-pushed the upgrade-dependencies branch from 6d44313 to aa06525 Compare September 13, 2016 17:15
@bcoe
Copy link
Member Author

bcoe commented Sep 13, 2016

@mourner cool, was curious if spawning yargs less frequently (which I believe has a 100ms overhead) would improve performance further -- this would specifically target people using nyc for instrumentation, rather than babel-plugin-istanbul.

@bcoe bcoe merged commit a9bdf0f into master Sep 13, 2016
@bcoe bcoe deleted the upgrade-dependencies branch September 13, 2016 18:39
@mourner
Copy link
Contributor

mourner commented Sep 14, 2016

@bcoe it probably shaved off a few hundred ms, but it's not really noticeable with a test suite that runs for 6+ minutes. :)

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

Successfully merging this pull request may close these issues.

2 participants