Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce allocations in CompletionItem.GetEntireDisplayText #76373

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

ToddGrun
Copy link
Contributor

This method's call to string.Concat shows up as about 0.4% of allocations during the typing scenario in the csharp editing speedometer test.

Manual local testing was showing over 90% of calls to this method are done with both prefix and suffix strings empty. Unfortunately, netfx's string.Concat implementation still allocates in this case. Instead, we'll check for this case, and either do the string.Concat if necessary or just use DisplayText directly.

*** allocations of interest from the csharp editing speedometer test ***
image

This method's call to string.Concat shows up as about 0.4% of allocations during the typing scenario in the csharp editing speedometer test.

Manual local testing was showing over 90% of calls to this method are done with both prefix and suffix strings empty. Unfortunately, netfx's string.Concat implementation still allocates in this case. Instead, we'll check for this case, and either do the string.Concat if necessary or just use DisplayText directly.
@ToddGrun ToddGrun requested a review from a team as a code owner December 11, 2024 12:42
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 11, 2024
@CyrusNajmabadi
Copy link
Member

Unfortunately, netfx's string.Concat implementation still allocates in this case

Wat

@ToddGrun
Copy link
Contributor Author

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants