Skip to content

Avoid creating identity to obtain public key #78255

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 24, 2025
Merged

Avoid creating identity to obtain public key #78255

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f5847b
*** WIP: Avoid creating identity to obtain public key
ToddGrun Apr 22, 2025
ee469da
VB version of optimization
ToddGrun Apr 23, 2025
5d3142f
Less clunky VB code
ToddGrun Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/PublicModel/AssemblySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,15 @@ bool IAssemblySymbol.GivesAccessTo(IAssemblySymbol assemblyWantingAccess)

AssemblyIdentity identity = UnderlyingAssemblySymbol.Identity;

// 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)
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

publicKey

I think we should make a similar change for VB implementation as well #Closed

Copy link
Contributor Author

@ToddGrun ToddGrun Apr 23, 2025

Choose a reason for hiding this comment

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

Yep, good catch. Done in commit 2.

? assemblyWantingAccessAssemblySymbol.UnderlyingAssemblySymbol.PublicKey.NullToEmpty()
: assemblyWantingAccess.Identity.PublicKey;

foreach (var key in myKeys)
{
IVTConclusion conclusion = identity.PerformIVTCheck(assemblyWantingAccess.Identity.PublicKey, key);
IVTConclusion conclusion = identity.PerformIVTCheck(publicKey, key);
Debug.Assert(conclusion != IVTConclusion.NoRelationshipClaimed);
if (conclusion == IVTConclusion.Match || conclusion == IVTConclusion.OneSignedOneNot)
{
Expand Down
12 changes: 11 additions & 1 deletion src/Compilers/VisualBasic/Portable/Symbols/AssemblySymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Return True
End If

' 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
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If TypeOf assemblyWantingAccess Is AssemblySymbol Then

I think doing the following instead would be more efficient:

Dim assemblyWantingAccessAssemblySymbol As AssemblySymbol = TryCast(assemblyWantingAccess, AssemblySymbol)
If assemblyWantingAccessAssemblySymbol IsNot Nothing Then

#Closed

Copy link
Contributor Author

@ToddGrun ToddGrun Apr 23, 2025

Choose a reason for hiding this comment

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

Commit 3. TryCast is the VB call that I tend to forget about.

Dim assemblyWantingAccessAssemblySymbol As AssemblySymbol = DirectCast(assemblyWantingAccess, AssemblySymbol)
publicKey = assemblyWantingAccessAssemblySymbol.PublicKey.NullToEmpty()
Else
publicKey = assemblyWantingAccess.Identity.PublicKey
End If

For Each key In myKeys
Dim conclusion As IVTConclusion = Me.Identity.PerformIVTCheck(assemblyWantingAccess.Identity.PublicKey, key)
Dim conclusion As IVTConclusion = Me.Identity.PerformIVTCheck(publicKey, key)
Debug.Assert(conclusion <> IVTConclusion.NoRelationshipClaimed)
If conclusion = IVTConclusion.Match Then
' Note that C# includes OrElse conclusion = IVTConclusion.OneSignedOneNot
Expand Down