Skip to content
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

Avoid task allocations for TextDocument.GetTextAsync #68090

Merged
merged 1 commit into from
May 4, 2023

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented May 4, 2023

Updates production code to use GetValueTaskAsync or GetTextSynchronously, as appropriate, to avoid task allocations in a call to GetTextAsync.

@sharwell sharwell requested review from 333fred, dibarbet and a team as code owners May 4, 2023 16:47
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 4, 2023
Updates production code to use GetValueTaskAsync or
GetTextSynchronously, as appropriate, to avoid task allocations in a
call to GetTextAsync.
var sourceText = await document.GetTextAsync().ConfigureAwait(false);
var textContainer = sourceText.Container;
return textContainer.TryGetTextBuffer();
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This is the only one that changed significantly

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@sharwell sharwell merged commit 2f32425 into dotnet:main May 4, 2023
@sharwell sharwell deleted the value-task-cleanup branch May 4, 2023 18:35
@ghost ghost added this to the Next milestone May 4, 2023

return new ValueTask<SourceText>(document.GetTextAsync(cancellationToken));
}
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would be supportive of you makign an API proposal for this and going through public API review as well. (not blocking this PR of course).

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

Successfully merging this pull request may close these issues.

4 participants