-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 misc/startup-cli-version benchmark #50684
Conversation
90e9da6
to
29b034f
Compare
You could add TypeScript too. Are all of these CommonJS? Is there anything we can add to benchmark ESM startup? |
We don’t have the TypeScript CLI checked in, only one library bundle is in the fixtures (so that's supposed to be
I can’t find any existing CLI checked into the code base using ESM. If we have something in the future or if any one of them gets updated to use ESM then that can be covered. Otherwise I guess this is just what we have for now. This just serves as “since we already have some real-world CLIs checked in the code base, might as well use them for some benchmarking to ensure we don’t regress real-world CLIs”. Whether these real-world CLIs uses ESM is up to them. If we really want to check ESM performance for CLIs that we don’t use, I’d suggest doing it in a different repo (the CLIs listed here are already making the checkout humongous, but then we at least have some proper use of them other than benchmarking). I'd say the same applies to the maintenance of this list - if for some reason we no longer use any of them (I doubt that though, maybe eslint can go away in the future? But the other two probably won't), we should just remove them from this benchmark and should't keep them in the codebase just for benchmarking. |
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI.
29b034f
to
a6eccf6
Compare
Yeah, this is something we need but I agree I don’t want to add unnecessary dependencies to the repo. You could perhaps have another set of spawn calls that include |
That sounds like a separate benchmark than this one, which is about real world CLIs. I haven’t seen any CLIs that would be loaded this way in the real world (maybe there are, but I just haven’t seen any) |
Landed in 59b27d6 |
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI. PR-URL: #50684 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI. PR-URL: nodejs#50684 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI. PR-URL: #50684 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>
tools/node_modules is removed from the tarball so it should not be used as part of unit tests or benchmarks. Fixes: nodejs#51145 Refs: nodejs#50684
tools/node_modules is removed from the tarball so it should not be used as part of unit tests or benchmarks. Fixes: nodejs#51145 Refs: nodejs#50684
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI. PR-URL: #50684 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>
In previous version of this fix, I've simply added a check if the tested tool is available or not. Unfortuntelly, this fails when only the first tool is to be run as part of the test-benchmark-misc, and it doesn't exist. benchmark/test-benchmark-misc ... AssertionError [ERR_ASSERTION]: benchmark file not running exactly one configuration in test: ... misc/startup-cli-version.js ... The solution is to move the tool that is not present in a tarball down the list. Fixes: nodejs#51146 Refs: nodejs#50684
PR-URL: nodejs#51746 Refs: nodejs#51146 Refs: nodejs#50684 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#51746 Refs: nodejs#51146 Refs: nodejs#50684 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This benchmarks the startup of various CLI tools that are already checked into the source code. We use --version because the output tends to be minimal and fewer operations are done to generate these so that the startup cost is still dominated by a more indispensible part of the CLI.