Auto merge of #14663 - x-hgg-x:resolver-perf, r=Eh2406

Improve resolver speed

### What does this PR try to resolve?

This PR improves the resolver speed after investigations from Eh2406/pubgrub-crates-benchmark#6.

### How should we test and review this PR?

Commit 1 adds a test showing a slow case in the resolver, where resolving time is cubic over the number of versions of a crate. It can be used for benchmarking the resolver.

Comparison of the resolving time with various values of `N=LAST_CRATE_VERSION_COUNT` and `C=TRANSITIVE_CRATES_COUNT`:

|      | N=100 | N=200 | N=400 |
|------|-------|-------|-------|
| C=1  | 0.25s | 0.5s  | 1.4s  |
| C=2  | 7s    | 44s   | 314s  |
| C=3  | 12s   | 77s   | 537s  |
| C=6  | 30s   | 149s  | 1107s |
| C=12 | 99s   | 447s  | 2393s |

Commit 2 replaces the default hasher with the hasher from `rustc-hash`, decreasing resolving time by more than 50%.

Performance comparison with the test from commit 1 by setting `LAST_CRATE_VERSION_COUNT = 100`:
| commit          | duration |
|-----------------|----------|
| master          | 16s      |
| with rustc-hash | 6.7s     |

Firefox profiles, can be read with https://profiler.firefox.com:
[perf.tar.gz](https://github.com/user-attachments/files/17318243/perf.tar.gz)

r? Eh2406

Author: bors

Cargo.lock

@@ -80,6 +80,7 @@ pulldown-cmark = { version = "0.11.0", default-features = false, features = ["ht
 rand = "0.8.5"
 regex = "1.10.5"
 rusqlite = { version = "0.32.0", features = ["bundled"] }
+rustc-hash = "2.0.0"
 rustfix = { version = "0.8.2", path = "crates/rustfix" }
 same-file = "1.0.6"
 security-framework = "2.11.1"
@@ -187,6 +188,7 @@ pathdiff.workspace = true
 rand.workspace = true
 regex.workspace = true
 rusqlite.workspace = true
+rustc-hash.workspace = true
 rustfix.workspace = true
 same-file.workspace = true
 semver.workspace = true

Cargo.toml

@@ -352,7 +352,7 @@ impl SatResolver {
 
         let mut by_name: HashMap<InternedString, Vec<Summary>> = HashMap::new();
         for pkg in registry {
-            by_name.entry(pkg.name()).or_default().push(pkg.clone())
+            by_name.entry(pkg.name()).or_default().push(pkg.clone());
         }
 
         let mut solver = varisat::Solver::new();

crates/resolver-tests/src/sat.rs

@@ -4,8 +4,8 @@ use cargo::util::GlobalContext;
 
 use resolver_tests::{
     helpers::{
-        assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg_id,
-        pkg_loc, registry, ToPkgId,
+        assert_contains, assert_same, dep, dep_kind, dep_loc, dep_req, loc_names, names, pkg,
+        pkg_dep, pkg_dep_with, pkg_id, pkg_loc, registry, ToDep, ToPkgId,
     },
     pkg, resolve, resolve_with_global_context,
 };
@@ -937,6 +937,50 @@ fn large_conflict_cache() {
     let _ = resolve(root_deps, &reg);
 }
 
+#[test]
+fn resolving_slow_case_missing_feature() {
+    let mut reg = Vec::new();
+
+    const LAST_CRATE_VERSION_COUNT: usize = 50;
+
+    // increase in resolve time is at least cubic over `INTERMEDIATE_CRATES_VERSION_COUNT`.
+    // it should be `>= LAST_CRATE_VERSION_COUNT` to reproduce slowdown.
+    const INTERMEDIATE_CRATES_VERSION_COUNT: usize = LAST_CRATE_VERSION_COUNT + 5;
+
+    // should be `>= 2` to reproduce slowdown
+    const TRANSITIVE_CRATES_COUNT: usize = 3;
+
+    reg.push(pkg_dep_with(("last", "1.0.0"), vec![], &[("f", &[])]));
+    for v in 1..LAST_CRATE_VERSION_COUNT {
+        reg.push(pkg(("last", format!("1.0.{v}"))));
+    }
+
+    reg.push(pkg_dep(
+        ("dep", "1.0.0"),
+        vec![
+            dep("last"), // <-- needed to reproduce slowdown
+            dep_req("intermediate-1", "1.0.0"),
+        ],
+    ));
+
+    for n in 0..INTERMEDIATE_CRATES_VERSION_COUNT {
+        let version = format!("1.0.{n}");
+        for c in 1..TRANSITIVE_CRATES_COUNT {
+            reg.push(pkg_dep(
+                (format!("intermediate-{c}"), &version),
+                vec![dep_req(&format!("intermediate-{}", c + 1), &version)],
+            ));
+        }
+        reg.push(pkg_dep(
+            (format!("intermediate-{TRANSITIVE_CRATES_COUNT}"), &version),
+            vec![dep_req("last", "1.0.0").with(&["f"])],
+        ));
+    }
+
+    let deps = vec![dep("dep")];
+    let _ = resolve(deps, &reg);
+}
+
 #[test]
 fn cyclic_good_error_message() {
     let input = vec![

crates/resolver-tests/tests/resolve.rs

@@ -1,5 +1,6 @@
-use std::collections::{BTreeMap, HashMap, HashSet};
+use std::collections::{BTreeMap, HashMap};
 
+use rustc_hash::{FxHashMap, FxHashSet};
 use tracing::trace;
 
 use super::types::ConflictMap;
@@ -140,17 +141,17 @@ pub(super) struct ConflictCache {
     // as a global cache which we never delete from. Any entry in this map is
     // unconditionally true regardless of our resolution history of how we got
     // here.
-    con_from_dep: HashMap<Dependency, ConflictStoreTrie>,
+    con_from_dep: FxHashMap<Dependency, ConflictStoreTrie>,
     // `dep_from_pid` is an inverse-index of `con_from_dep`.
     // For every `PackageId` this lists the `Dependency`s that mention it in `dep_from_pid`.
-    dep_from_pid: HashMap<PackageId, HashSet<Dependency>>,
+    dep_from_pid: FxHashMap<PackageId, FxHashSet<Dependency>>,
 }
 
 impl ConflictCache {
     pub fn new() -> ConflictCache {
         ConflictCache {
-            con_from_dep: HashMap::new(),
-            dep_from_pid: HashMap::new(),
+            con_from_dep: HashMap::default(),
+            dep_from_pid: HashMap::default(),
         }
     }
     pub fn find(
@@ -207,14 +208,11 @@ impl ConflictCache {
         );
 
         for c in con.keys() {
-            self.dep_from_pid
-                .entry(*c)
-                .or_insert_with(HashSet::new)
-                .insert(dep.clone());
+            self.dep_from_pid.entry(*c).or_default().insert(dep.clone());
         }
     }
 
-    pub fn dependencies_conflicting_with(&self, pid: PackageId) -> Option<&HashSet<Dependency>> {
+    pub fn dependencies_conflicting_with(&self, pid: PackageId) -> Option<&FxHashSet<Dependency>> {
         self.dep_from_pid.get(&pid)
     }
 }

src/cargo/core/resolver/conflict_cache.rs

@@ -19,13 +19,13 @@ pub struct ResolverContext {
     pub age: ContextAge,
     pub activations: Activations,
     /// list the features that are activated for each package
-    pub resolve_features: im_rc::HashMap<PackageId, FeaturesSet>,
+    pub resolve_features: im_rc::HashMap<PackageId, FeaturesSet, rustc_hash::FxBuildHasher>,
     /// get the package that will be linking to a native library by its links attribute
-    pub links: im_rc::HashMap<InternedString, PackageId>,
+    pub links: im_rc::HashMap<InternedString, PackageId, rustc_hash::FxBuildHasher>,
 
     /// a way to look up for a package in activations what packages required it
     /// and all of the exact deps that it fulfilled.
-    pub parents: Graph<PackageId, im_rc::HashSet<Dependency>>,
+    pub parents: Graph<PackageId, im_rc::HashSet<Dependency, rustc_hash::FxBuildHasher>>,
 }
 
 /// When backtracking it can be useful to know how far back to go.
@@ -40,7 +40,9 @@ pub type ContextAge = usize;
 /// semver compatible version of each crate.
 /// This all so stores the `ContextAge`.
 pub type ActivationsKey = (InternedString, SourceId, SemverCompatibility);
-pub type Activations = im_rc::HashMap<ActivationsKey, (Summary, ContextAge)>;
+
+pub type Activations =
+    im_rc::HashMap<ActivationsKey, (Summary, ContextAge), rustc_hash::FxBuildHasher>;
 
 /// A type that represents when cargo treats two Versions as compatible.
 /// Versions `a` and `b` are compatible if their left-most nonzero digit is the
@@ -74,10 +76,10 @@ impl ResolverContext {
     pub fn new() -> ResolverContext {
         ResolverContext {
             age: 0,
-            resolve_features: im_rc::HashMap::new(),
-            links: im_rc::HashMap::new(),
+            resolve_features: im_rc::HashMap::default(),
+            links: im_rc::HashMap::default(),
             parents: Graph::new(),
-            activations: im_rc::HashMap::new(),
+            activations: im_rc::HashMap::default(),
         }
     }
 

src/cargo/core/resolver/context.rs

@@ -679,7 +679,7 @@ fn activate(
         Rc::make_mut(
             cx.resolve_features
                 .entry(candidate.package_id())
-                .or_insert_with(Rc::default),
+                .or_default(),
         )
         .extend(used_features);
     }

src/cargo/core/resolver/mod.rs

@@ -22,7 +22,7 @@ impl<N: Eq + Ord + Clone, E: Default + Clone> Graph<N, E> {
             .entry(node)
             .or_insert_with(im_rc::OrdMap::new)
             .entry(child)
-            .or_insert_with(Default::default)
+            .or_default()
     }
 
     /// Returns the graph obtained by reversing all edges.