KEMBAR78
avoid some allocations during TLS handshake by wfurt · Pull Request #97348 · dotnet/runtime · GitHub
Skip to content

avoid some allocations during TLS handshake #97348

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

Jump to bottom
Merged
merged 4 commits into from
Feb 12, 2024
Merged

avoid some allocations during TLS handshake #97348

merged 4 commits into from
Feb 12, 2024

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Jan 22, 2024

this is follow-up #87874 and it contributes to #68951

The trivial change is SslStreamPal.Windows.cs. Somehow during the re-factoring I failed to notice that the flag is not set true by default any longer as it was when I started ;(
I found that when running final tests @davidfowl asked me to.

While running that I also found one more opportunity: When we query RemoteCertificate we always create safe handle and if there is no certificate (common for servers) then we set the handle to Invalid. This change shuffles the PAL code a little bit so we allocate SafeFreeCertContext only if there is certificate to wrap.

With that the allocations looks like this for 100 rounds on server:

image

@wfurt wfurt added this to the 9.0.0 milestone Jan 22, 2024
@wfurt wfurt requested review from rzikm and stephentoub January 22, 2024 21:39
@wfurt wfurt self-assigned this Jan 22, 2024
@ghost
Copy link

ghost commented Jan 22, 2024

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

Issue Details

this is follow-up #87874 and it contributes to #68951

The trivial change is SslStreamPal.Windows.cs. Somehow during the re-factoring I failed to notice that the flag is not set true by default any longer as it was when I started ;(
I found that when running final tests @davidfowl asked me to.

While running that I also found one more opportunity: When we query RemoteCertificate we always create safe handle and if there is no certificate (common for servers) then we set the handle to Invalid. This change shuffles the PAL code a little bit so we allocate SafeFreeCertContext only if there is certificate to wrap.

With that the allocations looks like this for 100 rounds on server:

image

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Security
Milestone: 9.0.0

rzikm
rzikm approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

This was referenced Feb 9, 2024
Closed
Closed
@wfurt wfurt merged commit d64c653 into dotnet:main Feb 12, 2024
@wfurt wfurt deleted the handshakeAlloc branch February 12, 2024 18:09
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rzikm rzikm rzikm approved these changes

@stephentoub stephentoub Awaiting requested review from stephentoub

Assignees

@wfurt wfurt

Labels

area-System.Net.Security

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@wfurt @stephentoub @rzikm