Skip to content

Conversation

@vcsjones
Copy link
Member

@vcsjones vcsjones commented Aug 1, 2023

Accessing CNG properties is relatively slow to a cryptographic operation itself. This avoids eagerly reading the AlgorithmGroup property.

This fixes a performance regression introduced in #77809 for key operations that need to read KeySize.

PR is best reviewed ignoring whitespace.

Accessing CNG properties is (relatively) slow to a cryptographic operation itself. This avoids eagerly reading the AlgorithmGroup property.
@ghost ghost added the area-System.Security label Aug 1, 2023
@ghost ghost assigned vcsjones Aug 1, 2023
@ghost
Copy link

ghost commented Aug 1, 2023

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Accessing CNG properties is (relatively) slow to a cryptographic operation itself. This avoids eagerly reading the AlgorithmGroup property.

PR is best reviewed ignoring whitespace.

Author: vcsjones
Assignees: vcsjones
Labels:

area-System.Security

Milestone: -

@vcsjones
Copy link
Member Author

vcsjones commented Aug 1, 2023

In the interest of getting this PR in for .NET 8, I simply moved the logic around so that AlgorithmGroup is not accessed until we know 1. we don't have a key size and 2. we are in the PCP.

We may want to consider memoizing more CNG properties, which I opened #89821 to track.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

thanks

@vcsjones vcsjones merged commit 1686311 into dotnet:main Aug 2, 2023
@vcsjones vcsjones deleted the fix-cngkey-speed branch August 2, 2023 04:35
@ghost ghost locked as resolved and limited conversation to collaborators Sep 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants