Skip to content
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.

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

[release/7.0] Unload MsQuic after checking for QUIC support to free resources. #74931

Closed
wants to merge 3 commits into from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 1, 2022

Backport of #74749 to release/7.0, closes #74629.

/cc @rzikm

Customer Impact

When using HttpClient, we also check whether the running platform supports QUIC (to enable HTTP3). However, the way we are checking QUIC support causes many threads to be allocated in the native MsQuic library (2* number of logical cores). This causes unnecessary resource increase even if the process does not end up using HTTP3 at all (HTTP3 is opt-in). This would therefore cause regression in memory usage when upgrading to .NET 7 in such cases.

Affected platforms include Windows 11, Windos Server 2022, many Linux platforms with msquic package installed.

Testing

Functional tests suite passes as part of the CI, resource consumption was checked manually.

Risk

Low, the fix consists of gracefully unloading MsQuic library from the process after checking QUIC support. The library is reloaded only when actually needed.

@ghost
Copy link

ghost commented Sep 1, 2022

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

Issue Details

Backport of #74749 to release/7.0

/cc @rzikm

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net

Milestone: -

@rzikm rzikm requested review from stephentoub and a team September 1, 2022 14:55
@rzikm rzikm linked an issue Sep 1, 2022 that may be closed by this pull request
@carlossanlop
Copy link
Member

The CI was down yesterday for maintenance. Only 5 checks were executed while that happened. I'm closing and reopening this PR so the full CI runs.

@carlossanlop carlossanlop reopened this Sep 1, 2022
@rzikm
Copy link
Member

rzikm commented Sep 1, 2022

Putting this on hold until we investigate #74952

@rzikm rzikm marked this pull request as draft September 1, 2022 18:52
@carlossanlop carlossanlop added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 2, 2022
@rzikm
Copy link
Member

rzikm commented Sep 5, 2022

There is a problem which manifests on Debian11+Docker+ARM64 with this change, we would need changes in MsQuic as well, so I am closing this for now.

@rzikm rzikm closed this Sep 5, 2022
@jkotas jkotas deleted the backport/pr-74749-to-release/7.0 branch September 13, 2022 02:04
@ghost ghost locked as resolved and limited conversation to collaborators Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpClient creates Quic threads even if HTTP/3 is not in use
3 participants