-
-
Notifications
You must be signed in to change notification settings - Fork 862
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
Improve require speed #358
Conversation
With newer version of nodejs(I tested on 12.10.0) all the require speeds increased quite a lot and With this in mind, I think inlining basic color converters in this module to reduce require time even further doesn't really make sense anymore, as the gains should be minor. So I consider the patch complete in terms of functionality. (Though it still makes some sense to do in general, but can be implemented in |
Since we’re removing undocumented colour spaces, this is probably a breaking change. |
@ExE-Boss changes in undocumented features is not considered API change, and not a breaking one for sure. Though in any case this code is scheduled for chalk 3.0 release as far as I understand. |
@stroncium Can you fix the merge conflict? |
098194b
to
2fd15c6
Compare
Awesome. Thanks for working on this 🙌 |
@stroncium out of curiosity: what tool did you use to create that terminal screenshot in the PR description? |
@Turbo87 That would be my private tool for node performance metrics and optimization. Didn't get to write docs for it and release publicly yet, sorry. |
=== undefined
check)ansi-styles
color space converters unless neededThis patch works in conjunction with(but works without them too, though to much smaller effect):
Require speed test results before:
Require speed test results after(pure require, without using template nor color conversion functionality):
Note: in both cases, a huge part of time is spent initializing
tty
(or initializingprocess.stdout
andprocess.stderr
, which includes initializingtty
), which is also triggered by a lot of other parts of stdlib(for example byconsole.log
with non-string arguments), and is required in order to detect if output supports color, so is expected to be initialized anyway by a lot of real life programs(notable exception: programs which conditionally produce no output). If we subtract the time required for this initializations, the final numbers will be ~10ms and ~5ms, respectively.Fixes #180
IssueHunt Summary
Referenced issues
This pull request has been submitted to:
IssueHunt has been backed by the following sponsors. Become a sponsor