Generate type generic composition information as constructed #105133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
This was referenced Jul 19, 2024
Build failure: Static graph-based restore failed with exit code .* but did not log an error.
#103526
Open
MichalStrehovsky
changed the title
Size statisticsPull request #105133
|
This was referenced Jul 25, 2024
Closed
Hmm, maybe we can restrict it to types that have DAM-annotations somewhere on the generic parameters? It still fixes the DI suppression case and I can't think of other holes right now. The extra code doesn't look too terrible and I'll run rt-sz to check how much that improves things. |
|
Looks better like this and the code is not much more complicated: Size statisticsPull request #105133
|
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkotas
approved these changes
Jul 26, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #105034.
Before this PR, whenever we had a generic instance
Foo<SomeType, OtherType<SomeOtherType>>and we had a constructedMethodTablefor this (i.e.MethodTablethat is suitable to be used for types on the GC heap), we'd still try to optimize out theMethodTables forSomeType,OtherType<SomeOtherType>into the "unconstructed" form (i.e. without a GCDesc or vtable).This is generally a safe optimization - if
Foo<,>actually had atypeof(T)somewhere, we'd upgrade the unconstructed forms into constructed forms, for example.One place where the unconstructed types can be obtained is when reflection-inspecting such instantiation - asking for
GetGenericArgumentsof a type could produce things that have unconstructedMethodTables in them. In general, this is also fine - one can't do much reflection with these and asking for e.g. names would still work fine. Trying to access members or activating something obtained fromGetGenericArgumentswould trigger trimming warnings, so that's also fine.Enter Microsoft.Extension.DependencyInjection and a trimming warning suppression in there.
DI has support for something called
IOption<T>where the T is annotated with DAM attributes. Then there's another type (OptionFactory<T>? Can't be bothered to look at that again.) that has the T annotated the same way. Then there's some code that will deconstruct theIOptioninstance that we saw statically, andMakeGenericTypeanOptionFactorywith its generic arguments. The warnings on MakeGenericType are suppressed on account of: T is annotated the same, and T is not a valuetype.The suppression sounds reasonable. It just doesn't hold right now, since nothing accesses the T in IOption and we're therefore left with the unconstructed form of the T. The OptionFactory though accesses the T and tries to activate it.
This is especially a problem if IOption is instantiated over a generic type that is instantiated over a valuetype. We know we need to keep the ctor due to the annotation, but not on unconstructed forms of the type.
I was struggling with how to fix this. The easiest fix is to just stop optimizing things into unconstructed forms. This is a size regression.
Cc @dotnet/ilc-contrib