install: rebind cached package ids before the second resolve pass

src/collections/zig_hash_map.rs

@@ -494,6 +494,17 @@ impl<K, V, C: HashContext<K>> HashMap<K, V, C> {
             .map(|(k, v)| crate::hash_map::KV { key: k, value: v })
     }
 
+    /// std `retain`. Calls `keep` exactly once per entry.
+    pub fn retain(&mut self, mut keep: impl FnMut(&K, &mut V) -> bool) {
+        for idx in 0..self.slots.len() {
+            if let Some((key, value)) = &mut self.slots[idx]
+                && !keep(key, value)
+            {
+                let _ = self.remove_by_index(idx);
+            }
+        }
+    }
+
     /// std `entry` API. `VacantEntry::insert` does a second probe (re-runs
     /// `get_or_put_slot`); acceptable for the few callers that use it.
     pub fn entry(&mut self, key: K) -> MapEntry<'_, K, V, C> {
@@ -680,3 +691,45 @@ impl<'a, K, V, C: HashContext<K>> VacantEntry<'a, K, V, C> {
         &mut self.map.slots[idx].as_mut().unwrap().1
     }
 }
+
+#[cfg(test)]
+mod retain_tests {
+    use super::*;
+
+    /// `keep` is shown each entry once, and the entries it accepts are still found by key.
+    fn check(keys: &[u64], rejected: &[u64]) {
+        // The home slot is `key & 7`, so the keys choose the runs.
+        let mut map: HashMap<u64, u32, IdentityContext<u64>> = HashMap::new();
+        for &key in keys {
+            map.insert(key, 0);
+        }
+        assert_eq!(map.capacity(), 8);
+        map.retain(|key, shown| {
+            *shown += 1;
+            !rejected.contains(key)
+        });
+        let mut left: Vec<(u64, u32)> = map.iter().map(|(key, shown)| (*key, *shown)).collect();
+        left.sort_unstable();
+        let mut expected: Vec<(u64, u32)> = (keys.iter().filter(|key| !rejected.contains(key)))
+            .map(|&key| (key, 1))
+            .collect();
+        expected.sort_unstable();
+        assert_eq!(left, expected);
+        assert_eq!(map.len(), expected.len());
+        for (key, _) in expected {
+            assert_eq!(map.get(&key), Some(&1));
+        }
+    }
+
+    #[test]
+    fn retain_shows_each_entry_once() {
+        // 2 and 10 share home slot 2: 10 is in slot 3, behind the rejected entry.
+        check(&[2, 10, 3], &[2]);
+        check(&[2, 10, 3], &[2, 10]);
+        // The run wraps: 6 in slot 6, 14 in slot 7, 22 in slot 0.
+        check(&[6, 14, 22], &[6]);
+        check(&[6, 14, 22], &[14]);
+        check(&[6, 14, 22, 30, 7, 15], &[6, 30]);
+        check(&[2, 10, 3], &[]);
+    }
+}

src/install/PackageManager/install_with_manager.rs

@@ -14,13 +14,14 @@ use crate::Subcommand;
 use crate::dependency::{DependencyExt as _, Tag as DependencyVersionTag};
 use crate::lockfile::{self, Lockfile, reachable};
 use crate::resolution::Tag as ResolutionTag;
+use crate::resolvers::folder_resolver::FolderResolution;
 use crate::update_transitive::{
     DirectDependencies, TransitiveUpdate, enqueue_peer_rows, moved_targets_after_clean,
-    plannable_peer_rows, print_kept_patched, redirect_moved_edges,
+    package_after_clean, plannable_peer_rows, print_kept_patched, redirect_moved_edges,
 };
 use crate::{
     Dependency, DependencyID, Features, PackageID, PackageNameHash, PatchTask, Resolution,
-    invalid_package_id,
+    TaskCallbackContext, invalid_package_id,
 };
 // Bring the typed `items_<field>()` column accessors into scope for
 // `MultiArrayList<Package>` / `Slice<Package>`.
@@ -45,6 +46,7 @@ use crate::package_manager_real::{
     UpdateRequest, enqueue_dependency_list, enqueue_dependency_with_main, enqueue_patch_task_pre,
     save_lockfile, setup_global_dir, update_lockfile_if_needed, write_yarn_lock,
 };
+use crate::package_manager_task as Task;
 
 use super::security_scanner;
 
@@ -733,6 +735,7 @@ pub fn install_with_manager(
                     outside,
                     root_package_json_path,
                     &mut root,
+                    &lockfile_before_clean,
                     log_level,
                 )?;
                 had_any_diffs |= !moved_by_overrides.is_empty();
@@ -2386,6 +2389,61 @@ fn clean_lockfile(
     Ok(before_clean)
 }
 
+/// A clean renumbers packages and rows. Moves the package ids that `manager` cached in the first pass to `manager.lockfile`, and forgets the rest.
+#[cold]
+#[inline(never)]
+fn rebind_to_cleaned_lockfile(manager: &mut PackageManager, before_first_clean: &Lockfile) {
+    let cleaned: &Lockfile = &manager.lockfile;
+    // Returns false if the clean dropped the package.
+    let moved = |package_id: &mut PackageID| {
+        package_after_clean(before_first_clean, cleaned, *package_id).is_some_and(|id| {
+            *package_id = id;
+            true
+        })
+    };
+
+    (manager.appended_task_packages).retain(|task_id, package_id| {
+        let is_kept = moved(package_id);
+        if !is_kept {
+            // The next dependency on the dropped package starts the task again.
+            let _ = manager.network_dedupe_map.remove(task_id);
+        }
+        is_kept
+    });
+    manager
+        .folders
+        .retain(|_, entry| match &mut entry.resolution {
+            FolderResolution::PackageId(package_id) => moved(package_id),
+            _ => true,
+        });
+    // A dropped registry package that is appended again asks for its tarball like a new package.
+    let resolutions = before_first_clean.packages.items_resolution();
+    for (id, resolution) in resolutions.iter().enumerate() {
+        if resolution.tag == ResolutionTag::Npm
+            && package_after_clean(before_first_clean, cleaned, id as PackageID).is_none()
+        {
+            let name = before_first_clean.packages.items_name()[id];
+            let _ = manager
+                .network_dedupe_map
+                .remove(&Task::Id::for_npm_package(
+                    before_first_clean.str(&name),
+                    resolution.npm().version,
+                ));
+        }
+    }
+
+    if manager.named_update_reachable.is_some() {
+        crate::update_scope::plan_named(manager);
+    }
+    // The peer pass of the first pass is over.
+    manager.look_ahead.in_peer_pass = false;
+    // A clean has no mapping for rows, so the rows that the first pass left waiting are dropped.
+    while manager.peer_dependencies.read_item().is_some() {}
+    for waiters in manager.task_queue.values_mut() {
+        waiters.retain(|waiter| !matches!(waiter, TaskCallbackContext::Dependency(_)));
+    }
+}
+
 /// Returns the rows that resolve to a version that their `overrides` rule does not allow, each
 /// with the package it resolves to.
 #[inline(never)]
@@ -2450,6 +2508,7 @@ fn reresolve_rows_outside_overrides(
     mut outside: Vec<(DependencyID, PackageID)>,
     root_package_json_path: &ZStr,
     root: &mut lockfile::Package,
+    before_first_clean: &Lockfile,
     log_level: Options::LogLevel,
 ) -> crate::Result<Vec<PackageID>> {
     // yarn applies `"x@^1.0.0"` in `resolutions` only to dependencies declared as exactly `^1.0.0`.
@@ -2502,6 +2561,7 @@ fn reresolve_rows_outside_overrides(
         return Ok(Vec::new());
     }
 
+    rebind_to_cleaned_lockfile(manager, before_first_clean);
     manager.lockfile.mark_loaded_packages()?;
     let mut moved: Vec<(DependencyID, PackageID)> = Vec::new();
     let mut peer_rows: Vec<DependencyID> = Vec::new();

src/install/update_scope.rs

@@ -26,7 +26,7 @@ pub struct UpdateScope<'a> {
     whole_workspace: bool,
     /// `--depth 0`: only the dependencies of the root and of workspaces are in scope.
     direct_only: bool,
-    /// Walked by `plan_named` before the differ invalidates rows; otherwise `reachable()` walks the lockfile it is given.
+    /// Walked by `plan_named` before rows are invalidated; otherwise `reachable()` walks the lockfile it is given.
     planned: Option<&'a DynamicBitSet>,
     lazy: OnceCell<DynamicBitSet>,
 }
@@ -63,7 +63,7 @@ impl<'a> UpdateScope<'a> {
     }
 }
 
-/// Runs on the loaded lockfile before the differ invalidates the rows it re-enqueues, like `TransitiveUpdate::plan`.
+/// Runs before rows are invalidated: by the differ on the loaded lockfile, like `TransitiveUpdate::plan`, and by `reresolve_rows_outside_overrides` on the cleaned one.
 pub fn plan_named(manager: &mut PackageManager) {
     let reachable = {
         let scope = UpdateScope::of(manager);
@@ -144,7 +144,7 @@ impl UpdateScope<'_> {
                 lockfile.packages.items_name_hash()[id],
                 lockfile.packages.items_name()[id].slice(lockfile.buffers.string_bytes.as_slice()),
             ),
-            // Packages appended after the walk (ids past its end) were resolved for an in-scope row.
+            // Packages appended after the walk (ids past its end) count as in scope.
             _ => {
                 !self.direct_only
                     && (self.whole_workspace

src/install/update_transitive.rs

@@ -506,42 +506,36 @@ pub(crate) fn redirect_moved_edges(
     redirect(lockfile, pairs, direct_only);
 }
 
+/// The package of `cleaned` with the name and the resolution of the package `old_id` of `before_clean`. `None` if the clean dropped it.
+pub(crate) fn package_after_clean(
+    before_clean: &Lockfile,
+    cleaned: &Lockfile,
+    old_id: PackageID,
+) -> Option<PackageID> {
+    let old_res = before_clean
+        .packages
+        .items_resolution()
+        .get(old_id as usize)?;
+    let old_buf = before_clean.buffers.string_bytes.as_slice();
+    let new_res = cleaned.packages.items_resolution();
+    let new_buf = cleaned.buffers.string_bytes.as_slice();
+    let entry =
+        (cleaned.package_index).get(&before_clean.packages.items_name_hash()[old_id as usize])?;
+    (entry.as_slice().iter().copied())
+        .find(|&id| new_res[id as usize].eql(old_res, new_buf, old_buf))
+}
+
 /// Packages of `cleaned` that the `moved` edges of `before_clean` resolved to, for seeding the security scan.
 pub(crate) fn moved_targets_after_clean(
     before_clean: &Lockfile,
     cleaned: &Lockfile,
     moved: &[(DependencyID, PackageID)],
 ) -> Vec<PackageID> {
-    if moved.is_empty() {
-        return Vec::new();
-    }
     let old_resolutions = before_clean.buffers.resolutions.as_slice();
-    let old_name_hashes = before_clean.packages.items_name_hash();
-    let old_res = before_clean.packages.items_resolution();
-    let old_buf = before_clean.buffers.string_bytes.as_slice();
-    let new_res = cleaned.packages.items_resolution();
-    let new_buf = cleaned.buffers.string_bytes.as_slice();
-
-    let mut targets: Vec<PackageID> = Vec::new();
-    for &(dep_id, _) in moved {
-        let Some(&old_id) = old_resolutions.get(dep_id as usize) else {
-            continue;
-        };
-        if (old_id as usize) >= old_res.len() {
-            continue;
-        }
-        let Some(entry) = cleaned.package_index.get(&old_name_hashes[old_id as usize]) else {
-            continue;
-        };
-        if let Some(&id) = entry
-            .as_slice()
-            .iter()
-            .find(|&&c| new_res[c as usize].eql(&old_res[old_id as usize], new_buf, old_buf))
-        {
-            targets.push(id);
-        }
-    }
-    targets
+    (moved.iter())
+        .filter_map(|&(dep_id, _)| old_resolutions.get(dep_id as usize))
+        .filter_map(|&old_id| package_after_clean(before_clean, cleaned, old_id))
+        .collect()
 }
 
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]