From d0d8afa7dc8a5ce1a40625f2242eba95bb5cdc32 Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Tue, 30 Jan 2024 16:34:34 -0800 Subject: [PATCH 1/7] Switch CreateNodeAsync to an iterative approach --- .../UnexpectedDependencyMessages.cs | 1 - .../SourceRepositoryDependencyProvider.cs | 6 +- .../Utility/BuildAssetsUtils.cs | 15 +- .../GraphModel/GraphNode.cs | 9 + .../GraphModel/Tracker.cs | 115 +++++++- .../Remote/RemoteDependencyWalker.cs | 276 +++++++++++++----- .../NuGet.Frameworks/FrameworkNameProvider.cs | 13 +- .../ContentModel/ManagedCodeConventions.cs | 10 +- .../RuntimeModel/RuntimeGraph.cs | 13 +- .../NuGet.ProjectModel/PackageSpecWriter.cs | 4 +- 10 files changed, 356 insertions(+), 106 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Diagnostics/UnexpectedDependencyMessages.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Diagnostics/UnexpectedDependencyMessages.cs index 8d7f019f1c5..c4012371375 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Diagnostics/UnexpectedDependencyMessages.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Diagnostics/UnexpectedDependencyMessages.cs @@ -73,7 +73,6 @@ public static IEnumerable GetMissingLowerBounds(IEnumerable !ignoreIds.Contains(e.Child.Name, StringComparer.OrdinalIgnoreCase) && DependencyRangeHasMissingExactMatch(e)) .OrderBy(e => e.Child.Name, StringComparer.OrdinalIgnoreCase) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs index 368ea7b9890..ad9e71c9c9c 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs @@ -231,7 +231,7 @@ Task FindLibraryIdentityAsync(LibraryRange libraryRange, Source { lock (_libraryMatchCache) { - result = Task.Run(() => FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken), cancellationToken); + result = FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken); _libraryMatchCache[libraryRange] = result; } } @@ -241,7 +241,7 @@ Task FindLibraryIdentityAsync(LibraryRange libraryRange, Source { if (!_libraryMatchCache.TryGetValue(libraryRange, out result)) { - result = Task.Run(() => FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken), cancellationToken); + result = FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken); _libraryMatchCache[libraryRange] = result; } } @@ -257,6 +257,7 @@ private async Task FindLibraryCoreAsync( ILogger logger, CancellationToken cancellationToken) { + await Task.Yield(); await EnsureResource(); if (libraryRange.VersionRange?.MinVersion != null && libraryRange.VersionRange.IsMinInclusive && !libraryRange.VersionRange.IsFloating) @@ -400,6 +401,7 @@ private async Task GetDependenciesCoreAsync( FindPackageByIdDependencyInfo packageInfo = null; try { + await Task.Yield(); await EnsureResource(); if (_throttle != null) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs index 4bf843f4bac..6d86f7578f2 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs @@ -704,19 +704,18 @@ private static IEnumerable GetLanguageConditions(string language, Sorted public static string GetLanguage(string nugetLanguage) { - var lang = nugetLanguage.ToUpperInvariant(); - // Translate S -> # - switch (lang) + if (StringComparer.OrdinalIgnoreCase.Equals(nugetLanguage, "CS")) + { + return "C#"; + } + else if (StringComparer.OrdinalIgnoreCase.Equals(nugetLanguage, "FS")) { - case "CS": - return "C#"; - case "FS": - return "F#"; + return "F#"; } // Return the language as it is - return lang; + return nugetLanguage.ToUpperInvariant(); } private static IEnumerable GetLanguageGroups( diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/GraphNode.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/GraphNode.cs index c1fe21ddf28..8a3d8108a33 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/GraphNode.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/GraphNode.cs @@ -62,6 +62,11 @@ internal bool AreAllParentsRejected() /// The count of additional items that will be added. internal void EnsureInnerNodeCapacity(int additionalSpace) { + if (additionalSpace <= 0) + { + return; + } + if (InnerNodes is List> innerList) { int requiredCapacity = innerList.Count + additionalSpace; @@ -70,6 +75,10 @@ internal void EnsureInnerNodeCapacity(int additionalSpace) innerList.Capacity = requiredCapacity; } } + else + { + InnerNodes = new List>(additionalSpace); + } } public override string ToString() diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/Tracker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/Tracker.cs index ffbe937cca6..904ce2a5a54 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/Tracker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/GraphModel/Tracker.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; using System.Collections.Generic; using System.Linq; @@ -44,7 +45,7 @@ public bool IsBestVersion(GraphItem item) { var version = item.Key.Version; - foreach (var known in entry.Items.NoAllocEnumerate()) + foreach (var known in entry) { if (version < known.Key.Version) { @@ -58,7 +59,13 @@ public bool IsBestVersion(GraphItem item) public IEnumerable> GetDisputes(GraphItem item) { - return TryGetEntry(item)?.Items ?? Enumerable.Empty>(); + var entry = TryGetEntry(item); + if (entry is null) + { + return Enumerable.Empty>(); + } + + return entry; } internal void Clear() @@ -83,7 +90,7 @@ private Entry GetOrAddEntry(GraphItem item) return entry; } - private sealed class Entry + private sealed class Entry : IEnumerable> { /// /// This field can have one of three values: @@ -123,17 +130,105 @@ public void AddItem(GraphItem item) } } + public Enumerator GetEnumerator() + { + return new Enumerator(this); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + return new Enumerator(this); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return new Enumerator(this); + } + public bool HasMultipleItems => _storage is HashSet>; - public IEnumerable> Items + public struct Enumerator : IEnumerator>, IDisposable, IEnumerator { - get + private enum Type : byte + { + Empty, + SingleItem, + MultipleItems, + } + + private readonly Type _type; + private readonly Entry _entry; + private GraphItem _current; + private bool _done; + private HashSet>.Enumerator _setEnumerator; + + public Enumerator(Entry entry) + { + _entry = entry; + _current = default!; + _done = false; + + if (_entry._storage is null) + { + _type = Type.Empty; + } + else if (_entry._storage is GraphItem) + { + _type = Type.SingleItem; + } + else + { + _type = Type.MultipleItems; + _setEnumerator = ((HashSet>)_entry._storage).GetEnumerator(); + } + } + + public readonly GraphItem Current => _current; + + readonly object IEnumerator.Current => Current; + + public bool MoveNext() + { + if (_done || _type == Type.Empty) + { + return false; + } + else if (_type == Type.SingleItem) + { + _done = true; + _current = (GraphItem)_entry._storage!; + + return true; + } + else + { + bool result = _setEnumerator.MoveNext(); + _current = _setEnumerator.Current; + + return result; + } + } + + public void Dispose() { - if (_storage is null) - return Enumerable.Empty>(); - if (_storage is GraphItem item) - return new[] { item }; - return (HashSet>)_storage; + if (_type == Type.MultipleItems) + { + _setEnumerator.Dispose(); + } + } + + void IEnumerator.Reset() + { + if (_type == Type.SingleItem) + { + _done = false; + } + else + { + ((IEnumerator)_setEnumerator).Reset(); + } + + _current = default!; } } } diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index 667c6e43011..fd6e49b3e17 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -19,6 +19,58 @@ namespace NuGet.DependencyResolver { public class RemoteDependencyWalker { + /// + /// Captures state to begin or resume processing of a GraphNode + /// + private readonly struct GraphNodeStackState + { + /// + /// The that is currently being processed. + /// + public readonly GraphNode GraphNode; + + /// + /// The dependencies of the current that will be updated as a final step. + /// + public readonly LightweightList> Dependencies; + + /// + /// Where we are when processing dependencies. Also used to flag when we are done. + /// + public readonly int DependencyIndex; + + /// + /// The for the current . + /// + public readonly LibraryRange LibraryRange; + + /// + /// The for the current . + /// + public readonly GraphEdge OuterEdge; + + /// + /// A indicating parent node status for the current . + /// + public readonly bool HasParentNodes; + + public GraphNodeStackState( + GraphNode graphNode, + LightweightList> unprocessedDependencies, + int dependencyIndex, + LibraryRange libraryRange, + GraphEdge outerEdge, + bool hasParentNodes) + { + GraphNode = graphNode; + Dependencies = unprocessedDependencies; + DependencyIndex = dependencyIndex; + LibraryRange = libraryRange; + OuterEdge = outerEdge; + HasParentNodes = hasParentNodes; + } + } + private readonly RemoteWalkContext _context; public RemoteDependencyWalker(RemoteWalkContext context) @@ -74,51 +126,42 @@ private async ValueTask> CreateGraphNodeAsync( GraphEdge outerEdge, TransitiveCentralPackageVersions transitiveCentralPackageVersions, bool hasParentNodes) - { - HashSet runtimeDependencies = null; - if (runtimeGraph != null && !string.IsNullOrEmpty(runtimeName)) - { - EvaluateRuntimeDependencies(ref libraryRange, runtimeName, runtimeGraph, ref runtimeDependencies); - } - - // Resolve the dependency from the cache or sources - GraphItem item = await ResolverUtility.FindLibraryCachedAsync( - _context.FindLibraryEntryCache, + { + // PERF: Since this method is so heavily called for more complex graphs, we need to handle the stack state ourselves to avoid repeated + // async state machine allocations. The stack object captures the state needed to restore the current "frame" so we can emulate the + // recursive calls. + var stackStates = new Stack(); + + GraphNode rootNode = await InitializeGraphNodeAsync(_context, libraryRange, framework, runtimeName, runtimeGraph, hasParentNodes); + var rootTasks = new LightweightList>(rootNode.Item.Data.Dependencies.Count); + + stackStates.Push(new GraphNodeStackState( + rootNode, + rootTasks, + 0, libraryRange, - framework, - runtimeName, - _context, - CancellationToken.None); + outerEdge, + hasParentNodes)); - bool hasInnerNodes = (item.Data.Dependencies.Count + (runtimeDependencies == null ? 0 : runtimeDependencies.Count)) > 0; - GraphNode node = new GraphNode(libraryRange, hasInnerNodes, hasParentNodes) + while (stackStates.Count > 0) { - Item = item - }; + // Restore the state for the current "frame" + GraphNodeStackState currentState = stackStates.Pop(); - Debug.Assert(node.Item != null, "FindLibraryCached should return an unresolved item instead of null"); - MergeRuntimeDependencies(runtimeDependencies, node); - - LightweightList>> tasks = EvaluateDependencies(libraryRange, framework, runtimeName, runtimeGraph, predicate, outerEdge, transitiveCentralPackageVersions, node); - - node.EnsureInnerNodeCapacity(tasks.Count); - foreach (var task in tasks) - { - var dependencyNode = await task; - dependencyNode.OuterNode = node; - node.InnerNodes.Add(dependencyNode); - } + LibraryRange currentLibraryRange = currentState.LibraryRange; + GraphEdge currentOuterEdge = currentState.OuterEdge; + bool currentHasParentNodes = currentState.HasParentNodes; - return node; + GraphNode node = currentState.GraphNode; + LightweightList> dependencies = currentState.Dependencies; - LightweightList>> EvaluateDependencies(LibraryRange libraryRange, NuGetFramework framework, string runtimeName, RuntimeGraph runtimeGraph, Func predicate, GraphEdge outerEdge, TransitiveCentralPackageVersions transitiveCentralPackageVersions, GraphNode node) - { - var tasks = new LightweightList>>(); // do not add nodes for all the centrally managed package versions to the graph // they will be added only if they are transitive - foreach (var dependency in node.Item.Data.Dependencies) + int index; + for (index = currentState.DependencyIndex; index < node.Item.Data.Dependencies.Count; index++) { + LibraryDependency dependency = node.Item.Data.Dependencies[index]; if (!IsDependencyValidForGraph(dependency)) { continue; @@ -126,14 +169,14 @@ LightweightList>> EvaluateDependencies( // Skip dependencies if the dependency edge has 'all' excluded and // the node is not a direct dependency. - if (outerEdge == null + if (currentOuterEdge == null || dependency.SuppressParent != LibraryIncludeFlags.All) { - var result = WalkParentsAndCalculateDependencyResult(outerEdge, dependency, predicate); + var result = WalkParentsAndCalculateDependencyResult(currentOuterEdge, dependency, predicate); // Check for a cycle, this is needed for A (project) -> A (package) // since the predicate will not be called for leaf nodes. - if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, libraryRange.Name)) + if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, currentLibraryRange.Name)) { result = (DependencyResult.Cycle, dependency); } @@ -141,17 +184,36 @@ LightweightList>> EvaluateDependencies( if (result.dependencyResult == DependencyResult.Acceptable) { // Dependency edge from the current node to the dependency - var innerEdge = new GraphEdge(outerEdge, node.Item, dependency); - - tasks.Add(CreateGraphNodeAsync( - dependency.LibraryRange, - framework, - runtimeName, - runtimeGraph, - predicate, + var innerEdge = new GraphEdge(currentOuterEdge, node.Item, dependency); + + var dependencyLibraryRange = dependency.LibraryRange; + + GraphNode newNode = await InitializeGraphNodeAsync(_context, dependencyLibraryRange, framework, runtimeName, runtimeGraph, false); + + dependencies.Add(newNode); + + // put parent node back on stack to either resume processing (index + 1) < node.Item.Data.Dependencies.Count + // or skip over the loop (index + 1 >= node.Item.Data.Dependencies.Count) and update Inner/Outer nodes. + stackStates.Push(new GraphNodeStackState( + node, + dependencies, + index + 1, + currentState.LibraryRange, + currentState.OuterEdge, + currentState.HasParentNodes)); + + var newNodeDependencies = new LightweightList>(newNode.Item.Data.Dependencies.Count); + // We have a new dependency that we need to evaluate. Push necessary state onto the stack so it can be evaluated. + stackStates.Push(new GraphNodeStackState( + newNode, + newNodeDependencies, + 0, + dependencyLibraryRange, innerEdge, - transitiveCentralPackageVersions, hasParentNodes: false)); + + // leave current loop to evaluate latest dependency. + break; } else { @@ -174,48 +236,86 @@ LightweightList>> EvaluateDependencies( OuterNode = node }; + node.EnsureInnerNodeCapacity(node.Item.Data.Dependencies.Count - index); node.InnerNodes.Add(dependencyNode); } } } } - return tasks; + // Once we finish processing all dependencies for the current node, + // we update the pointers. + if (index >= node.Item.Data.Dependencies.Count) + { + node.EnsureInnerNodeCapacity(dependencies.Count); + foreach (var dependency in dependencies) + { + dependency.OuterNode = node; + node.InnerNodes.Add(dependency); + } + } } - static void EvaluateRuntimeDependencies(ref LibraryRange libraryRange, string runtimeName, RuntimeGraph runtimeGraph, ref HashSet runtimeDependencies) + return rootNode; + + static async ValueTask> InitializeGraphNodeAsync(RemoteWalkContext context, LibraryRange libraryRange, NuGetFramework framework, string runtimeName, RuntimeGraph runtimeGraph, bool hasParentNodes) { - // HACK(davidfowl): This is making runtime.json support package redirects + HashSet runtimeDependencies = null; - // Look up any additional dependencies for this package - foreach (var runtimeDependency in runtimeGraph.FindRuntimeDependencies(runtimeName, libraryRange.Name).NoAllocEnumerate()) + if (runtimeGraph != null && !string.IsNullOrEmpty(runtimeName)) { - var libraryDependency = new LibraryDependency(noWarn: Array.Empty()) + // HACK(davidfowl): This is making runtime.json support package redirects + + // Look up any additional dependencies for this package + foreach (var runtimeDependency in runtimeGraph.FindRuntimeDependencies(runtimeName, libraryRange.Name).NoAllocEnumerate()) { - LibraryRange = new LibraryRange() + var libraryDependency = new LibraryDependency(noWarn: Array.Empty()) { - Name = runtimeDependency.Id, - VersionRange = runtimeDependency.VersionRange, - TypeConstraint = LibraryDependencyTarget.PackageProjectExternal - } - }; + LibraryRange = new LibraryRange() + { + Name = runtimeDependency.Id, + VersionRange = runtimeDependency.VersionRange, + TypeConstraint = LibraryDependencyTarget.PackageProjectExternal + } + }; - if (StringComparer.OrdinalIgnoreCase.Equals(runtimeDependency.Id, libraryRange.Name)) - { - if (libraryRange.VersionRange != null && - runtimeDependency.VersionRange != null && - libraryRange.VersionRange.MinVersion < runtimeDependency.VersionRange.MinVersion) + if (StringComparer.OrdinalIgnoreCase.Equals(runtimeDependency.Id, libraryRange.Name)) { - libraryRange = libraryDependency.LibraryRange; + if (libraryRange.VersionRange != null && + runtimeDependency.VersionRange != null && + libraryRange.VersionRange.MinVersion < runtimeDependency.VersionRange.MinVersion) + { + libraryRange = libraryDependency.LibraryRange; + } + } + else + { + // Otherwise it's a dependency of this node + runtimeDependencies ??= new HashSet(LibraryDependencyNameComparer.OrdinalIgnoreCaseNameComparer); + runtimeDependencies.Add(libraryDependency); } - } - else - { - // Otherwise it's a dependency of this node - runtimeDependencies ??= new HashSet(LibraryDependencyNameComparer.OrdinalIgnoreCaseNameComparer); - runtimeDependencies.Add(libraryDependency); } } + + // Resolve the dependency from the cache or sources + GraphItem rootItem = await ResolverUtility.FindLibraryCachedAsync( + context.FindLibraryEntryCache, + libraryRange, + framework, + runtimeName, + context, + CancellationToken.None); + + bool rootHasInnerNodes = (rootItem.Data.Dependencies.Count + (runtimeDependencies == null ? 0 : runtimeDependencies.Count)) > 0; + GraphNode node = new GraphNode(libraryRange, hasInnerNodes: rootHasInnerNodes, hasParentNodes: hasParentNodes) + { + Item = rootItem + }; + + Debug.Assert(node.Item != null, "FindLibraryCached should return an unresolved item instead of null"); + MergeRuntimeDependencies(runtimeDependencies, node); + + return node; } static void MergeRuntimeDependencies(HashSet runtimeDependencies, GraphNode node) @@ -223,9 +323,18 @@ static void MergeRuntimeDependencies(HashSet runtimeDependenc // Merge in runtime dependencies if (runtimeDependencies?.Count > 0) { + var newDependencies = new List(runtimeDependencies.Count + node.Item.Data.Dependencies.Count); foreach (var nodeDep in node.Item.Data.Dependencies) { - runtimeDependencies.Add(nodeDep); + if (!runtimeDependencies.Contains(nodeDep)) + { + newDependencies.Add(nodeDep); + } + } + + foreach (var runtimeDependency in runtimeDependencies) + { + newDependencies.Add(runtimeDependency); } // Create a new item on this node so that we can update it with the new dependencies from @@ -235,7 +344,7 @@ static void MergeRuntimeDependencies(HashSet runtimeDependenc { Data = new RemoteResolveResult() { - Dependencies = runtimeDependencies.ToList(), + Dependencies = newDependencies, Match = node.Item.Data.Match } }; @@ -561,6 +670,7 @@ internal struct LightweightList { private const int Fields = 10; private int _count; + private int _expectedCapacity; private T _firstItem; private T _secondItem; private T _thirdItem; @@ -576,6 +686,11 @@ internal struct LightweightList public readonly int Count => _count; + public LightweightList(int expectedCapacity) + { + _expectedCapacity = expectedCapacity; + } + public void Add(T task) { if (_count == 0) @@ -620,7 +735,20 @@ public void Add(T task) } else { - _additionalItems ??= new List(); + if (_additionalItems == null) + { + var listCapacity = _expectedCapacity - Fields; + if (listCapacity > 0) + { + _additionalItems = new List(listCapacity); + } + else + { + const int defaultListSize = 4; + _additionalItems = new List(defaultListSize); + } + } + _additionalItems.Add(task); } diff --git a/src/NuGet.Core/NuGet.Frameworks/FrameworkNameProvider.cs b/src/NuGet.Core/NuGet.Frameworks/FrameworkNameProvider.cs index e739a0556dc..e32ce6b14fe 100644 --- a/src/NuGet.Core/NuGet.Frameworks/FrameworkNameProvider.cs +++ b/src/NuGet.Core/NuGet.Frameworks/FrameworkNameProvider.cs @@ -405,16 +405,21 @@ private IEnumerable> GetEquivalentPermutations(HashSet(); - // include ourselves - equalFrameworks.Add(current!); + HashSet equalFrameworks; // find all equivalent frameworks for the current one if (_equivalentFrameworks.TryGetValue(current!, out HashSet? curFrameworks)) { - UnionWith(equalFrameworks, curFrameworks); + equalFrameworks = new HashSet(curFrameworks); + } + else + { + equalFrameworks = new HashSet(); } + // include ourselves + equalFrameworks.Add(current!); + foreach (var fw in equalFrameworks) { if (remaining != null && remaining.Count > 0) diff --git a/src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs b/src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs index 2adb3568b9e..015c0a89bfb 100644 --- a/src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs +++ b/src/NuGet.Core/NuGet.Packaging/ContentModel/ManagedCodeConventions.cs @@ -137,7 +137,15 @@ private static object CodeLanguage_Parser(string name, PatternTable table) } // Code language values must be alpha numeric. - return name.All(c => char.IsLetterOrDigit(c)) ? name : null; + // PERF: use foreach to avoid CharEnumerator allocation + foreach (char c in name) + { + if (!char.IsLetterOrDigit(c)) + { + return null; + } + } + return name; } private static object Locale_Parser(string name, PatternTable table) diff --git a/src/NuGet.Core/NuGet.Packaging/RuntimeModel/RuntimeGraph.cs b/src/NuGet.Core/NuGet.Packaging/RuntimeModel/RuntimeGraph.cs index 715ea602c4d..535da45c928 100644 --- a/src/NuGet.Core/NuGet.Packaging/RuntimeModel/RuntimeGraph.cs +++ b/src/NuGet.Core/NuGet.Packaging/RuntimeModel/RuntimeGraph.cs @@ -270,17 +270,22 @@ public IEnumerable FindRuntimeDependencies(string runt { var key = new RuntimeDependencyKey(runtimeName, packageId); - return _dependencyCache!.GetOrAdd(key, FindRuntimeDependenciesInternal); +#if NET472_OR_GREATER || NET5_0_OR_GREATER + + return _dependencyCache!.GetOrAdd(key, FindRuntimeDependenciesInternal, this); +#else + return _dependencyCache!.GetOrAdd(key, key => FindRuntimeDependenciesInternal(key, this)); +#endif } return Enumerable.Empty(); - List FindRuntimeDependenciesInternal(RuntimeDependencyKey key) + static List FindRuntimeDependenciesInternal(RuntimeDependencyKey key, RuntimeGraph runtimeGraph) { // Find all compatible RIDs - foreach (var expandedRuntime in ExpandRuntimeCached(key.RuntimeName)) + foreach (var expandedRuntime in runtimeGraph.ExpandRuntimeCached(key.RuntimeName)) { - if (Runtimes.TryGetValue(expandedRuntime, out RuntimeDescription? runtimeDescription)) + if (runtimeGraph.Runtimes.TryGetValue(expandedRuntime, out RuntimeDescription? runtimeDescription)) { if (runtimeDescription.RuntimeDependencySets.TryGetValue(key.PackageId, out var dependencySet)) { diff --git a/src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs b/src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs index 21db49e906a..08cb1eb8326 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs @@ -65,7 +65,7 @@ internal static void Write(PackageSpec packageSpec, IObjectWriter writer, bool h #pragma warning restore CS0612 // Type or member is obsolete - if (packageSpec.Dependencies.Any()) + if (packageSpec.Dependencies.Count > 0) { SetDependencies(writer, packageSpec.Dependencies); } @@ -553,7 +553,7 @@ internal static void SetCentralTransitveDependencyGroup(IObjectWriter writer, st private static void SetImports(IObjectWriter writer, IList frameworks) { - if (frameworks?.Any() == true) + if (frameworks != null && frameworks.Count > 0) { var imports = frameworks.Select(framework => framework.GetShortFolderName()); From 84a4b288f97ca7edc31d60df2d1b4ff486a6b16f Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Fri, 9 Feb 2024 12:22:50 -0800 Subject: [PATCH 2/7] WIP --- .../SourceRepositoryDependencyProvider.cs | 6 +- .../Remote/RemoteDependencyWalker.cs | 506 +++++++++++------- 2 files changed, 324 insertions(+), 188 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs index ad9e71c9c9c..368ea7b9890 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/SourceRepositoryDependencyProvider.cs @@ -231,7 +231,7 @@ Task FindLibraryIdentityAsync(LibraryRange libraryRange, Source { lock (_libraryMatchCache) { - result = FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken); + result = Task.Run(() => FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken), cancellationToken); _libraryMatchCache[libraryRange] = result; } } @@ -241,7 +241,7 @@ Task FindLibraryIdentityAsync(LibraryRange libraryRange, Source { if (!_libraryMatchCache.TryGetValue(libraryRange, out result)) { - result = FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken); + result = Task.Run(() => FindLibraryCoreAsync(libraryRange, cacheContext, logger, cancellationToken), cancellationToken); _libraryMatchCache[libraryRange] = result; } } @@ -257,7 +257,6 @@ private async Task FindLibraryCoreAsync( ILogger logger, CancellationToken cancellationToken) { - await Task.Yield(); await EnsureResource(); if (libraryRange.VersionRange?.MinVersion != null && libraryRange.VersionRange.IsMinInclusive && !libraryRange.VersionRange.IsFloating) @@ -401,7 +400,6 @@ private async Task GetDependenciesCoreAsync( FindPackageByIdDependencyInfo packageInfo = null; try { - await Task.Yield(); await EnsureResource(); if (_throttle != null) diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index fd6e49b3e17..33ee10598d2 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -19,58 +19,6 @@ namespace NuGet.DependencyResolver { public class RemoteDependencyWalker { - /// - /// Captures state to begin or resume processing of a GraphNode - /// - private readonly struct GraphNodeStackState - { - /// - /// The that is currently being processed. - /// - public readonly GraphNode GraphNode; - - /// - /// The dependencies of the current that will be updated as a final step. - /// - public readonly LightweightList> Dependencies; - - /// - /// Where we are when processing dependencies. Also used to flag when we are done. - /// - public readonly int DependencyIndex; - - /// - /// The for the current . - /// - public readonly LibraryRange LibraryRange; - - /// - /// The for the current . - /// - public readonly GraphEdge OuterEdge; - - /// - /// A indicating parent node status for the current . - /// - public readonly bool HasParentNodes; - - public GraphNodeStackState( - GraphNode graphNode, - LightweightList> unprocessedDependencies, - int dependencyIndex, - LibraryRange libraryRange, - GraphEdge outerEdge, - bool hasParentNodes) - { - GraphNode = graphNode; - Dependencies = unprocessedDependencies; - DependencyIndex = dependencyIndex; - LibraryRange = libraryRange; - OuterEdge = outerEdge; - HasParentNodes = hasParentNodes; - } - } - private readonly RemoteWalkContext _context; public RemoteDependencyWalker(RemoteWalkContext context) @@ -133,12 +81,36 @@ private async ValueTask> CreateGraphNodeAsync( // recursive calls. var stackStates = new Stack(); - GraphNode rootNode = await InitializeGraphNodeAsync(_context, libraryRange, framework, runtimeName, runtimeGraph, hasParentNodes); - var rootTasks = new LightweightList>(rootNode.Item.Data.Dependencies.Count); + HashSet rootRuntimeDependencies = null; + + if (runtimeGraph != null && !string.IsNullOrEmpty(runtimeName)) + { + EvaluateRuntimeDependencies(ref libraryRange, runtimeName, runtimeGraph, ref rootRuntimeDependencies); + } + + var rootItem = await ResolverUtility.FindLibraryCachedAsync( + _context.FindLibraryEntryCache, + libraryRange, + framework, + runtimeName, + _context, + CancellationToken.None); + + bool rootHasInnerNodes = (rootItem.Data.Dependencies.Count + (rootRuntimeDependencies == null ? 0 : rootRuntimeDependencies.Count)) > 0; + GraphNode rootNode = new GraphNode(libraryRange, rootHasInnerNodes, hasParentNodes) + { + Item = rootItem + }; + + LightweightList rootDependencies = new LightweightList(rootNode.Item.Data.Dependencies.Count); + + Debug.Assert(rootNode.Item != null, "FindLibraryCached should return an unresolved item instead of null"); + MergeRuntimeDependencies(rootRuntimeDependencies, rootNode); stackStates.Push(new GraphNodeStackState( rootNode, - rootTasks, + null, + rootDependencies, 0, libraryRange, outerEdge, @@ -148,174 +120,185 @@ private async ValueTask> CreateGraphNodeAsync( { // Restore the state for the current "frame" GraphNodeStackState currentState = stackStates.Pop(); - + GraphNode node = currentState.GraphNode; + LightweightList dependencyNodeCreationData = currentState.DependencyData; LibraryRange currentLibraryRange = currentState.LibraryRange; GraphEdge currentOuterEdge = currentState.OuterEdge; bool currentHasParentNodes = currentState.HasParentNodes; - GraphNode node = currentState.GraphNode; - LightweightList> dependencies = currentState.Dependencies; + int index = currentState.DependencyIndex; - // do not add nodes for all the centrally managed package versions to the graph - // they will be added only if they are transitive - int index; - for (index = currentState.DependencyIndex; index < node.Item.Data.Dependencies.Count; index++) + // When index is 0, this is the first time we're starting to process this node. We want to schedule any async work needed to resolve the + // current node's dependencies so that long-running operations can happen in parallel. + if (index == 0) { - LibraryDependency dependency = node.Item.Data.Dependencies[index]; - if (!IsDependencyValidForGraph(dependency)) + // do not add nodes for all the centrally managed package versions to the graph + // they will be added only if they are transitive + for (var i = 0; i < node.Item.Data.Dependencies.Count; i++) { - continue; - } - - // Skip dependencies if the dependency edge has 'all' excluded and - // the node is not a direct dependency. - if (currentOuterEdge == null - || dependency.SuppressParent != LibraryIncludeFlags.All) - { - var result = WalkParentsAndCalculateDependencyResult(currentOuterEdge, dependency, predicate); - - // Check for a cycle, this is needed for A (project) -> A (package) - // since the predicate will not be called for leaf nodes. - if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, currentLibraryRange.Name)) + LibraryDependency dependency = node.Item.Data.Dependencies[i]; + if (!IsDependencyValidForGraph(dependency)) { - result = (DependencyResult.Cycle, dependency); + continue; } - if (result.dependencyResult == DependencyResult.Acceptable) + // Skip dependencies if the dependency edge has 'all' excluded and + // the node is not a direct dependency. + if (currentOuterEdge == null + || dependency.SuppressParent != LibraryIncludeFlags.All) { - // Dependency edge from the current node to the dependency - var innerEdge = new GraphEdge(currentOuterEdge, node.Item, dependency); - - var dependencyLibraryRange = dependency.LibraryRange; - - GraphNode newNode = await InitializeGraphNodeAsync(_context, dependencyLibraryRange, framework, runtimeName, runtimeGraph, false); - - dependencies.Add(newNode); - - // put parent node back on stack to either resume processing (index + 1) < node.Item.Data.Dependencies.Count - // or skip over the loop (index + 1 >= node.Item.Data.Dependencies.Count) and update Inner/Outer nodes. - stackStates.Push(new GraphNodeStackState( - node, - dependencies, - index + 1, - currentState.LibraryRange, - currentState.OuterEdge, - currentState.HasParentNodes)); - - var newNodeDependencies = new LightweightList>(newNode.Item.Data.Dependencies.Count); - // We have a new dependency that we need to evaluate. Push necessary state onto the stack so it can be evaluated. - stackStates.Push(new GraphNodeStackState( - newNode, - newNodeDependencies, - 0, - dependencyLibraryRange, - innerEdge, - hasParentNodes: false)); + var result = WalkParentsAndCalculateDependencyResult(currentOuterEdge, dependency, predicate); - // leave current loop to evaluate latest dependency. - break; - } - else - { - // In case of conflict because of a centrally managed version that is not direct dependency - // the centrally managed package versions need to be added to the graph explicitly as they are not added otherwise - if (result.conflictingDependency != null && - result.conflictingDependency.VersionCentrallyManaged && - result.conflictingDependency.ReferenceType == LibraryDependencyReferenceType.None) + // Check for a cycle, this is needed for A (project) -> A (package) + // since the predicate will not be called for leaf nodes. + if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, currentLibraryRange.Name)) { - MarkCentralVersionForTransitiveProcessing(result.conflictingDependency, transitiveCentralPackageVersions, node); + result = (DependencyResult.Cycle, dependency); } - // Keep the node in the tree if we need to look at it later - if (result.dependencyResult == DependencyResult.PotentiallyDowngraded || - result.dependencyResult == DependencyResult.Cycle) + if (result.dependencyResult == DependencyResult.Acceptable) { - var dependencyNode = new GraphNode(dependency.LibraryRange) + // Dependency edge from the current node to the dependency + var innerEdge = new GraphEdge(currentOuterEdge, node.Item, dependency); + + var dependencyLibraryRange = dependency.LibraryRange; + + HashSet runtimeDependencies = null; + + if (runtimeGraph != null && !string.IsNullOrEmpty(runtimeName)) { - Disposition = result.dependencyResult == DependencyResult.Cycle ? Disposition.Cycle : Disposition.PotentiallyDowngraded, - OuterNode = node - }; + EvaluateRuntimeDependencies(ref dependencyLibraryRange, runtimeName, runtimeGraph, ref runtimeDependencies); + } - node.EnsureInnerNodeCapacity(node.Item.Data.Dependencies.Count - index); - node.InnerNodes.Add(dependencyNode); + var newGraphItemTask = ResolverUtility.FindLibraryCachedAsync( + _context.FindLibraryEntryCache, + dependencyLibraryRange, + framework, + runtimeName, + _context, + CancellationToken.None); + + // store all the data needed to construct this dependency. The library resolution may take a long time to resolve, so we just want to start that operation. + var graphNodeCreationData = new GraphNodeCreationData(newGraphItemTask, runtimeDependencies, dependencyLibraryRange, innerEdge, false); + dependencyNodeCreationData.Add(graphNodeCreationData); + } + else + { + // In case of conflict because of a centrally managed version that is not direct dependency + // the centrally managed package versions need to be added to the graph explicitly as they are not added otherwise + if (result.conflictingDependency != null && + result.conflictingDependency.VersionCentrallyManaged && + result.conflictingDependency.ReferenceType == LibraryDependencyReferenceType.None) + { + MarkCentralVersionForTransitiveProcessing(result.conflictingDependency, transitiveCentralPackageVersions, node); + } + + // Keep the node in the tree if we need to look at it later + if (result.dependencyResult == DependencyResult.PotentiallyDowngraded || + result.dependencyResult == DependencyResult.Cycle) + { + var dependencyNode = new GraphNode(dependency.LibraryRange) + { + Disposition = result.dependencyResult == DependencyResult.Cycle ? Disposition.Cycle : Disposition.PotentiallyDowngraded, + OuterNode = node + }; + + node.EnsureInnerNodeCapacity(node.Item.Data.Dependencies.Count - i); + node.InnerNodes.Add(dependencyNode); + } } } } + + // We know we'll need capacity for exactly this many items in the next block. We set it here once to avoid repeated checks. + node.EnsureInnerNodeCapacity(dependencyNodeCreationData.Count); } - // Once we finish processing all dependencies for the current node, - // we update the pointers. - if (index >= node.Item.Data.Dependencies.Count) + // This block evaluates one dependency at a time and index keeps track of which dependency we need to evaluate. + if (index < dependencyNodeCreationData.Count) { - node.EnsureInnerNodeCapacity(dependencies.Count); - foreach (var dependency in dependencies) + // It's time to actually construct the GraphNode that represents this dependency, so we need to wait for the GraphItem to be resolved. + GraphNodeCreationData graphNodeCreationData = dependencyNodeCreationData[index]; + var dependencyItem = await graphNodeCreationData.GraphItemTask; + + bool hasInnerNodes = (dependencyItem.Data.Dependencies.Count + (graphNodeCreationData.RuntimeDependencies == null ? 0 : graphNodeCreationData.RuntimeDependencies.Count)) > 0; + GraphNode newNode = new GraphNode(graphNodeCreationData.LibraryRange, hasInnerNodes, graphNodeCreationData.HasParentNodes) { - dependency.OuterNode = node; - node.InnerNodes.Add(dependency); - } + Item = dependencyItem + }; + + Debug.Assert(newNode.Item != null, "FindLibraryCached should return an unresolved item instead of null"); + MergeRuntimeDependencies(graphNodeCreationData.RuntimeDependencies, newNode); + + node.InnerNodes.Add(newNode); + + // We want to update the connections starting from the leaves of the graph and working up to the root, so we advance the evaluation of the current + // node (index + 1) and push that state onto our stack for future evaluation. If we're done processing all the dependencies, index will be >= dependencyNodeCreationData.Count + // on the next iteration which will let us enter the final block. + stackStates.Push(new GraphNodeStackState( + node, + currentState.ParentNode, + dependencyNodeCreationData, + index + 1, + currentState.LibraryRange, + currentState.OuterEdge, + currentState.HasParentNodes)); + + LightweightList newDependencies = new LightweightList(newNode.Item.Data.Dependencies.Count); + + // We have a new dependency that we need to evaluate before its parent, so we push it onto the stack after the parent. + stackStates.Push(new GraphNodeStackState( + newNode, + node, + newDependencies, + 0, + graphNodeCreationData.LibraryRange, + graphNodeCreationData.OuterEdge, + graphNodeCreationData.HasParentNodes)); + } + + // This block does the final edge connection after all dependencies are fully evaluated. + if (index >= dependencyNodeCreationData.Count) + { + node.OuterNode = currentState.ParentNode; } } return rootNode; - static async ValueTask> InitializeGraphNodeAsync(RemoteWalkContext context, LibraryRange libraryRange, NuGetFramework framework, string runtimeName, RuntimeGraph runtimeGraph, bool hasParentNodes) + static void EvaluateRuntimeDependencies(ref LibraryRange libraryRange, string runtimeName, RuntimeGraph runtimeGraph, ref HashSet runtimeDependencies) { - HashSet runtimeDependencies = null; + // HACK(davidfowl): This is making runtime.json support package redirects - if (runtimeGraph != null && !string.IsNullOrEmpty(runtimeName)) + // Look up any additional dependencies for this package + foreach (var runtimeDependency in runtimeGraph.FindRuntimeDependencies(runtimeName, libraryRange.Name).NoAllocEnumerate()) { - // HACK(davidfowl): This is making runtime.json support package redirects - - // Look up any additional dependencies for this package - foreach (var runtimeDependency in runtimeGraph.FindRuntimeDependencies(runtimeName, libraryRange.Name).NoAllocEnumerate()) + var libraryDependency = new LibraryDependency(noWarn: Array.Empty()) { - var libraryDependency = new LibraryDependency(noWarn: Array.Empty()) + LibraryRange = new LibraryRange() { - LibraryRange = new LibraryRange() - { - Name = runtimeDependency.Id, - VersionRange = runtimeDependency.VersionRange, - TypeConstraint = LibraryDependencyTarget.PackageProjectExternal - } - }; - - if (StringComparer.OrdinalIgnoreCase.Equals(runtimeDependency.Id, libraryRange.Name)) - { - if (libraryRange.VersionRange != null && - runtimeDependency.VersionRange != null && - libraryRange.VersionRange.MinVersion < runtimeDependency.VersionRange.MinVersion) - { - libraryRange = libraryDependency.LibraryRange; - } + Name = runtimeDependency.Id, + VersionRange = runtimeDependency.VersionRange, + TypeConstraint = LibraryDependencyTarget.PackageProjectExternal } - else + }; + + if (StringComparer.OrdinalIgnoreCase.Equals(runtimeDependency.Id, libraryRange.Name)) + { + if (libraryRange.VersionRange != null && + runtimeDependency.VersionRange != null && + libraryRange.VersionRange.MinVersion < runtimeDependency.VersionRange.MinVersion) { - // Otherwise it's a dependency of this node - runtimeDependencies ??= new HashSet(LibraryDependencyNameComparer.OrdinalIgnoreCaseNameComparer); - runtimeDependencies.Add(libraryDependency); + libraryRange = libraryDependency.LibraryRange; } } + else + { + // Otherwise it's a dependency of this node + runtimeDependencies ??= new HashSet(LibraryDependencyNameComparer.OrdinalIgnoreCaseNameComparer); + runtimeDependencies.Add(libraryDependency); + } } - - // Resolve the dependency from the cache or sources - GraphItem rootItem = await ResolverUtility.FindLibraryCachedAsync( - context.FindLibraryEntryCache, - libraryRange, - framework, - runtimeName, - context, - CancellationToken.None); - - bool rootHasInnerNodes = (rootItem.Data.Dependencies.Count + (runtimeDependencies == null ? 0 : runtimeDependencies.Count)) > 0; - GraphNode node = new GraphNode(libraryRange, hasInnerNodes: rootHasInnerNodes, hasParentNodes: hasParentNodes) - { - Item = rootItem - }; - - Debug.Assert(node.Item != null, "FindLibraryCached should return an unresolved item instead of null"); - MergeRuntimeDependencies(runtimeDependencies, node); - - return node; } static void MergeRuntimeDependencies(HashSet runtimeDependencies, GraphNode node) @@ -664,13 +647,112 @@ public int GetHashCode(LibraryDependency obj) return StringComparer.OrdinalIgnoreCase.GetHashCode(obj.Name); } } + + /// + /// Captures state to begin or resume processing of a GraphNode + /// + private readonly struct GraphNodeStackState + { + /// + /// The that is currently being processed. + /// + public readonly GraphNode GraphNode; + + /// + /// A reference to the parent . + /// + public readonly GraphNode ParentNode; + + /// + /// The dependencies of the current that will be updated as a final step. + /// + public readonly LightweightList DependencyData; + + /// + /// Where we are when processing dependencies. Also used to flag where we are in processing the current . + /// + public readonly int DependencyIndex; + + /// + /// The for the current . + /// + public readonly LibraryRange LibraryRange; + + /// + /// The for the current . + /// + public readonly GraphEdge OuterEdge; + + /// + /// A indicating parent node status for the current . + /// + public readonly bool HasParentNodes; + + public GraphNodeStackState( + GraphNode graphNode, + GraphNode parentNode, + LightweightList dependencies, + int dependencyIndex, + LibraryRange libraryRange, + GraphEdge outerEdge, + bool hasParentNodes) + { + GraphNode = graphNode; + ParentNode = parentNode; + DependencyData = dependencies; + DependencyIndex = dependencyIndex; + LibraryRange = libraryRange; + OuterEdge = outerEdge; + HasParentNodes = hasParentNodes; + } + } + + /// + /// Stores data that is required to create a for later use. + /// + private readonly struct GraphNodeCreationData + { + /// + /// A that represents the retrieval of the necessary to complete construction of the . + /// + public readonly Task> GraphItemTask; + + /// + /// The set of items used during creation. + /// + public readonly HashSet RuntimeDependencies; + + /// + /// The of this to construct. + /// + public readonly LibraryRange LibraryRange; + + /// + /// Edge pointing to the parent . + /// + public readonly GraphEdge OuterEdge; + + /// + /// Indicates if this node should have parent nodes. + /// + public readonly bool HasParentNodes; + + public GraphNodeCreationData(Task> graphItemTask, HashSet runtimeDependencies, LibraryRange libraryRange, GraphEdge outerEdge, bool hasParentNodes) + { + GraphItemTask = graphItemTask; + RuntimeDependencies = runtimeDependencies; + LibraryRange = libraryRange; + OuterEdge = outerEdge; + HasParentNodes = hasParentNodes; + } + } } internal struct LightweightList { private const int Fields = 10; + private readonly int _expectedCapacity; private int _count; - private int _expectedCapacity; private T _firstItem; private T _secondItem; private T _thirdItem; @@ -691,6 +773,62 @@ public LightweightList(int expectedCapacity) _expectedCapacity = expectedCapacity; } + public readonly T this[int index] + { + get + { + if ((uint)index >= (uint)_count) + { + throw new ArgumentOutOfRangeException(nameof(index)); + } + + if (index == 0) + { + return _firstItem; + } + else if (index == 1) + { + return _secondItem; + } + else if (index == 2) + { + return _thirdItem; + } + else if (index == 3) + { + return _fourthItem; + } + else if (index == 4) + { + return _fifthItem; + } + else if (index == 5) + { + return _sixthItem; + } + else if (index == 6) + { + return _seventhItem; + } + else if (index == 7) + { + return _eighthItem; + } + else if (index == 8) + { + return _ninthItem; + } + else if (index == 9) + { + return _tenthItem; + } + else + { + return _additionalItems[index - Fields]; + } + } + } + public void Add(T task) { if (_count == 0) From a3ce5ab099300674ba96c078ccc4a752771f30e8 Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Tue, 13 Feb 2024 10:34:54 -0800 Subject: [PATCH 3/7] Add tests --- .../TrackerTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/NuGet.Core.Tests/NuGet.DependencyResolver.Core.Tests/TrackerTests.cs b/test/NuGet.Core.Tests/NuGet.DependencyResolver.Core.Tests/TrackerTests.cs index 1c805c6d4f2..66cf0d3748c 100644 --- a/test/NuGet.Core.Tests/NuGet.DependencyResolver.Core.Tests/TrackerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.DependencyResolver.Core.Tests/TrackerTests.cs @@ -173,6 +173,36 @@ public void IsBestVersion() Assert.True(tracker.IsBestVersion(itemA2)); Assert.True(tracker.IsBestVersion(itemA3)); Assert.True(tracker.IsBestVersion(itemB1)); + + tracker.Clear(); + + tracker.Track(itemB1); + + Assert.True(tracker.IsBestVersion(itemA1)); + Assert.True(tracker.IsBestVersion(itemA2)); + Assert.True(tracker.IsBestVersion(itemA3)); + Assert.True(tracker.IsBestVersion(itemB1)); + + tracker.Track(itemA3); + + Assert.False(tracker.IsBestVersion(itemA1)); + Assert.False(tracker.IsBestVersion(itemA2)); + Assert.True(tracker.IsBestVersion(itemA3)); + Assert.True(tracker.IsBestVersion(itemB1)); + + tracker.Track(itemA2); + + Assert.False(tracker.IsBestVersion(itemA1)); + Assert.False(tracker.IsBestVersion(itemA2)); + Assert.True(tracker.IsBestVersion(itemA3)); + Assert.True(tracker.IsBestVersion(itemB1)); + + tracker.Track(itemA1); + + Assert.False(tracker.IsBestVersion(itemA1)); + Assert.False(tracker.IsBestVersion(itemA2)); + Assert.True(tracker.IsBestVersion(itemA3)); + Assert.True(tracker.IsBestVersion(itemB1)); } private static GraphItem MakeItem(string name, int version) From c7095f7386a240030385afe4dc8bff213d2cff03 Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Tue, 20 Feb 2024 14:11:13 -0800 Subject: [PATCH 4/7] Remove redundant "'HasParentNodes" parameter --- .../Remote/RemoteDependencyWalker.cs | 31 +++++-------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index 33ee10598d2..254bd410c18 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -113,8 +113,7 @@ private async ValueTask> CreateGraphNodeAsync( rootDependencies, 0, libraryRange, - outerEdge, - hasParentNodes)); + outerEdge)); while (stackStates.Count > 0) { @@ -124,7 +123,6 @@ private async ValueTask> CreateGraphNodeAsync( LightweightList dependencyNodeCreationData = currentState.DependencyData; LibraryRange currentLibraryRange = currentState.LibraryRange; GraphEdge currentOuterEdge = currentState.OuterEdge; - bool currentHasParentNodes = currentState.HasParentNodes; int index = currentState.DependencyIndex; @@ -179,7 +177,7 @@ private async ValueTask> CreateGraphNodeAsync( CancellationToken.None); // store all the data needed to construct this dependency. The library resolution may take a long time to resolve, so we just want to start that operation. - var graphNodeCreationData = new GraphNodeCreationData(newGraphItemTask, runtimeDependencies, dependencyLibraryRange, innerEdge, false); + var graphNodeCreationData = new GraphNodeCreationData(newGraphItemTask, runtimeDependencies, dependencyLibraryRange, innerEdge); dependencyNodeCreationData.Add(graphNodeCreationData); } else @@ -222,7 +220,7 @@ private async ValueTask> CreateGraphNodeAsync( var dependencyItem = await graphNodeCreationData.GraphItemTask; bool hasInnerNodes = (dependencyItem.Data.Dependencies.Count + (graphNodeCreationData.RuntimeDependencies == null ? 0 : graphNodeCreationData.RuntimeDependencies.Count)) > 0; - GraphNode newNode = new GraphNode(graphNodeCreationData.LibraryRange, hasInnerNodes, graphNodeCreationData.HasParentNodes) + GraphNode newNode = new GraphNode(graphNodeCreationData.LibraryRange, hasInnerNodes, false) { Item = dependencyItem }; @@ -241,8 +239,7 @@ private async ValueTask> CreateGraphNodeAsync( dependencyNodeCreationData, index + 1, currentState.LibraryRange, - currentState.OuterEdge, - currentState.HasParentNodes)); + currentState.OuterEdge)); LightweightList newDependencies = new LightweightList(newNode.Item.Data.Dependencies.Count); @@ -253,8 +250,7 @@ private async ValueTask> CreateGraphNodeAsync( newDependencies, 0, graphNodeCreationData.LibraryRange, - graphNodeCreationData.OuterEdge, - graphNodeCreationData.HasParentNodes)); + graphNodeCreationData.OuterEdge)); } // This block does the final edge connection after all dependencies are fully evaluated. @@ -683,19 +679,13 @@ private readonly struct GraphNodeStackState /// public readonly GraphEdge OuterEdge; - /// - /// A indicating parent node status for the current . - /// - public readonly bool HasParentNodes; - public GraphNodeStackState( GraphNode graphNode, GraphNode parentNode, LightweightList dependencies, int dependencyIndex, LibraryRange libraryRange, - GraphEdge outerEdge, - bool hasParentNodes) + GraphEdge outerEdge) { GraphNode = graphNode; ParentNode = parentNode; @@ -703,7 +693,6 @@ public GraphNodeStackState( DependencyIndex = dependencyIndex; LibraryRange = libraryRange; OuterEdge = outerEdge; - HasParentNodes = hasParentNodes; } } @@ -732,18 +721,12 @@ private readonly struct GraphNodeCreationData /// public readonly GraphEdge OuterEdge; - /// - /// Indicates if this node should have parent nodes. - /// - public readonly bool HasParentNodes; - - public GraphNodeCreationData(Task> graphItemTask, HashSet runtimeDependencies, LibraryRange libraryRange, GraphEdge outerEdge, bool hasParentNodes) + public GraphNodeCreationData(Task> graphItemTask, HashSet runtimeDependencies, LibraryRange libraryRange, GraphEdge outerEdge) { GraphItemTask = graphItemTask; RuntimeDependencies = runtimeDependencies; LibraryRange = libraryRange; OuterEdge = outerEdge; - HasParentNodes = hasParentNodes; } } } From ce97ee84013963ab1661fb641d77a2d1db2f2cb1 Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Fri, 16 Feb 2024 16:38:53 -0800 Subject: [PATCH 5/7] Remove unnecessary LibraryRange --- .../Remote/RemoteDependencyWalker.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index 254bd410c18..f1f7c97909e 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -112,7 +112,6 @@ private async ValueTask> CreateGraphNodeAsync( null, rootDependencies, 0, - libraryRange, outerEdge)); while (stackStates.Count > 0) @@ -121,7 +120,6 @@ private async ValueTask> CreateGraphNodeAsync( GraphNodeStackState currentState = stackStates.Pop(); GraphNode node = currentState.GraphNode; LightweightList dependencyNodeCreationData = currentState.DependencyData; - LibraryRange currentLibraryRange = currentState.LibraryRange; GraphEdge currentOuterEdge = currentState.OuterEdge; int index = currentState.DependencyIndex; @@ -149,7 +147,7 @@ private async ValueTask> CreateGraphNodeAsync( // Check for a cycle, this is needed for A (project) -> A (package) // since the predicate will not be called for leaf nodes. - if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, currentLibraryRange.Name)) + if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, node.Key.Name)) { result = (DependencyResult.Cycle, dependency); } @@ -238,7 +236,6 @@ private async ValueTask> CreateGraphNodeAsync( currentState.ParentNode, dependencyNodeCreationData, index + 1, - currentState.LibraryRange, currentState.OuterEdge)); LightweightList newDependencies = new LightweightList(newNode.Item.Data.Dependencies.Count); @@ -249,7 +246,6 @@ private async ValueTask> CreateGraphNodeAsync( node, newDependencies, 0, - graphNodeCreationData.LibraryRange, graphNodeCreationData.OuterEdge)); } @@ -669,11 +665,6 @@ private readonly struct GraphNodeStackState /// public readonly int DependencyIndex; - /// - /// The for the current . - /// - public readonly LibraryRange LibraryRange; - /// /// The for the current . /// @@ -684,14 +675,12 @@ public GraphNodeStackState( GraphNode parentNode, LightweightList dependencies, int dependencyIndex, - LibraryRange libraryRange, GraphEdge outerEdge) { GraphNode = graphNode; ParentNode = parentNode; DependencyData = dependencies; DependencyIndex = dependencyIndex; - LibraryRange = libraryRange; OuterEdge = outerEdge; } } From 01f3d5381f20abb01e292b75d054e16de1b803eb Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Fri, 16 Feb 2024 18:29:44 -0800 Subject: [PATCH 6/7] Remove ParentNode --- .../Remote/RemoteDependencyWalker.cs | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index f1f7c97909e..d024d62504d 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -109,7 +109,6 @@ private async ValueTask> CreateGraphNodeAsync( stackStates.Push(new GraphNodeStackState( rootNode, - null, rootDependencies, 0, outerEdge)); @@ -120,6 +119,7 @@ private async ValueTask> CreateGraphNodeAsync( GraphNodeStackState currentState = stackStates.Pop(); GraphNode node = currentState.GraphNode; LightweightList dependencyNodeCreationData = currentState.DependencyData; + LibraryRange currentLibraryRange = node.Key; GraphEdge currentOuterEdge = currentState.OuterEdge; int index = currentState.DependencyIndex; @@ -147,7 +147,7 @@ private async ValueTask> CreateGraphNodeAsync( // Check for a cycle, this is needed for A (project) -> A (package) // since the predicate will not be called for leaf nodes. - if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, node.Key.Name)) + if (StringComparer.OrdinalIgnoreCase.Equals(dependency.Name, currentLibraryRange.Name)) { result = (DependencyResult.Cycle, dependency); } @@ -233,27 +233,21 @@ private async ValueTask> CreateGraphNodeAsync( // on the next iteration which will let us enter the final block. stackStates.Push(new GraphNodeStackState( node, - currentState.ParentNode, dependencyNodeCreationData, index + 1, currentState.OuterEdge)); LightweightList newDependencies = new LightweightList(newNode.Item.Data.Dependencies.Count); + newNode.OuterNode = node; + // We have a new dependency that we need to evaluate before its parent, so we push it onto the stack after the parent. stackStates.Push(new GraphNodeStackState( newNode, - node, newDependencies, 0, graphNodeCreationData.OuterEdge)); } - - // This block does the final edge connection after all dependencies are fully evaluated. - if (index >= dependencyNodeCreationData.Count) - { - node.OuterNode = currentState.ParentNode; - } } return rootNode; @@ -650,11 +644,6 @@ private readonly struct GraphNodeStackState /// public readonly GraphNode GraphNode; - /// - /// A reference to the parent . - /// - public readonly GraphNode ParentNode; - /// /// The dependencies of the current that will be updated as a final step. /// @@ -672,13 +661,11 @@ private readonly struct GraphNodeStackState public GraphNodeStackState( GraphNode graphNode, - GraphNode parentNode, LightweightList dependencies, int dependencyIndex, GraphEdge outerEdge) { GraphNode = graphNode; - ParentNode = parentNode; DependencyData = dependencies; DependencyIndex = dependencyIndex; OuterEdge = outerEdge; From c3e2bf53332b118e9e72464d987d8d3e07327bc3 Mon Sep 17 00:00:00 2001 From: Eric Arndt Date: Mon, 19 Feb 2024 11:32:07 -0800 Subject: [PATCH 7/7] Smaller LightweightList --- .../Remote/RemoteDependencyWalker.cs | 67 +------------------ 1 file changed, 1 insertion(+), 66 deletions(-) diff --git a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs index d024d62504d..f5392c265fb 100644 --- a/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs +++ b/src/NuGet.Core/NuGet.DependencyResolver.Core/Remote/RemoteDependencyWalker.cs @@ -709,7 +709,7 @@ public GraphNodeCreationData(Task> graphItemTask, internal struct LightweightList { - private const int Fields = 10; + private const int Fields = 5; private readonly int _expectedCapacity; private int _count; private T _firstItem; @@ -717,11 +717,6 @@ internal struct LightweightList private T _thirdItem; private T _fourthItem; private T _fifthItem; - private T _sixthItem; - private T _seventhItem; - private T _eighthItem; - private T _ninthItem; - private T _tenthItem; private List _additionalItems; @@ -761,26 +756,6 @@ public readonly T this[int index] { return _fifthItem; } - else if (index == 5) - { - return _sixthItem; - } - else if (index == 6) - { - return _seventhItem; - } - else if (index == 7) - { - return _eighthItem; - } - else if (index == 8) - { - return _ninthItem; - } - else if (index == 9) - { - return _tenthItem; - } else { return _additionalItems[index - Fields]; @@ -810,26 +785,6 @@ public void Add(T task) { _fifthItem = task; } - else if (_count == 5) - { - _sixthItem = task; - } - else if (_count == 6) - { - _seventhItem = task; - } - else if (_count == 7) - { - _eighthItem = task; - } - else if (_count == 8) - { - _ninthItem = task; - } - else if (_count == 9) - { - _tenthItem = task; - } else { if (_additionalItems == null) @@ -894,26 +849,6 @@ public bool MoveNext() { _current = _itemList._fifthItem; } - else if (_index == 5) - { - _current = _itemList._sixthItem; - } - else if (_index == 6) - { - _current = _itemList._seventhItem; - } - else if (_index == 7) - { - _current = _itemList._eighthItem; - } - else if (_index == 8) - { - _current = _itemList._ninthItem; - } - else if (_index == 9) - { - _current = _itemList._tenthItem; - } else { _current = _itemList._additionalItems[_index - Fields];