Avoid creating identity to obtain public key #78255
Conversation
Going to start in draft mode and will promote if a test insertion shows value. Will update with performance justitications at that point.
ghost
added
Area-Compilers
untriaged
ToddGrun
changed the title
|
@dotnet/roslyn-compiler -- This is ready for review |
|
|
||
| // Avoid using the identity to obtain the public key if possible to avoid the allocations associated | ||
| // with identity creation | ||
| ImmutableArray<byte> publicKey = (assemblyWantingAccess is AssemblySymbol assemblyWantingAccessAssemblySymbol) |
There was a problem hiding this comment.
Yep, good catch. Done in commit 2.
|
Done with review pass (commit 1) |
|
@AlekseyTs -- Question for you. There remains about 50 MB (1%) of allocations from calls to get_Identity in the after image above where AssemblySymbolKey.Resolve is using the Identity to get the Name. I notice there is also a Name property on ISymbol. Is ISymbol.Name in that context the same as the IAssemblySymbol.Identity.Name value? It appears to be from the debugging I've done, and if so, I'd like to go ahead and change that too. |
| ' Avoid using the identity to obtain the public key if possible to avoid the allocations associated | ||
| ' with identity creation | ||
| Dim publicKey As ImmutableArray(Of Byte) | ||
| If TypeOf assemblyWantingAccess Is AssemblySymbol Then |
There was a problem hiding this comment.
There was a problem hiding this comment.
Commit 3. TryCast is the VB call that I tend to forget about.
|
Done with review pass (commit 2) |
Honestly, I do not know the answer to this question. To be comfortable with a change like that, I think a more thorough investigation should be done (an examination of all possible ways the property and the assembly identity get their values, etc.). We would also like to place asserts in certain places enforcing the invariant (assuming we will prove there is an invariant today) going forward. |
|
@dotnet/roslyn-compiler for 2nd review. Thanks! |
4 participants
Reduce allocations in AssemblySymbol.GivesAccessTo due to getting the assemblyWantingAccess's Identity.
During the completion scenarios in the C# speedometer tests, about 3% of allocations in the VS process are used obtaining this identity just to get the PublicKey. However, in the case where assemblyWantingAccess is an AssemblySymbol, there is a way to obtain this without needing to obtain the full Identity.
*** Previous allocations ***
*** Allocations with this change ***
To demonstrate that these Identity calls weren't just calculated at a later point, below are the allocations due to calls to get_Identity before/after:
*** Previous allocations in get_Identity ***
*** Allocations in get_Identity with this change ***
