Skip resolved child dependencies in the new resolver if an identical …

…one has already been resolved (#6258)
NuGet · Feb 13, 2025 · e716e78 · e716e78
1 parent 6a2ad25
commit e716e78
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 21 deletions.
diff --git a/...Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.ResolvedDependencyGraphItem.cs b/...Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.ResolvedDependencyGraphItem.cs
@@ -35,17 +35,29 @@ public class ResolvedDependencyGraphItem
             /// <param name="item">The <see cref="GraphItem{TItem}" /> with a <see cref="RemoteResolveResult" /> of the resolved library.</param>
             /// <param name="dependencyGraphItem">The <see cref="DependencyGraphItem" /> that is being resolved.</param>
             /// <param name="indexingTable">The <see cref="DependencyGraphItemIndexer" /> to use when indexing dependencies.</param>
+            /// <param name="parents">An optional <see cref="HashSet{T}" /> containing parent <see cref="LibraryRangeIndex" /> values.</param>
             public ResolvedDependencyGraphItem(
                 GraphItem<RemoteResolveResult> item,
                 DependencyGraphItem dependencyGraphItem,
-                DependencyGraphItemIndexer indexingTable)
+                DependencyGraphItemIndexer indexingTable,
+                HashSet<LibraryRangeIndex>? parents = null)
             {
                 Item = item;
 
                 LibraryDependency = dependencyGraphItem.LibraryDependency;
                 LibraryRangeIndex = dependencyGraphItem.LibraryRangeIndex;
                 Path = dependencyGraphItem.Path;
 
+                if (parents != null)
+                {
+                    Parents = parents;
+                }
+                else if (dependencyGraphItem.IsCentrallyPinnedTransitivePackage && !dependencyGraphItem.IsRootPackageReference)
+                {
+                    // Keep track of parents if the item is transitively pinned
+                    Parents = new HashSet<LibraryRangeIndex> { dependencyGraphItem.Parent };
+                }
+
                 int dependencyCount = item.Data.Dependencies.Count;
 
                 if (dependencyCount == 0)
@@ -102,7 +114,7 @@ public ResolvedDependencyGraphItem(
             /// <summary>
             /// Gets or sets a <see cref="HashSet{T}" /> of all of the parent <see cref="LibraryRangeIndex" /> of this dependency graph item.
             /// </summary>
-            public HashSet<LibraryRangeIndex>? Parents { get; set; }
+            public HashSet<LibraryRangeIndex>? Parents { get; }
 
             /// <summary>
             /// Gets an array containing the <see cref="LibraryRangeIndex" /> values of all parent dependency graph items and their parent up to the root.

diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.cs
@@ -968,7 +968,6 @@ private async Task<Dictionary<LibraryDependencyIndex, ResolvedDependencyGraphIte
                     // Create a resolved dependency graph item and add it to the list of chosen items
                     chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable)
                     {
-                        Parents = currentDependencyGraphItem.IsCentrallyPinnedTransitivePackage && !currentDependencyGraphItem.IsRootPackageReference ? new HashSet<LibraryRangeIndex>() { currentDependencyGraphItem.Parent } : null,
                         IsCentrallyPinnedTransitivePackage = currentDependencyGraphItem.IsCentrallyPinnedTransitivePackage,
                         IsRootPackageReference = currentDependencyGraphItem.IsRootPackageReference,
                         Suppressions = new List<HashSet<LibraryDependencyIndex>>
@@ -1116,7 +1115,6 @@ private async Task<Dictionary<LibraryDependencyIndex, ResolvedDependencyGraphIte
                         // Add the item to the list of chosen items
                         chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable)
                         {
-                            Parents = currentDependencyGraphItem.IsCentrallyPinnedTransitivePackage && !currentDependencyGraphItem.IsRootPackageReference ? new HashSet<LibraryRangeIndex>() { currentDependencyGraphItem.Parent } : null,
                             IsCentrallyPinnedTransitivePackage = currentDependencyGraphItem.IsCentrallyPinnedTransitivePackage,
                             IsRootPackageReference = currentDependencyGraphItem.IsRootPackageReference,
                             Suppressions = new List<HashSet<LibraryDependencyIndex>>
@@ -1159,8 +1157,6 @@ private async Task<Dictionary<LibraryDependencyIndex, ResolvedDependencyGraphIte
                     }
                     else // The current item and chosen item have the same version
                     {
-                        chosenResolvedItem.Parents ??= new HashSet<LibraryRangeIndex>();
-
                         if (!chosenResolvedItem.IsRootPackageReference)
                         {
                             // Keep track of the parents of this item
@@ -1177,9 +1173,8 @@ private async Task<Dictionary<LibraryDependencyIndex, ResolvedDependencyGraphIte
                             // Replace the chosen item with the current one since they are basically the same and process its children
                             resolvedDependencyGraphItems.Remove(currentDependencyGraphItem.LibraryDependencyIndex);
 
-                            chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable)
+                            chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable, chosenResolvedItem.Parents)
                             {
-                                Parents = chosenResolvedItem.Parents,
                                 IsCentrallyPinnedTransitivePackage = chosenResolvedItem.IsCentrallyPinnedTransitivePackage,
                                 IsRootPackageReference = chosenResolvedItem.IsRootPackageReference,
                                 Suppressions = new List<HashSet<LibraryDependencyIndex>>
@@ -1211,9 +1206,8 @@ private async Task<Dictionary<LibraryDependencyIndex, ResolvedDependencyGraphIte
                                 // Replace the chosen item with the current item with the the combined list of suppressions and process its children
                                 resolvedDependencyGraphItems.Remove(currentDependencyGraphItem.LibraryDependencyIndex);
 
-                                chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable)
+                                chosenResolvedItem = new ResolvedDependencyGraphItem(currentGraphItem, currentDependencyGraphItem, _indexingTable, chosenResolvedItem.Parents)
                                 {
-                                    Parents = chosenResolvedItem.Parents,
                                     IsCentrallyPinnedTransitivePackage = chosenResolvedItem.IsCentrallyPinnedTransitivePackage,
                                     IsRootPackageReference = chosenResolvedItem.IsRootPackageReference,
                                     Suppressions =
@@ -1301,15 +1295,7 @@ .. chosenResolvedItem.Suppressions
                         continue;
                     }
 
-                    HashSet<LibraryDependency>? runtimeDependencies = default;
-
-                    // Evaluate the runtime dependencies if any
-                    if (EvaluateRuntimeDependencies(ref childDependency, runtimeGraph, pair.RuntimeIdentifier, ref runtimeDependencies))
-                    {
-                        // EvaluateRuntimeDependencies() returns true if the version of the dependency was changed, which also changes the LibraryRangeIndex so that must be updated in the chosen item's array of library range indices.
-                        chosenResolvedItem.SetRangeIndexForDependencyAt(i, _indexingTable.Index(childDependency.LibraryRange));
-                    }
-
+                    LibraryRangeIndex childLibraryRangeIndex = chosenResolvedItem.GetRangeIndexForDependencyAt(i);
                     bool isPackage = childDependency.LibraryRange.TypeConstraintAllows(LibraryDependencyTarget.Package);
                     bool isRootPackageReference = (currentDependencyGraphItem.LibraryDependencyIndex == LibraryDependencyIndex.Project) && isPackage;
 
@@ -1324,15 +1310,37 @@ .. chosenResolvedItem.Suppressions
                         continue;
                     }
 
+                    // See if a dependency with the same version and no suppressions has already been resolved.  If so, its children have already been added to the queue.
+                    if (resolvedDependencyGraphItems.TryGetValue(childLibraryDependencyIndex, out ResolvedDependencyGraphItem? childResolvedDependencyGraphItem)
+                        && childResolvedDependencyGraphItem.LibraryRangeIndex == childLibraryRangeIndex
+                        && childResolvedDependencyGraphItem.Suppressions.Count == 1
+                        && childResolvedDependencyGraphItem.Suppressions[0].Count == 0)
+                    {
+                        if (!childResolvedDependencyGraphItem.IsRootPackageReference)
+                        {
+                            // Keep track of the parents of this item
+                            childResolvedDependencyGraphItem.Parents?.Add(currentDependencyGraphItem.LibraryRangeIndex);
+                        }
+
+                        continue;
+                    }
+
+                    HashSet<LibraryDependency>? runtimeDependencies = default;
+
+                    // Evaluate the runtime dependencies if any
+                    if (EvaluateRuntimeDependencies(ref childDependency, runtimeGraph, pair.RuntimeIdentifier, ref runtimeDependencies))
+                    {
+                        // EvaluateRuntimeDependencies() returns true if the version of the dependency was changed, which also changes the LibraryRangeIndex so that must be updated in the chosen item's array of library range indices.
+                        chosenResolvedItem.SetRangeIndexForDependencyAt(i, _indexingTable.Index(childDependency.LibraryRange));
+                    }
+
                     VersionRange? pinnedVersionRange = null;
 
                     // Determine if the package is transitively pinned
                     bool isCentrallyPinnedTransitiveDependency = isCentralPackageTransitivePinningEnabled
                         && isPackage
                         && pinnedPackageVersions?.TryGetValue(childLibraryDependencyIndex, out pinnedVersionRange) == true;
 
-                    LibraryRangeIndex childLibraryRangeIndex = chosenResolvedItem.GetRangeIndexForDependencyAt(i);
-
                     if (isCentrallyPinnedTransitiveDependency && !isRootPackageReference)
                     {
                         // If central transitive pinning is enabled the LibraryDependency must be recreated as not to mutate the in-memory copy