Optimize allocations in TextReader.ReadToEnd utilized by SourceText.From #70017
Conversation
This file contains 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
The profile I'm looking currently shows 9.4% of allocation in our codeanalysis process under SourceText.From. These allocations occur during the TextReader.ReadToEnd call. Fortunately, Roslyn is already passing in a TextReader that knows it's length, and thus we can optimize this path. In netfx, this should at least get rid of the extra sizing allocations done during TextReader.ReadToEnd as it appends to it's internal stringbuilder. So, this should see a reduction of about 20% of the allocations in this codepath. In netcore (which is the case for this profile), we do better as this essentially gets rid of all allocations except for the equivalent of the StringBuilder.ToString call. So, this should see a reduction of about 2/3 of the allocations of this codepath.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
tmat
approved these changes
Sep 19, 2023
tmat
requested changes
Sep 19, 2023
There was a problem hiding this comment.
Actually, the previous impl ended up calling LargeText.Decode if the length was greater than LargeObjectHeapLimitInChars. The new impl would allocate the string on LOH
Should we call SourceText.From(reader, length, encoding, checksumAlgorithm) from CreateText?
| public override string ReadToEnd() | ||
| { | ||
| #if NETCOREAPP | ||
| return string.Create(Length, this, static (chars, state) => state.Read(chars)); |
|
That's a good observation. I thought I was being smart with the TextFactoryService change, but I didn't consider the LOH case. In reply to: 1633907243 |
…override that takes in the length as it prevents LOH allocations.
tmat
approved these changes
Sep 19, 2023
ghost
added this to the 
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The profile I'm looking currently shows 9.4% of allocation in our codeanalysis process under SourceText.From. These allocations occur during the TextReader.ReadToEnd call. Fortunately, Roslyn is already passing in a TextReader that knows it's length, and thus we can optimize this path.
In netfx, this should at least get rid of the extra sizing allocations done during TextReader.ReadToEnd as it appends to it's internal stringbuilder. So, this should see a reduction of about 20% of the allocations in this codepath.
In netcore (which is the case for this profile), we do better as this essentially gets rid of all allocations except for the equivalent of the StringBuilder.ToString call. So, this should see a reduction of about 2/3 of the allocations of this codepath.