-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Use combined node flags #39403
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 combined node flags #39403
Conversation
@typescript-bot perf test. |
Heya @Kingwl, I've started to run the perf test suite on this PR at 84bad5e. You can monitor the build here. Update: The results are in! |
@Kingwl Here they are:Comparison Report - master..39403
System
Hosts
Scenarios
|
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 looks like the right thing, but can you run the perf test again after merging from master? It looks a tiny bit slower, but it could just be noise.
@typescript-bot perf test. |
Heya @Kingwl, I've started to run the perf test suite on this PR at 660b399. You can monitor the build here. Update: The results are in! |
@Kingwl Here they are:Comparison Report - master..39403
System
Hosts
Scenarios
|
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.
The second round of perf results are not significantly slower except in a couple of cases. I think there is probably a very slight slowdown, but also that it's worth it to have the feature work right.
This is ready to merge whenever github's merge-checking code allows me to push the merge button. |
Fixes a part of #39374