[Performance] Reduce allocations in PENamedTypeSymbol.EnsureTypeParametersAreLoaded
#70766
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
ghost
added
the
Community
| var moduleSymbol = ContainingPEModule; | ||
|
|
||
| // If this is a nested type generic parameters in metadata include generic parameters of the outer types. | ||
| int firstIndex = _genericParameterHandles.Count - _arity; | ||
|
|
||
| TypeParameterSymbol[] ownedParams = new TypeParameterSymbol[_arity]; |
There was a problem hiding this comment.
@333fred avoiding this unnecessary allocation on every type does seem like a nice to have.
| @@ -2658,19 +2658,26 @@ private void EnsureTypeParametersAreLoaded() | |||
| { | |||
| if (_lazyTypeParameters.IsDefault) | |||
| { | |||
| if (_arity == 0) | |||
| { | |||
| _lazyTypeParameters = ImmutableArray<TypeParameterSymbol>.Empty; | |||
There was a problem hiding this comment.
As an alternative, we could check arity and assign this way inside the constructor. There is a guarantee that there is no race with any other thread there,
There was a problem hiding this comment.
I would prefer that approach myself.
There was a problem hiding this comment.
I would prefer that approach myself.
I assume "that" refers to the second comment? 2456a06
|
Done with review pass (commit 1) |
The interlocked API is also used to make the assignment atomic. In that sense, it really doesn't matter whether we are trying to assign the same instance underneath or not. What we are trying to avoid as well is partial assignment. |
|
@Youssef1313 Thank you for the contribution. |
ImmutableArray.Create(element1)andImmutableArray.Create(element1, element2)respectively, which would be even more efficient than ArrayBuilder. I'm not sure though if it's worth doing it that way.