Skip to content

Conversation

@ToddGrun
Copy link
Contributor

CSharpAttributeData.IsTargetEarlyAttribute was calling the Linq version of this list, and thus boxing the SeparatedSyntaxList enumerator, showing up as 5.9 MB (0.1%) of allocations in our OOP process during solution load.

Not a huge win, but it's nice to add to the pit of success for using this type.

Modifications to CSharpAttributeData started with a small cleanup to IsTargetEarlyAttribute to demonstrate the Count() usage. I then looked to see what other Linq usage was in the file, and it made sense to change those too.

*** Allocations in question ***
image

CSharpAttributeData.IsTargetEarlyAttribute was calling the Linq version of this list, and thus boxing the SeparatedSyntaxList enumerator, showing up as 6.5 MB (0.1%) of allocations in our OOP process during solution load.

Not a huge win, but it's nice to add to the pit of success for using this type.

Modifications to AttributeData just started with changing IsTargetEarlyAttribute to demonstrate the Count usage. I then looked to see what other Linq usage was in the file, and it made sense to change those too.
@ToddGrun ToddGrun requested a review from a team as a code owner April 28, 2025 14:17
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 28, 2025
return list.IndexOf(kind) >= 0;
}

internal static int Count<TNode>(this SeparatedSyntaxList<TNode> list, Func<TNode, bool> predicate)
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 28, 2025

Choose a reason for hiding this comment

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

internal static int Count(this SeparatedSyntaxList list, Func<TNode, bool> predicate)

Is there something C# specific in this helper? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, and I'm very amenable to moving this wherever you think appropriate. I saw the linq-like extension methods right above this one, so that is where I put this, but if you think there's a better place for this, I'll go ahead and move it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is nothing C# specific in this helper, I think it should not be added to CSharpExtensions. Please look for a better place so that VB code could also take advantage of the helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to a non-C# specific location, but kept as an extension method.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 1)

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 2)

@ToddGrun ToddGrun enabled auto-merge (squash) April 28, 2025 16:56
@ToddGrun ToddGrun merged commit 65cc369 into dotnet:main Apr 28, 2025
23 of 24 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 28, 2025
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025
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.

4 participants