Generic virtual method analysis tweaks #493
Conversation
Two tweaks: * Do not go into the SearchDynamicDependencies codepath if the method cannot be overriden. I believe the SearchDynamicDependencies wouldn't come up with anything. This mostly helps generic virtual method use in LINQ. Doesn't help much in Avalonia. * Don't do a while loop over the inheritance chain. I believe the dependencies on less derived methods will be injected as part of those GVMDependencies nodes so this feels unnecessary.
MichalStrehovsky
added
area-NativeAOT-coreclr
MichalStrehovsky
removed
the
* NO MERGE *
|
/azp run runtimelab-nativeaot Pri-0 Tests |
|
Azure Pipelines failed to run 1 pipeline(s). |
|
So this drops the compilation time of the Avalonia demo app in #452 from ~12 minutes to:
Hopefully I didn't break anything, |
Thanks for nothing, Azure pipelines. Great failure logs! |
|
/azp run runtimelab-nativeaot Pri-0 Tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -114,10 +121,10 @@ public override bool HasDynamicDependencies | |||
|
|
|||
| public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) | |||
| { | |||
| Debug.Assert(_method.IsVirtual && _method.HasInstantiation); | |||
There was a problem hiding this comment.
Deleting this because we assert this in the ctor.
| @@ -130,13 +137,11 @@ public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependenci | |||
| if (!potentialOverrideType.IsDefType) | |||
| continue; | |||
|
|
|||
| Debug.Assert(!potentialOverrideType.IsRuntimeDeterminedSubtype); | |||
There was a problem hiding this comment.
Deleting this because IsRuntimeDeterminedSubtype is expensive and getting one here would mean we already have a bad EEType in the system. EETypeNode asserts this is not the case.
| @@ -186,23 +191,10 @@ public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependenci | |||
| continue; | |||
| } | |||
|
|
|||
| overrideTypeCur = potentialOverrideType; | |||
There was a problem hiding this comment.
Couldn't figure out when this loop would have a meaningful impact. We could consider not checking in this part of the change and keep the loop, but I don't like to have code around that nobody understands and we have no test coverage for (there's apparently no failing test with this deleted even though we test generic virtual methods in the Generics test and in the DynamicGenerics test).
Three tweaks: