-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Default Http3Support to false for PublishTrimmed #117655
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR disables HTTP/3 support by default in Native AOT builds to reduce application size and avoid unsupported configurations.
- Adds an MSBuild property
Http3Support
defaulting tofalse
- Inserts a runtime host configuration option to turn off HTTP/3 in
SocketsHttpHandler
Comments suppressed due to low confidence (2)
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets:42
- Add a brief comment above this property explaining why HTTP/3 support is disabled by default in Native AOT (e.g., to save size and avoid unsupported scenarios) for future maintainers.
<Http3Support Condition="'$(Http3Support)' == ''">false</Http3Support>
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets:51
- Consider adding a unit or integration test to verify that HTTP/3 support is indeed disabled by default for Native AOT applications and that enabling
Http3Support
toggles this behavior as expected.
<RuntimeHostConfigurationOption Include="System.Net.SocketsHttpHandler.Http3Support"
Nit: We rely on Microsoft-provided distro-specific msquic binary. You have to add Microsoft Linux package feed for given distro and install msquic from there: https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/quic/quic-overview#platform-dependencies . |
This makes sense to me. I think we may want to go even further and disabled it by default for all trimmed apps, not just NAOT.
I think so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm perfectly fine with disabling this on native AOT.
Will we follow this up to update Kestrel to also trim out its HTTP/3 support by default when trimming? I'm wondering if we need a more general name than "System.Net.SocketsHttpHandler.Http3Support". |
Any opinions about disabling this in trimmed apps in general like Jan suggested?
This is currently doc'd (https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options) as specific to System.Net.Http. i don't know what size savings we would be talking about for Kestrel and whether we'd ever need to control it independently. Cc @davidfowl for opinion. |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Is trimming automatic when you just specify RID when publishing (or any other way it's on automatically without user input)? If so, than I'm rather against as it would be too easy for the customers to unintentionally turn off H/3. |
Trimming only happens (for desktop platforms) when PublishTrimmed or PublishAot is set to true. Mobile platforms trim by default, but I don't think we have an msquic story there anyway. |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Then I'm fine with "disabling this in trimmed apps in general" as well. |
Okay, pushed out a commit that moves this default under PublishTrimmed. Does the test update look reasonable too? |
|
/ba-g helix deadletters on wasm |
EDIT: per discussion below, disabling for PublishTrimmed in general, not just PublishAot.
Since HTTP3 doesn't work with native AOT (or single file for that matter) without extra gestures outside Linux (#73290), we should probably require an extra gesture to include the code too. The extra code is sizeable - I've seen 15% savings from turning Http3Support off.
There are multiple ways to go about this:
1 is a possibility. People could discover this switch in docs. In practice, very few people will discover it.
I think 2 is an option because the delivery mechanism for QUIC outside Linux is our own build of msquic.dll (AFAIK) that we don't include in native AOT apps and user needs to manually copy. AFAIK on Linux we rely on an OS-provided msquic. I could be wrong. If I'm wrong, we can just cross out option 2.
3 is nice and consistent. We'll need to consider marking this a breaking change (does this fit definition of a breaking change?)
Cc @dotnet/ncl for throughts/review.