diff --git a/test/NumSharp.Tests.Build.Analyzer/OwnershipRobustnessTests.cs b/test/NumSharp.Tests.Build.Analyzer/OwnershipRobustnessTests.cs index 6d1f6f31b..cd67a2c21 100644 --- a/test/NumSharp.Tests.Build.Analyzer/OwnershipRobustnessTests.cs +++ b/test/NumSharp.Tests.Build.Analyzer/OwnershipRobustnessTests.cs @@ -123,6 +123,44 @@ public async Task VeryWideType_Completes_AndCountsEveryForgottenMember() Assert.AreEqual(n - 1, r.CountOf("NDW017"), "every member except the one disposed is reported"); } + /// + /// A domain model whose types all reference each other (entities with navigation properties): + /// the "holds NDArrays" answers of a cycle must be memoized, or every type re-walks every path + /// through the graph - exponential in the number of types. + /// + private static string DenselyCyclicModel(int n, bool withArray) + { + var sb = new StringBuilder(); + for (int i = 0; i < n; i++) + { + sb.Append("public class M").Append(i).Append(" {"); + for (int j = 0; j < n; j++) + if (j != i) + sb.Append(" public M").Append(j).Append(" P").Append(j).Append(" { get; set; }"); + if (withArray && i == n - 1) + sb.Append(" NDArray _x;"); + sb.AppendLine(" }"); + } + return sb.ToString(); + } + + [TestMethod] + [Timeout(60000)] + public async Task DenselyCyclicModelWithoutArrays_CompletesClean() + { + var r = await Run(DenselyCyclicModel(14, withArray: false)); + Assert.AreEqual(0, r.Ndw.Length, "types that only reference each other store no NDArray"); + } + + [TestMethod] + [Timeout(60000)] + public async Task DenselyCyclicModelWithAnArray_FlagsEveryType() + { + const int n = 14; + var r = await Run(DenselyCyclicModel(n, withArray: true)); + Assert.AreEqual(n, r.CountOf("NDW016"), "every type reaches the one array through the graph"); + } + [TestMethod] public async Task ManyTypes_Complete() { diff --git a/tools/NumSharp.Build.Analyzer/OwnershipModel.cs b/tools/NumSharp.Build.Analyzer/OwnershipModel.cs index 7a335dc28..aa274ee11 100644 --- a/tools/NumSharp.Build.Analyzer/OwnershipModel.cs +++ b/tools/NumSharp.Build.Analyzer/OwnershipModel.cs @@ -86,11 +86,21 @@ public HolderMember(ISymbol symbol, ITypeSymbol type) /// public List HolderMembersOf(INamedTypeSymbol type) { - bool cut = false; - return HolderMembersOf(type, new HashSet(SymbolEqualityComparer.Default) { type }, ref cut); + // A source class/struct that does not hold NDArrays as a whole has no holding member: each of + // its storing members feeds the type's own answer. That answer is memoized, so a type unrelated + // to NumSharp costs one lookup here instead of a walk of its member graph. + if (IsInSource(type) && (type.TypeKind == TypeKind.Class || type.TypeKind == TypeKind.Struct) + && !HoldsNDArrays(type)) + return new List(); + + // The type itself is on the walk's stack: a member that reaches arrays only by coming back to + // this type adds nothing. Answers of the cycles through it stay pending and are not memoized. + var walk = new Walk(); + int low = walk.Enter(type); + return HolderMembersOf(type, walk, ref low); } - private List HolderMembersOf(INamedTypeSymbol type, HashSet visiting, ref bool cut) + private List HolderMembersOf(INamedTypeSymbol type, Walk walk, ref int low) { var list = new List(); bool inSource = IsInSource(type); @@ -137,7 +147,7 @@ private List HolderMembersOf(INamedTypeSymbol type, HashSet HolderMembersOf(INamedTypeSymbol type, HashSetDoes a value of this type (transitively) hold an NDArray somebody must dispose? public bool HoldsNDArrays(ITypeSymbol t) { - bool cut = false; - return HoldsCore(t, null, ref cut); + int low = int.MaxValue; + return HoldsCore(t, null, ref low); } - private bool HoldsCore(ITypeSymbol t, HashSet visiting, ref bool cut) + private bool HoldsCore(ITypeSymbol t, Walk walk, ref int low) { if (t == null) return false; if (_holds.TryGetValue(t, out var memo)) return memo; - bool root = visiting == null; - if (root) - visiting = new HashSet(SymbolEqualityComparer.Default); - if (!visiting.Add(t)) + if (walk == null) + walk = new Walk(); + if (walk.IndexOf(t, out int seen)) { - // A cycle (a type reachable from itself): this occurrence contributes nothing; the - // type's answer is decided by its other members. The enclosing answers are then - // inexact and must not be memoized — only the ROOT answer is exact (a cycle back to - // the root cannot change what the root itself holds). - cut = true; + // A cycle (a type reachable from itself): this occurrence contributes nothing; the type's + // answer is decided by its other members. Every type visited in this walk and not yet + // memoized is still on the stack, waiting for the cycle it belongs to. + if (seen < low) + low = seen; return false; } - bool localCut = false; - bool result = Compute(t, visiting, ref localCut); - visiting.Remove(t); - if (root || !localCut) - _holds[t] = result; - if (localCut) - cut = true; + // Tarjan's strongly connected components over the "stores a value of" graph, so each type is + // walked once. "Holds" is the least fixed point of an OR over that graph: a true answer is + // exact as soon as it is found, and the types of one cycle (one component) share one answer, + // the answer of the type the walk entered the component through. Memoizing only answers that + // involved no cycle at all made every type re-walk every path through a model whose types + // reference each other - exponential in the number of types. + int index = walk.Enter(t); + int ownLow = index; + bool result = Compute(t, walk, ref ownLow); + if (result) + _holds[t] = true; + if (ownLow == index) + { + // t is the entry of its component: everything above it on the stack belongs to the + // component and reaches t, so it holds exactly when t does. + foreach (var member in walk.ExitComponent(t)) + _holds[member] = result || _holds.TryGetValue(member, out var held) && held; + } + if (ownLow < low) + low = ownLow; return result; } - private bool Compute(ITypeSymbol t, HashSet visiting, ref bool cut) + /// One walk: Tarjan's visit order and the stack of types whose component is still open. + private sealed class Walk + { + private readonly Dictionary _index = new Dictionary(SymbolEqualityComparer.Default); + private readonly Stack _stack = new Stack(); + + /// Visits : gives it the next index and puts it on the stack. + public int Enter(ITypeSymbol t) + { + int index = _index.Count; + _index[t] = index; + _stack.Push(t); + return index; + } + + /// Whether was visited in this walk (and so is still on the stack, its answer not memoized). + public bool IndexOf(ITypeSymbol t, out int index) => _index.TryGetValue(t, out index); + + /// Pops the component entered through : the entry and every type above it. + public List ExitComponent(ITypeSymbol entry) + { + var component = new List(); + ITypeSymbol t; + do + { + t = _stack.Pop(); + component.Add(t); + } while (!SymbolEqualityComparer.Default.Equals(t, entry)); + return component; + } + } + + private bool Compute(ITypeSymbol t, Walk walk, ref int low) { switch (t) { case IArrayTypeSymbol arr: - return HoldsCore(arr.ElementType, visiting, ref cut); + return HoldsCore(arr.ElementType, walk, ref low); case IPointerTypeSymbol _: case IDynamicTypeSymbol _: return false; case ITypeParameterSymbol tp: foreach (var c in tp.ConstraintTypes) - if (HoldsCore(c, visiting, ref cut)) + if (HoldsCore(c, walk, ref low)) return true; return false; } @@ -216,7 +270,7 @@ private bool Compute(ITypeSymbol t, HashSet visiting, ref bool cut) if (named.IsTupleType) { foreach (var e in named.TupleElements) - if (HoldsCore(e.Type, visiting, ref cut)) + if (HoldsCore(e.Type, walk, ref low)) return true; return false; } @@ -231,7 +285,7 @@ private bool Compute(ITypeSymbol t, HashSet visiting, ref bool cut) for (var g = named; g != null; g = g.ContainingType) if (g.IsGenericType) foreach (var arg in g.TypeArguments) - if (HoldsCore(arg, visiting, ref cut)) + if (HoldsCore(arg, walk, ref low)) return true; // A declared class/struct: its own storing members, then its base. Only an assembly that @@ -242,9 +296,9 @@ private bool Compute(ITypeSymbol t, HashSet visiting, ref bool cut) if (!IsInSource(named) && !CanNameNDArray(named.ContainingAssembly)) return false; - if (HolderMembersOf(named, visiting, ref cut).Count > 0) + if (HolderMembersOf(named, walk, ref low).Count > 0) return true; - return named.BaseType != null && HoldsCore(named.BaseType, visiting, ref cut); + return named.BaseType != null && HoldsCore(named.BaseType, walk, ref low); } ///