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

Prevent StringBuilder resizes during SyntaxNode.GetText #70447

Conversation

ToddGrun
Copy link
Contributor

In the profile I'm looking at, CSWin32.SourceGenerator is the primary caller of this method, but the ExtractMethodCodeRefactoringProvider also works it's way down into this code via SemanticDocument.CreateAsync.

In the profile I'm looking at, CSWin32.SourceGenerator is the primary caller of this method, but the extractmethod coderefactoring provider also works it's way into this code.
@ToddGrun ToddGrun requested a review from a team as a code owner October 18, 2023 23:26
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 18, 2023
@CyrusNajmabadi
Copy link
Member

In the profile I'm looking at, CSWin32.SourceGenerator

@tarekgh do you know who could look at this?

@tarekgh
Copy link
Member

tarekgh commented Oct 19, 2023

@jkoritzinsky are you the right person to answer questions regarding CSWin32.SourceGenerator?

@jkoritzinsky
Copy link
Member

@AArnott is one of the owners of CsWin32. He'll know who to contact.

@AArnott
Copy link
Contributor

AArnott commented Oct 19, 2023

I maintain CsWin32 pretty exclusively. Is there a more efficient way for CsWin32 to be doing something? If so, I'm happy to make the fix.

@jaredpar
Copy link
Member

@dotnet/roslyn-compiler PTAL

@CyrusNajmabadi
Copy link
Member

I maintain CsWin32 pretty exclusively. Is there a more efficient way for CsWin32 to be doing something? If so, I'm happy to make the fix.

Generally, it would be good for an SG to not use syntax nodes as an intermediary representation. They're massively expensive. See the convo on using a simple indenting string writer. If intermediary values need to be stored, keeping it as a simple string which can be blitted to the final doc would be ideal.

@AArnott
Copy link
Contributor

AArnott commented Oct 19, 2023

@CyrusNajmabadi Understood. I know the thread you're talking about and you make some solid arguments. But what you suggest will be a massive rewrite of CsWin32, so it's not likely to happen soon. As I think I mentioned over at that other thread, I might work it in as part of the rewrite to enable incremental source generation.

@CyrusNajmabadi
Copy link
Member

@AArnott Sounds good :)

@ToddGrun ToddGrun merged commit 62905f0 into dotnet:main Oct 19, 2023
@ghost ghost added this to the Next milestone Oct 19, 2023
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers 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.

9 participants