From 4ba7ed7761a74ae458f59ad6162c8a7efd3265e3 Mon Sep 17 00:00:00 2001 From: campersau Date: Fri, 23 Oct 2026 19:45:43 +0200 Subject: [PATCH] fix(analyzer): walk each type once when deciding which types hold NDArrays OwnershipModel.HoldsCore memoized a "holds NDArrays" answer only when its walk had cut no cycle, and HolderMembersOf put the asking type itself on the walk. In a model whose types reference each other (entities with navigation properties) nearly every answer involves a cycle, so nothing was memoized and every type re-walked every path through the graph - exponential in the number of types. NDArrayHolderAnalyzer (NDW016/NDW017) runs this for every named type of the compilation. Seen in a consumer that references NumSharp only indirectly: NDArrayHolderAnalyzer took 321.6 s of analyzer time on one project (99 % of all analyzers, a 108 s compile) that declares no NDArray field at all; with this change 0.025 s and a 5 s compile, with the same diagnostics (none). HoldsCore now runs Tarjan's strongly connected components over the "stores a value of" graph: each type is visited once per walk, a true answer is memoized as soon as it is found, and the types of one cycle are memoized together with the answer of the type the walk entered the cycle through - "holds" is the least fixed point of an OR over the graph, so a cycle holds exactly when its entry does. HolderMembersOf answers a source class/struct that does not hold NDArrays as a whole from that memo instead of walking its members; for a holding type it keeps the type on the walk's stack as before, so a member that reaches arrays only through the type itself still adds nothing. Two tests pin it: a model of 14 types that all reference each other completes (no NDArray: clean; one NDArray: all 14 flagged NDW016). Both time out after 60 s without the change. All 330 tests of NumSharp.Tests.Build.Analyzer pass on net8.0 and net10.0. --- .../OwnershipRobustnessTests.cs | 38 ++++++ .../NumSharp.Build.Analyzer/OwnershipModel.cs | 114 +++++++++++++----- 2 files changed, 122 insertions(+), 30 deletions(-) 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); } ///