-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Limit expansion in TemplateConstructableTypes
#117502
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
base: main
Are you sure you want to change the base?
Limit expansion in TemplateConstructableTypes
#117502
Conversation
Out of curiosity, will all of this be trimmed out if you don't actually use |
We generate this whenever method on a generic type/generic method is a target of reflection due to our rule that says This is also used to support generic virtual and interface methods. The other key to get rid of this data is not to use generic virtual methods. |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
bdaff9d
to
90a8217
Compare
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
The template type loader is a limited type loader that we use in native AOT to support scenarios such as `MakeGenericType`. We keep around extra MethodTables and metadata to be able to construct new type instantiations (e.g. `List<string>`) at runtime from template instantiations (e.g. `List<__Canon>`) we made at compile time. The template instantiation is a `MethodTable` like any other that we make a copy of and patch up with the help of the metadata ("native layout metadata"). Patching up involves e.g. building interface list (`List<string>` should have `IList<string>` in the interface list). This patching up may involve loading more new types from templates (e.g. the mentioned `IList<string>` that needs to be loadable from a `IList<__Canon>` template MethodTable). The job of the compiler is to figure out all the templates we might need (recursively) to build a type. This is done in places using a rather non-exact `TemplateConstructableTypes` call that just decomposes the type and makes templates for _everything_. Some of these might not be actually needed. This is an attempt to somewhat limit it.
Ever since GVM support was added to native AOT, we were generating the GVM resolution metadata for every type considered allocated. This included GVMs that were never even called (see `TypeGVMEntriesNode` that simply goes over everything on the type). This PR introduces tracking with method level granularity. I ran into this in a different PR where this was dragging `double`/`float` into compilation just because `int` implements generic math.
a6d587b
to
66ec273
Compare
The size savings for CsWinRT look not too bad! 👀 |
This reverts commit 66ec273.
This is currently multiple changes in one, I'm peeling some of it off into #118632 for reviewability, but it looks promising! |
The test failure looks weird. Is it related to the changes?
|
Looking at this change, it changes the policy of what the expansion analysis is doing. Before the change, if a type ended up being marked as something that had a template, it would force all the various bits and pieces of the type to be constructed, such that a MakeGenericType on it would succeed reliably. After the change, the policy is more that template construction will not trigger additional codegen. If it so happens that the needed code for the final application will not work, it is what it is. I'm not currently familiar enough with the code to determine if this will fail in an explainable fashion, but it is plausible that the failures that occur would be explained by the various annotations we have. |
I couldn't quickly figure out this fails only on Linux. The test is trim unsafe and based on what I'm seeing it shouldn't even work in main, but for some reason I guess it works. I'll need to debug it next week. From compiler logs the test should be failing in main already |
Ah, so the test does not succeed - neither in main nor here. We're taking the SkipTestException path and then hit some null ref. It didn't repro locally for me and a retry in the CI cleared it. Filed #118979 to try collect something from the infra to see if there's any history of this. |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #82384
The template type loader is a limited type loader that we use in native AOT to support scenarios such as
MakeGenericType
. We keep around extra MethodTables and metadata to be able to construct new type instantiations (e.g.List<string>
) at runtime from template instantiations (e.g.List<__Canon>
) we made at compile time.The template instantiation is a
MethodTable
like any other that we make a copy of and patch up with the help of the metadata ("native layout metadata"). Patching up involves e.g. building interface list (List<string>
should haveIList<string>
in the interface list). This patching up may involve loading more new types from templates (e.g. the mentionedIList<string>
that needs to be loadable from aIList<__Canon>
template MethodTable).The job of the compiler is to figure out all the templates we might need (recursively) to build a type. This is done in places using a rather non-exact
TemplateConstructableTypes
call that just decomposes the type and makes templates for everything. Some of these might not be actually needed.This is an attempt to somewhat limit it.
Cc @dotnet/ilc-contrib