-
Notifications
You must be signed in to change notification settings - Fork 394
perf: speed up all commands by 75+ ms by enabling V8 code cache when available (node>=22.8.0
)
#7173
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
Conversation
Our minimum supported version is 18, so we need to rely on node 18 types.
@@ -203,7 +204,6 @@ | |||
"eslint-config-prettier": "^10.1.1", | |||
"eslint-plugin-n": "^17.16.1", | |||
"form-data": "4.0.2", | |||
"is-ci": "4.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was unused
"@types/node": "22.13.10", | ||
"@types/node": "18.19.86", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was incorrectly upgraded. We support min. 18 so we need v18 types.
The `preAction` hook was overwriting the whole payload, so any prior calls to `setAnalyticsPayload` would have their data thrown away. This didn't matter, but I'd like to introduce an early call.
See https://nodejs.org/api/module.html#module-compile-cache. In local benchmarking on various machines, this led to a 75-400 ms improvement to command run time across the board, at the expense of a moderate increase in first-time run time. See inline - to account for the common CI single-run use case, this is disabled when we detect a CI environment.
node>=22.8.0
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestions around how to make this a little cleaner and better documented--address those as you see fit. I marked the only potential blocking issue with a
This also removes the child process handling.
🐎 |
Summary
See https://nodejs.org/api/module.html#module-compile-cache.
In local benchmarking on various machines, this led to a 75-400 ms improvement to command run time across the board, at the expense of a moderate increase in first-time run time.
See inline - to account for the common CI single-run use case, this is disabled when we detect a CI environment.
See individual commits for context on the other changes here.