Switched 'ts.performance' to a mixed mode only uses native performance APIs when necessary#42586
Merged
Switched 'ts.performance' to a mixed mode only uses native performance APIs when necessary#42586
Conversation
amcasey
approved these changes
Feb 1, 2021
Member
amcasey
left a comment
There was a problem hiding this comment.
The original repro had emit time tripled from a TS 4.0 baseline. With this change, they're back within 10% of TS 4.0. Since the repro is noisy and the number of other changes is huge, I'm satisfied.
Member
|
@typescript-bot perf test this |
Collaborator
|
Heya @amcasey, I've started to run the perf test suite on this PR at a20bfa2. You can monitor the build here. Update: The results are in! |
Collaborator
|
@amcasey Here they are:Comparison Report - master..42586
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
Author
|
Perf numbers look good to me |
Member
|
It looks like nothing in the perf suite does a meaningful amount of emit, which is probably why we missed the regression. Should we add something? (Unfortunately, I don't have an example in mind.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@amcasey recently discovered that the Web Performance API in NodeJS adds quite a bit more overhead to the total compile time than what we were doing before. This changes our use of native performance APIs in the following ways:
marktimes andmeasuredurations ints.performancerather than usingPerformanceObserver.markandmeasurewhen any of the following are true:--cpu-profoption specified (to capture user timing events in the profile)--profoption specified (to capture user timing events in the profile)--generateCpuProfileoption specified (to capture user timing events in the profile)sys.debugModeis true (since a debugger can start a cpu profile at any time)If none of those cases are true, we will not call the native
markandmeasuremethods. I've filed nodejs/diagnostics#464 to track the performance issue we encountered in NodeJS, and it sounds like they plan to have a patch available in the near future that improves their implementation.