From e542fa98ccf1482fe456a31640a3fe7f438e437d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Sat, 29 Aug 2026 10:39:10 -0400 Subject: [PATCH 1/2] Avoid boxing dependency list enumerators Enumerate known concrete dependency lists and arrays directly while preserving custom enumerable and mutation semantics. Add focused coverage for collection shapes, ordering, conditional dependencies, and mutation. --- .../DependencyAnalyzerTests.cs | 402 ++++++++++++++++++ .../ILCompiler.Compiler.Tests.csproj | 1 + .../DependencyAnalyzer.cs | 79 +++- 3 files changed, 466 insertions(+), 16 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs new file mode 100644 index 00000000000000..1c150d3b4d5515 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs @@ -0,0 +1,402 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; + +using ILCompiler.DependencyAnalysisFramework; + +using Xunit; + +using CombinedDependencyListEntry = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.CombinedDependencyListEntry; +using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.DependencyList; +using DependencyListEntry = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.DependencyListEntry; + +namespace ILCompiler.Compiler.Tests +{ + public class DependencyAnalyzerTests + { + public enum DependencyCollectionKind + { + Array, + DependencyList, + Enumerable, + List, + ReimplementedList, + } + + public static IEnumerable StaticDependencyData() + { + DependencyCollectionKind[] collectionKinds = + [ + DependencyCollectionKind.Array, + DependencyCollectionKind.DependencyList, + DependencyCollectionKind.Enumerable, + DependencyCollectionKind.List, + DependencyCollectionKind.ReimplementedList, + ]; + + return CreateDependencyData(collectionKinds); + } + + public static IEnumerable ConditionalDependencyData() + { + DependencyCollectionKind[] collectionKinds = + [ + DependencyCollectionKind.Array, + DependencyCollectionKind.Enumerable, + DependencyCollectionKind.List, + DependencyCollectionKind.ReimplementedList, + ]; + + return CreateDependencyData(collectionKinds); + } + + public static IEnumerable ConditionalDependencyCollectionKinds() + { + yield return new object[] { DependencyCollectionKind.Array }; + yield return new object[] { DependencyCollectionKind.Enumerable }; + yield return new object[] { DependencyCollectionKind.List }; + yield return new object[] { DependencyCollectionKind.ReimplementedList }; + } + + public static IEnumerable MutableStaticDependencyLists() + { + yield return new object[] { DependencyCollectionKind.DependencyList }; + yield return new object[] { DependencyCollectionKind.List }; + } + + [Theory] + [MemberData(nameof(StaticDependencyData))] + public void StaticDependenciesPreserveOrder(DependencyCollectionKind collectionKind, int count) + { + TestNode[] dependencyNodes = CreateNodes("dependency", count); + DependencyListEntry[] entries = CreateStaticEntries(dependencyNodes); + var root = new TestNode("root") + { + StaticDependencies = CreateStaticDependencies(collectionKind, entries), + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + + analyzer.AddRoot(root, "root"); + analyzer.ComputeMarkedNodes(); + + DependencyNodeCore[] expected = new DependencyNodeCore[count + 1]; + expected[0] = root; + CopyExpectedNodes(expected, 1, dependencyNodes, collectionKind); + Assert.Equal(expected, analyzer.MarkedNodeList); + } + + [Theory] + [MemberData(nameof(ConditionalDependencyData))] + public void ConditionalDependenciesPreserveOrder(DependencyCollectionKind collectionKind, int count) + { + var condition = new TestNode("condition"); + TestNode[] dependencyNodes = CreateNodes("dependency", count); + CombinedDependencyListEntry[] entries = CreateConditionalEntries(dependencyNodes, condition); + var root = new TestNode("root") + { + ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + + analyzer.AddRoot(condition, "condition"); + analyzer.AddRoot(root, "root"); + analyzer.ComputeMarkedNodes(); + + DependencyNodeCore[] expected = new DependencyNodeCore[count + 2]; + expected[0] = condition; + expected[1] = root; + CopyExpectedNodes(expected, 2, dependencyNodes, collectionKind); + Assert.Equal(expected, analyzer.MarkedNodeList); + } + + [Theory] + [MemberData(nameof(ConditionalDependencyCollectionKinds))] + public void NullConditionalDependencyIsUnconditional(DependencyCollectionKind collectionKind) + { + var dependency = new TestNode("dependency"); + CombinedDependencyListEntry[] entries = + [ + new CombinedDependencyListEntry(dependency, null, "unconditional"), + ]; + var root = new TestNode("root") + { + ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + + analyzer.AddRoot(root, "root"); + analyzer.ComputeMarkedNodes(); + + Assert.Equal(new DependencyNodeCore[] { root, dependency }, analyzer.MarkedNodeList); + } + + [Theory] + [MemberData(nameof(ConditionalDependencyData))] + public void ConditionalDependenciesAreMarkedWhenConditionAppears(DependencyCollectionKind collectionKind, int count) + { + var condition = new TestNode("condition"); + TestNode[] dependencyNodes = CreateNodes("dependency", count); + var conditionProvider = new TestNode("condition provider") + { + StaticDependencies = + [ + new DependencyListEntry(condition, "condition"), + ], + }; + CombinedDependencyListEntry[] entries = CreateConditionalEntries(dependencyNodes, condition); + var root = new TestNode("root") + { + StaticDependencies = + [ + new DependencyListEntry(conditionProvider, "condition provider"), + ], + ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + + analyzer.AddRoot(root, "root"); + analyzer.ComputeMarkedNodes(); + + DependencyNodeCore[] expected = new DependencyNodeCore[count + 3]; + expected[0] = root; + expected[1] = conditionProvider; + expected[2] = condition; + CopyExpectedNodes(expected, 3, dependencyNodes, collectionKind); + Assert.Equal(expected, analyzer.MarkedNodeList); + } + + [Theory] + [MemberData(nameof(MutableStaticDependencyLists))] + public void StaticDependencyListMutationIsDetected(DependencyCollectionKind collectionKind) + { + List dependencies = collectionKind switch + { + DependencyCollectionKind.DependencyList => new DependencyList(), + DependencyCollectionKind.List => new List(), + _ => throw new UnreachableException(), + }; + var addedDependency = new TestNode("added dependency"); + var dependency = new TestNode( + "dependency", + () => dependencies.Add(new DependencyListEntry(addedDependency, "added dependency"))); + dependencies.Add(new DependencyListEntry(dependency, "dependency")); + var root = new TestNode("root") + { + StaticDependencies = dependencies, + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + analyzer.AddRoot(root, "root"); + + Assert.Throws(analyzer.ComputeMarkedNodes); + } + + [Fact] + public void ConditionalDependencyListMutationIsDetected() + { + var condition = new TestNode("condition"); + var addedDependency = new TestNode("added dependency"); + var dependencies = new List(); + var dependency = new TestNode( + "dependency", + () => dependencies.Add(new CombinedDependencyListEntry(addedDependency, condition, "added dependency"))); + dependencies.Add(new CombinedDependencyListEntry(dependency, condition, "dependency")); + var root = new TestNode("root") + { + ConditionalDependencies = dependencies, + }; + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + analyzer.AddRoot(condition, "condition"); + analyzer.AddRoot(root, "root"); + + Assert.Throws(analyzer.ComputeMarkedNodes); + } + + private static DependencyAnalyzer, object> CreateAnalyzer() + { + return new DependencyAnalyzer, object>(new object(), resultSorter: null); + } + + private static IEnumerable CreateDependencyData(DependencyCollectionKind[] collectionKinds) + { + int[] counts = [0, 1, 3]; + foreach (DependencyCollectionKind collectionKind in collectionKinds) + { + foreach (int count in counts) + { + yield return new object[] { collectionKind, count }; + } + } + } + + private static TestNode[] CreateNodes(string namePrefix, int count) + { + var nodes = new TestNode[count]; + for (int i = 0; i < nodes.Length; i++) + { + nodes[i] = new TestNode($"{namePrefix} {i}"); + } + + return nodes; + } + + private static DependencyListEntry[] CreateStaticEntries(TestNode[] dependencyNodes) + { + var entries = new DependencyListEntry[dependencyNodes.Length]; + for (int i = 0; i < entries.Length; i++) + { + entries[i] = new DependencyListEntry(dependencyNodes[i], $"dependency {i}"); + } + + return entries; + } + + private static CombinedDependencyListEntry[] CreateConditionalEntries( + TestNode[] dependencyNodes, + TestNode condition) + { + var entries = new CombinedDependencyListEntry[dependencyNodes.Length]; + for (int i = 0; i < entries.Length; i++) + { + entries[i] = new CombinedDependencyListEntry(dependencyNodes[i], condition, $"dependency {i}"); + } + + return entries; + } + + private static IEnumerable CreateStaticDependencies( + DependencyCollectionKind collectionKind, + DependencyListEntry[] entries) + { + return collectionKind switch + { + DependencyCollectionKind.Array => entries, + DependencyCollectionKind.DependencyList => new DependencyList(entries), + DependencyCollectionKind.Enumerable => Enumerate(entries), + DependencyCollectionKind.List => new List(entries), + DependencyCollectionKind.ReimplementedList => new ReimplementedEnumerableList(entries), + _ => throw new UnreachableException(), + }; + } + + private static IEnumerable CreateConditionalDependencies( + DependencyCollectionKind collectionKind, + CombinedDependencyListEntry[] entries) + { + return collectionKind switch + { + DependencyCollectionKind.Array => entries, + DependencyCollectionKind.Enumerable => Enumerate(entries), + DependencyCollectionKind.List => new List(entries), + DependencyCollectionKind.ReimplementedList => new ReimplementedEnumerableList(entries), + _ => throw new UnreachableException(), + }; + } + + private static IEnumerable Enumerate(T[] items) + { + foreach (T item in items) + { + yield return item; + } + } + + private static void CopyExpectedNodes( + DependencyNodeCore[] destination, + int destinationIndex, + TestNode[] nodes, + DependencyCollectionKind collectionKind) + { + if (collectionKind == DependencyCollectionKind.ReimplementedList) + { + for (int i = nodes.Length - 1; i >= 0; i--) + { + destination[destinationIndex++] = nodes[i]; + } + } + else + { + for (int i = 0; i < nodes.Length; i++) + { + destination[destinationIndex++] = nodes[i]; + } + } + } + + private sealed class ReimplementedEnumerableList : List, IEnumerable, IEnumerable + { + public ReimplementedEnumerableList(IEnumerable items) + : base(items) + { + } + + IEnumerator IEnumerable.GetEnumerator() + { + for (int i = Count - 1; i >= 0; i--) + { + yield return this[i]; + } + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)this).GetEnumerator(); + } + } + + private sealed class TestNode : DependencyNodeCore + { + private readonly string _name; + private readonly Action _onMarked; + + public TestNode(string name, Action onMarked = null) + { + _name = name; + _onMarked = onMarked; + } + + public IEnumerable StaticDependencies { get; set; } = Array.Empty(); + + public IEnumerable ConditionalDependencies { get; set; } + + public override bool InterestingForDynamicDependencyAnalysis => false; + + public override bool HasDynamicDependencies => false; + + public override bool HasConditionalStaticDependencies => ConditionalDependencies is not null; + + public override bool StaticDependenciesAreComputed => true; + + public override IEnumerable GetStaticDependencies(object context) + { + return StaticDependencies; + } + + public override IEnumerable GetConditionalStaticDependencies(object context) + { + return ConditionalDependencies; + } + + public override IEnumerable SearchDynamicDependencies( + List> markedNodes, + int firstNode, + object context) + { + return Array.Empty(); + } + + protected override void OnMarked(object context) + { + _onMarked?.Invoke(); + } + + protected override string GetName(object context) + { + return _name; + } + } + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj index 69e9d87637f92d..88c5f54858ac07 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/ILCompiler.Compiler.Tests.csproj @@ -40,6 +40,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs index f4adb44450ee85..a47c6923f2354e 100644 --- a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs +++ b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs @@ -183,34 +183,81 @@ private void GetStaticDependenciesImpl(DependencyNodeCore IEnumerable.DependencyListEntry> staticDependencies = node.GetStaticDependencies(_dependencyContext); if (staticDependencies != null) { - foreach (DependencyNodeCore.DependencyListEntry dependency in staticDependencies) + // Preserve custom enumeration behavior on List subclasses by only fast-pathing known exact types. + if (staticDependencies is List.DependencyListEntry> dependencyList && + (dependencyList.GetType() == typeof(DependencyNodeCore.DependencyList) || + dependencyList.GetType() == typeof(List.DependencyListEntry>))) { - AddToMarkStack(dependency.Node, dependency.Reason, node, null); + foreach (DependencyNodeCore.DependencyListEntry dependency in dependencyList) + { + AddToMarkStack(dependency.Node, dependency.Reason, node, null); + } + } + else if (staticDependencies is DependencyNodeCore.DependencyListEntry[] dependencyArray) + { + foreach (DependencyNodeCore.DependencyListEntry dependency in dependencyArray) + { + AddToMarkStack(dependency.Node, dependency.Reason, node, null); + } + } + else + { + foreach (DependencyNodeCore.DependencyListEntry dependency in staticDependencies) + { + AddToMarkStack(dependency.Node, dependency.Reason, node, null); + } } } if (node.HasConditionalStaticDependencies) { - foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in node.GetConditionalStaticDependencies(_dependencyContext)) + IEnumerable.CombinedDependencyListEntry> conditionalDependencies = + node.GetConditionalStaticDependencies(_dependencyContext); + if (conditionalDependencies is List.CombinedDependencyListEntry> dependencyList && + conditionalDependencies.GetType() == typeof(List.CombinedDependencyListEntry>)) { - if (dependency.OtherReasonNode is null || dependency.OtherReasonNode.Marked) + foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in dependencyList) { - AddToMarkStack(dependency.Node, dependency.Reason, node, dependency.OtherReasonNode); + ProcessConditionalDependency(node, dependency); } - else + } + else if (conditionalDependencies is DependencyNodeCore.CombinedDependencyListEntry[] dependencyArray) + { + foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in dependencyArray) { - HashSet.CombinedDependencyListEntry> storedDependencySet; - if (!_conditional_dependency_store.TryGetValue(dependency.OtherReasonNode, out storedDependencySet)) - { - storedDependencySet = new HashSet.CombinedDependencyListEntry>(); - _conditional_dependency_store.Add(dependency.OtherReasonNode, storedDependencySet); - } - // Swap out other reason node as we're storing that as the dictionary key - DependencyNodeCore.CombinedDependencyListEntry conditionalDependencyStoreEntry = - new DependencyNodeCore.CombinedDependencyListEntry(dependency.Node, node, dependency.Reason); - storedDependencySet.Add(conditionalDependencyStoreEntry); + ProcessConditionalDependency(node, dependency); } } + else + { + foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in conditionalDependencies) + { + ProcessConditionalDependency(node, dependency); + } + } + } + } + + private void ProcessConditionalDependency( + DependencyNodeCore node, + in DependencyNodeCore.CombinedDependencyListEntry dependency) + { + if (dependency.OtherReasonNode is null || dependency.OtherReasonNode.Marked) + { + AddToMarkStack(dependency.Node, dependency.Reason, node, dependency.OtherReasonNode); + } + else + { + HashSet.CombinedDependencyListEntry> storedDependencySet; + if (!_conditional_dependency_store.TryGetValue(dependency.OtherReasonNode, out storedDependencySet)) + { + storedDependencySet = new HashSet.CombinedDependencyListEntry>(); + _conditional_dependency_store.Add(dependency.OtherReasonNode, storedDependencySet); + } + // Swap out other reason node as we're storing that as the dictionary key + DependencyNodeCore.CombinedDependencyListEntry conditionalDependencyStoreEntry = + new DependencyNodeCore.CombinedDependencyListEntry(dependency.Node, node, dependency.Reason); + storedDependencySet.Add(conditionalDependencyStoreEntry); } } From a3edfc705ff87a59c0265d78338ee2fbd2fc8b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Sat, 29 Aug 2026 21:27:48 -0400 Subject: [PATCH 2/2] Refine dependency list fast paths Move concrete-list selection to dependency producers so the analyzer no longer relies on runtime type checks. Preserve the enumerable fallback and cover empty concrete-list results. --- .../Compiler/DependencyAnalysis/ObjectNode.cs | 17 +- .../VirtualMethodUseNode.cs | 11 ++ .../DependencyAnalyzerTests.cs | 174 ++++++++++++++---- .../Compiler/DependencyAnalysis/EETypeNode.cs | 13 ++ .../DependencyAnalysis/ScannedMethodNode.cs | 15 ++ .../DependencyAnalyzer.cs | 51 +++-- .../DependencyNodeCore.cs | 20 ++ ...ompiler.DependencyAnalysisFramework.csproj | 6 + .../DependencyAnalysis/MethodCodeNode.cs | 16 +- 9 files changed, 251 insertions(+), 72 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs index 151fa1e8f587bf..0a41531922108a 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs @@ -54,6 +54,18 @@ public virtual bool ShouldSkipEmittingObjectNode(NodeFactory factory) public override bool InterestingForDynamicDependencyAnalysis => false; public sealed override IEnumerable GetStaticDependencies(NodeFactory factory) + { + DependencyList dependencies = GetStaticDependencyList(factory); + return dependencies is not null ? dependencies : Array.Empty(); + } + + internal sealed override bool TryGetStaticDependencyList(NodeFactory factory, out DependencyList dependencies) + { + dependencies = GetStaticDependencyList(factory); + return true; + } + + private DependencyList GetStaticDependencyList(NodeFactory factory) { DependencyList dependencies = ComputeNonRelocationBasedDependencies(factory); Relocation[] relocs = GetData(factory, true).Relocs; @@ -76,10 +88,7 @@ public sealed override IEnumerable GetStaticDependencies(No dependencies.Add(wasmTypeNode, "Wasm Method Code Nodes Require Signature"); } - if (dependencies == null) - return Array.Empty(); - else - return dependencies; + return dependencies; } protected virtual DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory) diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs index 48825d0e82c059..fe7f2112a9e06c 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs @@ -61,6 +61,17 @@ protected override void OnMarked(NodeFactory factory) #if !READYTORUN public override IEnumerable GetStaticDependencies(NodeFactory factory) + { + return GetStaticDependencyList(factory); + } + + internal override bool TryGetStaticDependencyList(NodeFactory factory, out DependencyList dependencies) + { + dependencies = GetStaticDependencyList(factory); + return true; + } + + private DependencyList GetStaticDependencyList(NodeFactory factory) { DependencyList dependencies = new DependencyList(); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs index 1c150d3b4d5515..1de00537054749 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyAnalyzerTests.cs @@ -74,10 +74,8 @@ public void StaticDependenciesPreserveOrder(DependencyCollectionKind collectionK { TestNode[] dependencyNodes = CreateNodes("dependency", count); DependencyListEntry[] entries = CreateStaticEntries(dependencyNodes); - var root = new TestNode("root") - { - StaticDependencies = CreateStaticDependencies(collectionKind, entries), - }; + var root = new TestNode("root"); + SetStaticDependencies(root, collectionKind, entries); DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(root, "root"); @@ -87,6 +85,7 @@ public void StaticDependenciesPreserveOrder(DependencyCollectionKind collectionK expected[0] = root; CopyExpectedNodes(expected, 1, dependencyNodes, collectionKind); Assert.Equal(expected, analyzer.MarkedNodeList); + Assert.Equal(collectionKind == DependencyCollectionKind.DependencyList ? 0 : 1, root.StaticEnumerableAccessCount); } [Theory] @@ -96,10 +95,8 @@ public void ConditionalDependenciesPreserveOrder(DependencyCollectionKind collec var condition = new TestNode("condition"); TestNode[] dependencyNodes = CreateNodes("dependency", count); CombinedDependencyListEntry[] entries = CreateConditionalEntries(dependencyNodes, condition); - var root = new TestNode("root") - { - ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), - }; + var root = new TestNode("root"); + SetConditionalDependencies(root, collectionKind, entries); DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(condition, "condition"); @@ -111,6 +108,7 @@ public void ConditionalDependenciesPreserveOrder(DependencyCollectionKind collec expected[1] = root; CopyExpectedNodes(expected, 2, dependencyNodes, collectionKind); Assert.Equal(expected, analyzer.MarkedNodeList); + Assert.Equal(collectionKind == DependencyCollectionKind.List ? 0 : 1, root.ConditionalEnumerableAccessCount); } [Theory] @@ -122,10 +120,8 @@ public void NullConditionalDependencyIsUnconditional(DependencyCollectionKind co [ new CombinedDependencyListEntry(dependency, null, "unconditional"), ]; - var root = new TestNode("root") - { - ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), - }; + var root = new TestNode("root"); + SetConditionalDependencies(root, collectionKind, entries); DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(root, "root"); @@ -140,22 +136,20 @@ public void ConditionalDependenciesAreMarkedWhenConditionAppears(DependencyColle { var condition = new TestNode("condition"); TestNode[] dependencyNodes = CreateNodes("dependency", count); - var conditionProvider = new TestNode("condition provider") - { - StaticDependencies = + var conditionProvider = new TestNode("condition provider"); + conditionProvider.SetStaticDependencies( + (IEnumerable) [ new DependencyListEntry(condition, "condition"), - ], - }; + ]); CombinedDependencyListEntry[] entries = CreateConditionalEntries(dependencyNodes, condition); - var root = new TestNode("root") - { - StaticDependencies = + var root = new TestNode("root"); + root.SetStaticDependencies( + (IEnumerable) [ new DependencyListEntry(conditionProvider, "condition provider"), - ], - ConditionalDependencies = CreateConditionalDependencies(collectionKind, entries), - }; + ]); + SetConditionalDependencies(root, collectionKind, entries); DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(root, "root"); @@ -184,10 +178,15 @@ public void StaticDependencyListMutationIsDetected(DependencyCollectionKind coll "dependency", () => dependencies.Add(new DependencyListEntry(addedDependency, "added dependency"))); dependencies.Add(new DependencyListEntry(dependency, "dependency")); - var root = new TestNode("root") + var root = new TestNode("root"); + if (dependencies is DependencyList dependencyList) { - StaticDependencies = dependencies, - }; + root.SetStaticDependencies(dependencyList); + } + else + { + root.SetStaticDependencies((IEnumerable)dependencies); + } DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(root, "root"); @@ -204,10 +203,8 @@ public void ConditionalDependencyListMutationIsDetected() "dependency", () => dependencies.Add(new CombinedDependencyListEntry(addedDependency, condition, "added dependency"))); dependencies.Add(new CombinedDependencyListEntry(dependency, condition, "dependency")); - var root = new TestNode("root") - { - ConditionalDependencies = dependencies, - }; + var root = new TestNode("root"); + root.SetConditionalDependencies(dependencies); DependencyAnalyzer, object> analyzer = CreateAnalyzer(); analyzer.AddRoot(condition, "condition"); analyzer.AddRoot(root, "root"); @@ -215,6 +212,22 @@ public void ConditionalDependencyListMutationIsDetected() Assert.Throws(analyzer.ComputeMarkedNodes); } + [Fact] + public void NullConcreteDependencyListsDoNotUseEnumerableFallback() + { + var root = new TestNode("root"); + root.SetNullStaticDependencyList(); + root.SetNullConditionalDependencyList(); + DependencyAnalyzer, object> analyzer = CreateAnalyzer(); + + analyzer.AddRoot(root, "root"); + analyzer.ComputeMarkedNodes(); + + Assert.Equal(new DependencyNodeCore[] { root }, analyzer.MarkedNodeList); + Assert.Equal(0, root.StaticEnumerableAccessCount); + Assert.Equal(0, root.ConditionalEnumerableAccessCount); + } + private static DependencyAnalyzer, object> CreateAnalyzer() { return new DependencyAnalyzer, object>(new object(), resultSorter: null); @@ -282,6 +295,21 @@ private static IEnumerable CreateStaticDependencies( }; } + private static void SetStaticDependencies( + TestNode node, + DependencyCollectionKind collectionKind, + DependencyListEntry[] entries) + { + if (collectionKind == DependencyCollectionKind.DependencyList) + { + node.SetStaticDependencies(new DependencyList(entries)); + } + else + { + node.SetStaticDependencies(CreateStaticDependencies(collectionKind, entries)); + } + } + private static IEnumerable CreateConditionalDependencies( DependencyCollectionKind collectionKind, CombinedDependencyListEntry[] entries) @@ -296,6 +324,21 @@ private static IEnumerable CreateConditionalDepende }; } + private static void SetConditionalDependencies( + TestNode node, + DependencyCollectionKind collectionKind, + CombinedDependencyListEntry[] entries) + { + if (collectionKind == DependencyCollectionKind.List) + { + node.SetConditionalDependencies(new List(entries)); + } + else + { + node.SetConditionalDependencies(CreateConditionalDependencies(collectionKind, entries)); + } + } + private static IEnumerable Enumerate(T[] items) { foreach (T item in items) @@ -351,6 +394,16 @@ private sealed class TestNode : DependencyNodeCore { private readonly string _name; private readonly Action _onMarked; + private IEnumerable _staticDependencies = Array.Empty(); + private IEnumerable _conditionalDependencies; + private DependencyList _staticDependencyList; + private List _conditionalDependencyList; + private bool _providesStaticDependencyList; + private bool _providesConditionalDependencyList; + + public int StaticEnumerableAccessCount { get; private set; } + + public int ConditionalEnumerableAccessCount { get; private set; } public TestNode(string name, Action onMarked = null) { @@ -358,26 +411,73 @@ public TestNode(string name, Action onMarked = null) _onMarked = onMarked; } - public IEnumerable StaticDependencies { get; set; } = Array.Empty(); - - public IEnumerable ConditionalDependencies { get; set; } - public override bool InterestingForDynamicDependencyAnalysis => false; public override bool HasDynamicDependencies => false; - public override bool HasConditionalStaticDependencies => ConditionalDependencies is not null; + public override bool HasConditionalStaticDependencies => + _providesConditionalDependencyList || _conditionalDependencies is not null; public override bool StaticDependenciesAreComputed => true; + public void SetStaticDependencies(IEnumerable dependencies) + { + _staticDependencies = dependencies; + } + + public void SetStaticDependencies(DependencyList dependencies) + { + _staticDependencies = dependencies; + _staticDependencyList = dependencies; + _providesStaticDependencyList = true; + } + + public void SetConditionalDependencies(IEnumerable dependencies) + { + _conditionalDependencies = dependencies; + } + + public void SetConditionalDependencies(List dependencies) + { + _conditionalDependencies = dependencies; + _conditionalDependencyList = dependencies; + _providesConditionalDependencyList = true; + } + + public void SetNullStaticDependencyList() + { + _providesStaticDependencyList = true; + } + + public void SetNullConditionalDependencyList() + { + _providesConditionalDependencyList = true; + } + public override IEnumerable GetStaticDependencies(object context) { - return StaticDependencies; + StaticEnumerableAccessCount++; + return _staticDependencies; } public override IEnumerable GetConditionalStaticDependencies(object context) { - return ConditionalDependencies; + ConditionalEnumerableAccessCount++; + return _conditionalDependencies; + } + + internal override bool TryGetStaticDependencyList(object context, out DependencyList dependencies) + { + dependencies = _staticDependencyList; + return _providesStaticDependencyList; + } + + internal override bool TryGetConditionalStaticDependencyList( + object context, + out List dependencies) + { + dependencies = _conditionalDependencyList; + return _providesConditionalDependencyList; } public override IEnumerable SearchDynamicDependencies( diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs index da95fde9335dc1..f58dd1ae7ae6f5 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -313,6 +313,19 @@ public override bool HasConditionalStaticDependencies } public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) + { + return GetConditionalStaticDependencyList(factory); + } + + internal override bool TryGetConditionalStaticDependencyList( + NodeFactory factory, + out List dependencies) + { + dependencies = GetConditionalStaticDependencyList(factory); + return true; + } + + private List GetConditionalStaticDependencyList(NodeFactory factory) { List result = new List(); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ScannedMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ScannedMethodNode.cs index cd4dd351393edf..a42059f1223166 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ScannedMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ScannedMethodNode.cs @@ -82,8 +82,23 @@ public override IEnumerable GetStaticDependencies(NodeFacto return _dependencies; } + internal override bool TryGetStaticDependencyList(NodeFactory factory, out DependencyList dependencies) + { + Debug.Assert(_dependencies != null); + dependencies = _dependencies; + return true; + } + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => _conditionalDependencies; + internal override bool TryGetConditionalStaticDependencyList( + NodeFactory factory, + out List dependencies) + { + dependencies = _conditionalDependencies; + return true; + } + protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler); public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; diff --git a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs index a47c6923f2354e..99efca273c77db 100644 --- a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs +++ b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs @@ -180,29 +180,23 @@ private void ComputeDependencies(List> // Internal details private void GetStaticDependenciesImpl(DependencyNodeCore node) { - IEnumerable.DependencyListEntry> staticDependencies = node.GetStaticDependencies(_dependencyContext); - if (staticDependencies != null) + if (node.TryGetStaticDependencyList(_dependencyContext, out DependencyNodeCore.DependencyList staticDependencies)) { - // Preserve custom enumeration behavior on List subclasses by only fast-pathing known exact types. - if (staticDependencies is List.DependencyListEntry> dependencyList && - (dependencyList.GetType() == typeof(DependencyNodeCore.DependencyList) || - dependencyList.GetType() == typeof(List.DependencyListEntry>))) + if (staticDependencies != null) { - foreach (DependencyNodeCore.DependencyListEntry dependency in dependencyList) + foreach (DependencyNodeCore.DependencyListEntry dependency in staticDependencies) { AddToMarkStack(dependency.Node, dependency.Reason, node, null); } } - else if (staticDependencies is DependencyNodeCore.DependencyListEntry[] dependencyArray) + } + else + { + IEnumerable.DependencyListEntry> enumerableDependencies = + node.GetStaticDependencies(_dependencyContext); + if (enumerableDependencies != null) { - foreach (DependencyNodeCore.DependencyListEntry dependency in dependencyArray) - { - AddToMarkStack(dependency.Node, dependency.Reason, node, null); - } - } - else - { - foreach (DependencyNodeCore.DependencyListEntry dependency in staticDependencies) + foreach (DependencyNodeCore.DependencyListEntry dependency in enumerableDependencies) { AddToMarkStack(dependency.Node, dependency.Reason, node, null); } @@ -211,26 +205,23 @@ private void GetStaticDependenciesImpl(DependencyNodeCore if (node.HasConditionalStaticDependencies) { - IEnumerable.CombinedDependencyListEntry> conditionalDependencies = - node.GetConditionalStaticDependencies(_dependencyContext); - if (conditionalDependencies is List.CombinedDependencyListEntry> dependencyList && - conditionalDependencies.GetType() == typeof(List.CombinedDependencyListEntry>)) + if (node.TryGetConditionalStaticDependencyList( + _dependencyContext, + out List.CombinedDependencyListEntry> conditionalDependencies)) { - foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in dependencyList) + if (conditionalDependencies != null) { - ProcessConditionalDependency(node, dependency); - } - } - else if (conditionalDependencies is DependencyNodeCore.CombinedDependencyListEntry[] dependencyArray) - { - foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in dependencyArray) - { - ProcessConditionalDependency(node, dependency); + foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in conditionalDependencies) + { + ProcessConditionalDependency(node, dependency); + } } } else { - foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in conditionalDependencies) + IEnumerable.CombinedDependencyListEntry> enumerableDependencies = + node.GetConditionalStaticDependencies(_dependencyContext); + foreach (DependencyNodeCore.CombinedDependencyListEntry dependency in enumerableDependencies) { ProcessConditionalDependency(node, dependency); } diff --git a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyNodeCore.cs b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyNodeCore.cs index 8c8e37893cc333..c70d94070c1453 100644 --- a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyNodeCore.cs +++ b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyNodeCore.cs @@ -127,6 +127,26 @@ public abstract bool StaticDependenciesAreComputed public abstract IEnumerable GetConditionalStaticDependencies(DependencyContextType context); + // Allows nodes that naturally produce a concrete list to avoid exposing it as IEnumerable + // to the dependency analyzer. A true result means the list was provided, including when it is null. + internal virtual bool TryGetStaticDependencyList( + DependencyContextType context, + out DependencyList dependencies) + { + dependencies = null; + return false; + } + + // Allows nodes that naturally produce a concrete list to avoid exposing it as IEnumerable + // to the dependency analyzer. A true result means the list was provided, including when it is null. + internal virtual bool TryGetConditionalStaticDependencyList( + DependencyContextType context, + out List dependencies) + { + dependencies = null; + return false; + } + public abstract IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, DependencyContextType context); internal void CallOnMarked(DependencyContextType context) diff --git a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/ILCompiler.DependencyAnalysisFramework.csproj b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/ILCompiler.DependencyAnalysisFramework.csproj index 28aa33b5afe733..29c137e64edfd4 100644 --- a/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/ILCompiler.DependencyAnalysisFramework.csproj +++ b/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/ILCompiler.DependencyAnalysisFramework.csproj @@ -15,6 +15,12 @@ false Debug;Release;Checked + + + + + + diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/MethodCodeNode.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/MethodCodeNode.cs index f8418a8d67befc..c035b2b97cd71d 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/MethodCodeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/MethodCodeNode.cs @@ -67,10 +67,24 @@ public virtual void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder public override bool HasConditionalStaticDependencies => CodeBasedDependencyAlgorithm.HasConditionalDependenciesDueToMethodCodePresence(_method); public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) + { + CombinedDependencyList dependencies = GetConditionalStaticDependencyList(factory); + return dependencies is not null ? dependencies : Array.Empty(); + } + + internal override bool TryGetConditionalStaticDependencyList( + NodeFactory factory, + out List dependencies) + { + dependencies = GetConditionalStaticDependencyList(factory); + return true; + } + + private CombinedDependencyList GetConditionalStaticDependencyList(NodeFactory factory) { CombinedDependencyList dependencies = null; CodeBasedDependencyAlgorithm.AddConditionalDependenciesDueToMethodCodePresence(ref dependencies, factory, _method); - return dependencies ?? (IEnumerable)Array.Empty(); + return dependencies; } protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)