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.

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

System.Net.Quic is not trimmed on mobile platforms #46915

Closed
Tracked by #45832
marek-safar opened this issue Jan 13, 2021 · 11 comments · Fixed by #49261
Closed
Tracked by #45832

System.Net.Quic is not trimmed on mobile platforms #46915

marek-safar opened this issue Jan 13, 2021 · 11 comments · Fixed by #49261
Labels
area-System.Net.Quic bug size-reduction Issues impacting final app size primary for size sensitive workloads
Milestone
6.0.0

Comments

@marek-safar
Copy link
Contributor

marek-safar commented Jan 13, 2021 •
edited
Loading

System.Net.Quic is not supported except on Windows but still cannot be trimmed because it's initialized aggressively via static constructors even on platforms that don't support it like iOS or android. Initialization chain is like

	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.ctor
	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.cctor
	  at System.Net.Quic.Implementations.MsQuic.MsQuicImplementationProvider:get_IsSupported
	  at System.Net.Http.HttpConnectionPool:.ctor
	  at System.Net.Http.HttpConnectionPoolManager:SendAsyncCore
	  at System.Net.Http.HttpConnectionPoolManager:SendAsync
	  at System.Net.Http.HttpConnectionHandler:SendAsync

There is IsSupported property but it does not conform to the recommended Feature switch setup so it cannot be used either.

@eerhardt @karelz
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net untriaged New issue has not been triaged by the area owner labels Jan 13, 2021
@ghost
Copy link

ghost commented Jan 13, 2021

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

Issue Details

System.Net.Quic is not supported except on Windows but still cannot be trimmed because it's initialized aggressively via static constructors even on platforms that don't support it like iOS or android. Initialization chain is like

	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.ctor
	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.cctor
	  at System.Net.Quic.Implementations.MsQuic.MsQuicImplementationProvider:get_IsSupported
	  at System.Net.Http.HttpConnectionPool:.ctor
	  at System.Net.Http.HttpConnectionPoolManager:SendAsyncCore
	  at System.Net.Http.HttpConnectionPoolManager:SendAsync
	  at System.Net.Http.HttpConnectionHandler:SendAsync

There is IsSupported property but it does not conform to the recommended Feature switch setup so it cannot be used either.

@eerhardt

Author: marek-safar
Assignees: -
Labels:

area-System.Net, untriaged
Milestone: -

@ghost
Copy link

ghost commented Jan 13, 2021

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

Issue Details

System.Net.Quic is not supported except on Windows but still cannot be trimmed because it's initialized aggressively via static constructors even on platforms that don't support it like iOS or android. Initialization chain is like

	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.ctor
	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.cctor
	  at System.Net.Quic.Implementations.MsQuic.MsQuicImplementationProvider:get_IsSupported
	  at System.Net.Http.HttpConnectionPool:.ctor
	  at System.Net.Http.HttpConnectionPoolManager:SendAsyncCore
	  at System.Net.Http.HttpConnectionPoolManager:SendAsync
	  at System.Net.Http.HttpConnectionHandler:SendAsync

There is IsSupported property but it does not conform to the recommended Feature switch setup so it cannot be used either.

@eerhardt

Author: marek-safar
Assignees: -
Labels:

area-System.Net.Quic, untriaged
Milestone: -

@marek-safar marek-safar added the size-reduction Issues impacting final app size primary for size sensitive workloads label Jan 13, 2021
@ghost
Copy link

ghost commented Jan 13, 2021

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

System.Net.Quic is not supported except on Windows but still cannot be trimmed because it's initialized aggressively via static constructors even on platforms that don't support it like iOS or android. Initialization chain is like

	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.ctor
	  at System.Net.Quic.Implementations.MsQuic.Internal.MsQuicApi:.cctor
	  at System.Net.Quic.Implementations.MsQuic.MsQuicImplementationProvider:get_IsSupported
	  at System.Net.Http.HttpConnectionPool:.ctor
	  at System.Net.Http.HttpConnectionPoolManager:SendAsyncCore
	  at System.Net.Http.HttpConnectionPoolManager:SendAsync
	  at System.Net.Http.HttpConnectionHandler:SendAsync

There is IsSupported property but it does not conform to the recommended Feature switch setup so it cannot be used either.

@eerhardt @karelz

Author: marek-safar
Assignees: -
Labels:

area-System.Net.Quic, size-reduction, untriaged
Milestone: -

@marek-safar marek-safar mentioned this issue Jan 13, 2021
Closed
11 tasks
@karelz karelz added the bug label Jan 13, 2021
@karelz karelz added this to the 6.0.0 milestone Jan 13, 2021
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Jan 13, 2021
@geoffkizer
Copy link
Contributor

What needs to happen to fix this?

Note, S.N.Quic is also supported on Linux and eventually on MacOS as well.

@eerhardt
Copy link
Member

What needs to happen to fix this?

I think the initial thinking is to change MsQuicImplementationProvider:get_IsSupported to conform to a feature-switch. See other implementations for reference like

The idea being that vertical app models that want to disable/trim certain features can set these feature switches in their MSBuild Sdk files. So here Xamarin can set System.Net.Quic.IsSupported=false and all the Quic implementation will get trimmed away by the linker in a Xamarin app.

@geoffkizer
Copy link
Contributor

The MsQuicImplementationProvider:get_IsSupported API isn't intended to ship; it's just a temporary measure while we have multiple providers that we are developing/testing with.

I suspect what we want is something like a single public static bool IsSupported on some top-level QUIC class (maybe QuicConnection? not sure).

@marek-safar
Copy link
Contributor Author

I suspect what we want is something like a single public static bool IsSupported on some top-level QUIC class

Ideally, the code would be pulled only when needed. For example when http version is set to 3 or higher. If that's not possible then we resort to static property like that but that has a tax on developers to know about it. We could also PNSE the code on the platforms where the code won't be supported anytime soon (e.g. Browsers).

Note, S.N.Quic is also supported on Linux and eventually on MacOS as well.

These are only a few platforms we support.

@geoffkizer
Copy link
Contributor

Note, S.N.Quic is also supported on Linux and eventually on MacOS as well.

These are only a few platforms we support.

I mentioned that because the original issue description above said

System.Net.Quic is not supported except on Windows

@geoffkizer
Copy link
Contributor

geoffkizer commented Jan 14, 2021 •
edited
Loading

Ideally, the code would be pulled only when needed. For example when http version is set to 3 or higher.

This makes sense. We are only calling into System.Net.Quic from the HttpConnectionPool ctor in order to check whether it's supported or not. We can easily defer this check.

@SamMonoRT
Copy link
Member

@eerhardt - trying to find the right owner for this. Is that someone on your team ?

@marek-safar
Copy link
Contributor Author

I changed System.Net.Quick to be PNSE assembly so this should no longer be a concern for mobile

@marek-safar marek-safar modified the milestones: 6.0.0, Future Mar 2, 2021
@marek-safar marek-safar mentioned this issue Mar 9, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 9, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 15, 2021
@karelz karelz removed this from the Future milestone May 20, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Net.Quic bug size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Don't call into unsupported APIs on build targets which don't support quic marek-safar/runtime
5 participants
@marek-safar @karelz @eerhardt @geoffkizer @SamMonoRT