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

When is AOT + MakeGenericType safe? #71625

Closed
JamesNK opened this issue Jul 3, 2022 · 5 comments
Closed

When is AOT + MakeGenericType safe? #71625

JamesNK opened this issue Jul 3, 2022 · 5 comments
Labels
area-NativeAOT-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@JamesNK
Copy link
Member

JamesNK commented Jul 3, 2022

MakeGenericType generates warnings when PublishAot is enabled. I took at look at dotnet/runtime and there are some places where the IL3050 warning from MakeGenericType is suppressed.

What are the rules around MakeGenericType? When is it safe to have it in a framework or library? And what happens if tries to create a type that it can't?

@MichalStrehovsky MichalStrehovsky transferred this issue from dotnet/linker Jul 5, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 5, 2022
@ghost
Copy link

ghost commented Jul 5, 2022

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

Issue Details

MakeGenericType generics warnings from some assemblies when PublishAot is enabled. I took at look at dotnet/runtime and there are some places where the IL3050 warning from MakeGenericType is suppressed.

What are the rules around MakeGenericType? When is it safe to have it in a framework or library? And what happens if tries to create a type that it can't?

Author: JamesNK
Assignees: -
Labels:

area-Meta, untriaged

Milestone: -

@MichalStrehovsky MichalStrehovsky added area-NativeAOT-coreclr and removed area-Meta untriaged New issue has not been triaged by the area owner labels Jul 5, 2022
@MichalStrehovsky
Copy link
Member

The rule is basically "the native code for the generic instantiation has to be present".

I don't know if there's a more succinct version of this documented somewhere - I had a twitter thread on how generic code is done in .NET that kind of explains what the problem is here: https://twitter.com/MStrehovsky/status/1244602445461950470.

We generally suppress the warnings within CoreLib in places where the compiler is doing work to ensure the native code will be available at runtime. That's not very useful for things outside CoreLib.

As an implementation detail (that has been true since .NET 2.0 and is unlikely to change), code for instantiations over reference types is fully shareable. So:

  1. If we can make sure this fully shareable code is generated,
  2. and the arguments to MakeGeneric* are all reference types (arrays, classes, delegates, interfaces)

...the warning can be suppressed as well.

Fully shareable code generation can be triggered in three ways, basically:

  • Make it obvious to the dataflow analysis within the compiler: typeof(Gen<>).MakeGenericType(anything) - this will trigger shared code generation for Gen<> because it's immediately visible we are using it with MakeGeneric. Same for typeof(Foo).GetMethod("SomeMethod").MakeGenericMethod(anthing) - Foo.SomeMethod<> will get shared code.
  • DynamicDependencyAttribute.
  • Root it from ILLink.Descriptors.xml, or trimming-root the whole assembly that contains the type with TrimmerRootAssembly. This will trigger a heuristic that generates the shared code since for Native AOT, the type is not particularly useful if it's not instantiated somehow. Not particularly desirable. Avoid.

If you don't have the shared code available at runtime, there will be an exception at the time of MakeGeneric. For example:

typeof(Foo<>).MakeGenericType(typeof(Bar));

class Foo<T> { }
struct Bar { }

At runtime:

Unhandled Exception: System.Reflection.MissingMetadataException: 'Foo<Bar>' is missing metadata. For more information, please visit http://go.microsoft.com/fwlink/?LinkID=392859
   at System.Reflection.Runtime.General.TypeUnifier.WithVerifiedTypeHandle(RuntimeConstructedGenericTypeInfo, RuntimeTypeInfo[]) + 0x1ab
   at System.Reflection.Runtime.General.TypeUnifier.GetConstructedGenericTypeWithTypeHandle(RuntimeTypeInfo, RuntimeTypeInfo[]) + 0x3f
   at System.Reflection.Runtime.TypeInfos.RuntimeTypeInfo.MakeGenericType(Type[]) + 0x4d1
   at Program.<Main>$(String[]) + 0xa3

(The exception type and string is expected to change in .NET 7. Probably to PlatformNotSupportedException. The existing message dates back to .NET Native for UWP apps.)

@MichalStrehovsky
Copy link
Member

One more implementation detail that we don't intend to change:

using System;

new Foo<Bar>();
typeof(Foo<>).MakeGenericType(typeof(Bar));
Console.WriteLine("It worked!");
typeof(Foo<>).MakeGenericType(typeof(Baz));

class Foo<T> { }
struct Bar { }
struct Baz { }

This will print "It worked" and then crash with the unhandled exception.

This is because we made native code for Foo<Bar> statically reachable and it was available to satisfy the MakeGeneric call.

So if there's an upper bound on the list of structs, one can also suppress if it's used with structs and the static code is made sure to be available. It will generally not work well if the structs are user-controlled.

@JamesNK
Copy link
Member Author

JamesNK commented Jul 5, 2022

Thanks, this is good info.

All the cases I've been looking at include a constant for the generic definition (i.e. typeof(Foo<>).MakeGenericType(typeof(Bar));). I think only one of them could have a non-reference argument.

Kind of related follow up question: Do you know how optimized Microsoft.Extensions.DependencyInjection is when it comes to open generics?

services.TryAddSingleton(typeof(ServerCallHandlerFactory<>));
services.TryAddSingleton(typeof(IGrpcServiceActivator<>), typeof(DefaultGrpcServiceActivator<>));
services.TryAddSingleton(typeof(IGrpcInterceptorActivator<>), typeof(DefaultGrpcInterceptorActivator<>));

The argument type is always a class in these examples. Is DI generating optimal code?

@MichalStrehovsky
Copy link
Member

Kind of related follow up question: Do you know how optimized Microsoft.Extensions.DependencyInjection is when it comes to open generics?

Do you mean in the context of NativeAOT? CoreCLR-NativeAOT uses the same code generator as CoreCLR-JIT - RyuJIT. So the generated code will be the same. CoreCLR-JIT can do PGO-driven dynamic recompilation so it has a chance to produce better code in some cases, but I don't know how big of a difference it would make here. On the other hand, NativeAOT has a whole program view so it can devirtualize all interface calls to their actual targets if there's only one type implementing the interface within the app (we don't do that yet but there's a WIP pull request).

@MichalStrehovsky MichalStrehovsky added the question Answer questions and provide assistance, not an issue with source code or documentation. label Jul 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

2 participants