Skip to content

Reduce allocations in PENamedTypeSymbol.GetMemberTypesPrivate#81605

Merged
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
nareshjo:dev/nareshjo/GetMemberTypesPrivateAlloc
Dec 11, 2025
Merged

Reduce allocations in PENamedTypeSymbol.GetMemberTypesPrivate#81605
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
nareshjo:dev/nareshjo/GetMemberTypesPrivateAlloc

Conversation

@nareshjo
Copy link
Contributor

@nareshjo nareshjo commented Dec 9, 2025

This pull request was generated by the VS Perf Rel AI Agent. Please review this AI-generated PR with extra care! For more information, visit our wiki. Please share feedback with TIP Insights

  • Issue: PENamedTypeSymbol.GetMemberTypesPrivate creates ArrayBuilder without capacity, causing repeated array resize allocations during AddRange operations. ETW traces show this method as an allocation hotspot.
  • Issue type: DO specify an up-front capacity if one is known.
  • Proposed fix: Pre-calculate total capacity by summing nested type array lengths before creating ArrayBuilder. This eliminates multiple O(n) array resize allocations by performing a single O(n) pass upfront. The same dictionary values are enumerated again immediately afterward during AddRange, so the capacity calculation adds negligible overhead compared to array resizes.

Best practices wiki
See related failure in PRISM
ADO work item

@nareshjo nareshjo requested a review from a team as a code owner December 9, 2025 17:55
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Dec 9, 2025
Comment on lines +1798 to +1804
int capacity = 0;
foreach (var typeArray in _lazyNestedTypes.Values)
{
capacity += typeArray.Length;
}

var builder = ArrayBuilder<NamedTypeSymbol>.GetInstance(capacity);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int capacity = 0;
foreach (var typeArray in _lazyNestedTypes.Values)
{
capacity += typeArray.Length;
}
var builder = ArrayBuilder<NamedTypeSymbol>.GetInstance(capacity);
var builder = ArrayBuilder<NamedTypeSymbol>.GetInstance(_lazyNestedTypes.Values.Sum(static a => a.Length));

Copy link
Contributor

@ToddGrun ToddGrun Dec 9, 2025

Choose a reason for hiding this comment

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

If we are doing the work to determine the exact capacity up front, then this would be better off just creating the array explicitly, rather than using an ArrayBuilder and hitting the pool, something like:

private ImmutableArray<NamedTypeSymbol> GetMemberTypesPrivate()
{
    var count = _lazyNestedTypes.Values.Sum(static a => a.Length);
    var result = new PENamedTypeSymbol[count];
    var destIndex = 0;

    foreach (var typeArray in _lazyNestedTypes.Values)
    {
        typeArray.CopyTo(result, destIndex);

        destIndex += typeArray.Length;
    }

    return ImmutableCollectionsMarshal.AsImmutableArray<NamedTypeSymbol>(result);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use explicit array.

@CyrusNajmabadi
Copy link
Contributor

ETW traces show this method as an allocation hotspot. Issue type: DO specify an up-front capacity if one is known.

Could you link me to those ETW traces. You can read me internally if needed.

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi
Copy link
Contributor

Thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit fe39a92 into dotnet:main Dec 11, 2025
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 11, 2025
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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants