-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Rewrite createTextWriter to buffer appends #44242
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
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at 43bcc88. You can monitor the build here. Update: The results are in! |
|
@amcasey Here they are:Comparison Report - master..44242
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
@amcasey Here they are:Comparison Report - master..44242
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
This appears to have cut memory allocations by ~10% in the target internal project. Edit: Interestingly, the time improvement is much smaller - closer to 1%. Maybe that's because emit is at the end of the pipeline and doesn't really need to be cleaned up before the process exits? |
|
I switched back from |
|
How's the perf if you only replace the computeLineStarts() call with a scan for count and last line start position? |
|
Optimizing the slow case (i.e. when you need to do line math) isn't very worthwhile since it's so rare. The speed win comes from not calling |
|
@amcasey |
|
@typescript-bot perf test this |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here they are:Comparison Report - main..44242
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Markdown test Successfully analyzed 1 of 3 visited repos
|
|
@typescript-bot user test this |
|
Heya @amcasey, I've started to run the diff-based user code test suite on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
Heya @amcasey, I've started to run the diff-based top-repos suite on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
@amcasey Here are the results of running the user test suite comparing Everything looks good! |
|
@amcasey Here are the results of running the top-repos suite comparing Everything looks good! |
|
@typescript-bot user test tsserver |
|
@typescript-bot user test tsserver |
|
Heya @amcasey, I've started to run the diff-based user code test suite (tsserver) on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
@amcasey Here are the results of running the user test suite comparing Everything looks good! |
|
@typescript-bot test tsserver top100 |
|
Heya @amcasey, I've started to run the diff-based top-repos suite (tsserver) on this PR at 457a7c6. You can monitor the build here. Update: The results are in! |
|
@amcasey Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Detailspalantir/blueprint
|
|
Closing in favor of #55657. |
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
A more ambitious version of #44241