Store files in HashSet

Author: lionel-

crates/oak_db/src/inputs.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use aether_url::UrlId;
 use oak_package_metadata::namespace::Namespace;
 
@@ -124,13 +126,16 @@ pub struct OrphanRoot {
     /// **Placement invariant.** Files here must have `package(db) ==
     /// None`. Call this setter only through `oak_scan`'s helpers,
     /// which keep the back-pointer and the container in sync.
+    ///
+    /// Unordered: these are unanchored files looked up by URL, with no
+    /// collation chain among them, so membership is all that matters.
     #[returns(ref)]
-    pub files: Vec<File>,
+    pub files: HashSet<File>,
 }
 
 impl OrphanRoot {
     pub fn empty(db: &dyn Db) -> Self {
-        Self::new(db, vec![])
+        Self::new(db, HashSet::new())
     }
 }
 
@@ -161,15 +166,17 @@ impl OrphanRoot {
 /// of them gets pulled back into a live container.
 #[salsa::input]
 pub struct StaleRoot {
+    /// Unordered: entity-reuse storage looked up by URL, no collation chain,
+    /// so membership is all that matters.
     #[returns(ref)]
-    pub files: Vec<File>,
+    pub files: HashSet<File>,
     #[returns(ref)]
     pub packages: Vec<Package>,
 }
 
 impl StaleRoot {
     pub fn empty(db: &dyn Db) -> Self {
-        Self::new(db, vec![], vec![])
+        Self::new(db, HashSet::new(), vec![])
     }
 }
 

crates/oak_db/src/tests/db.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use oak_package_metadata::namespace::Namespace;
 use salsa::Setter;
 
@@ -73,7 +75,9 @@ fn test_file_by_url_finds_library_package_file() {
 fn test_file_by_url_finds_orphan_file() {
     let mut db = TestDb::new();
     let file = File::new(&db, file_url("untitled.R"), String::new(), None);
-    db.orphan_root().set_files(&mut db).to(vec![file]);
+    db.orphan_root()
+        .set_files(&mut db)
+        .to(HashSet::from([file]));
 
     assert_eq!(db.file_by_url(&file_url("untitled.R")), Some(file));
 }

crates/oak_db/src/tests/resolver.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use oak_semantic::semantic_index::DefinitionKind;
 use oak_semantic::semantic_index::ScopeId;
 use oak_semantic::semantic_index::SemanticCallKind;
@@ -350,7 +352,9 @@ fn test_source_anchors_to_parent_dir_when_no_workspace() {
         None,
     );
     let b = File::new(&db, file_url("dir/b.R"), "x <- 1\n".to_string(), None);
-    db.orphan_root().set_files(&mut db).to(vec![a, b]);
+    db.orphan_root()
+        .set_files(&mut db)
+        .to(HashSet::from([a, b]));
 
     let index = a.semantic_index(&db);
     assert!(index.exports().contains_key("x"));
@@ -368,7 +372,9 @@ fn test_source_path_with_parent_dir_segments() {
         None,
     );
     let b = File::new(&db, file_url("dir/b.R"), "x <- 1\n".to_string(), None);
-    db.orphan_root().set_files(&mut db).to(vec![a, b]);
+    db.orphan_root()
+        .set_files(&mut db)
+        .to(HashSet::from([a, b]));
 
     let index = a.semantic_index(&db);
     assert!(index.exports().contains_key("x"));

crates/oak_scan/src/inputs.rs

@@ -17,6 +17,7 @@
 //! input structs.
 
 use std::collections::HashSet;
+use std::hash::Hash;
 use std::path::PathBuf;
 
 use aether_url::UrlId;
@@ -178,7 +179,7 @@ impl<DB: Db + DbInputs> DbScan for DB {
         };
 
         let orphan = self.orphan_root();
-        let Some(orphan_files) = with_removed(orphan.files(self), file) else {
+        let Some(orphan_files) = with_cow_remove(orphan.files(self), file) else {
             // A workspace or library root holds it, nothing to do.
             return;
         };
@@ -187,7 +188,7 @@ impl<DB: Db + DbInputs> DbScan for DB {
         orphan.set_files(self).to(orphan_files);
 
         let stale = self.stale_root();
-        if let Some(stale_files) = with_appended(stale.files(self), file) {
+        if let Some(stale_files) = with_cow_insert(stale.files(self), file) {
             stale.set_files(self).to(stale_files);
         }
     }
@@ -384,36 +385,37 @@ pub(crate) fn upsert_root_file<DB: Db + DbInputs>(
 /// entry. Also used by [`watch::remove_watched_file`] when a file
 /// disappears from disk.
 pub(crate) fn remove_from_pkg_files<DB: Db + DbInputs>(db: &mut DB, pkg: Package, file: File) {
-    if let Some(files) = with_removed(pkg.files(db), file) {
+    if let Some(files) = with_cow_filter(pkg.files(db), file) {
         pkg.set_files(db).to(files);
         return;
     }
-    if let Some(scripts) = with_removed(pkg.scripts(db), file) {
+    if let Some(scripts) = with_cow_filter(pkg.scripts(db), file) {
         pkg.set_scripts(db).to(scripts);
     }
 }
 
 pub(crate) fn remove_from_orphan<DB: Db + DbInputs>(db: &mut DB, file: File) {
     let orphan = db.orphan_root();
-    if let Some(files) = with_removed(orphan.files(db), file) {
+    if let Some(files) = with_cow_remove(orphan.files(db), file) {
         orphan.set_files(db).to(files);
     }
 }
 
 fn add_to_orphan_files<DB: Db + DbInputs>(db: &mut DB, file: File) {
     let orphan = db.orphan_root();
-    if let Some(files) = with_appended(orphan.files(db), file) {
+    if let Some(files) = with_cow_insert(orphan.files(db), file) {
         orphan.set_files(db).to(files);
     }
 }
 
-/// The container with `file` appended, or `None` if it's already there.
+/// The ordered container with `file` appended, or `None` if it's already there.
 ///
 /// `None` means nothing would change, so the caller skips the salsa write and
-/// the clone. This keeps the "clone only when the field actually changes"
-/// rule in one place, shared by every container update on `Root` / `Package` /
-/// `OrphanRoot`.
-pub(crate) fn with_appended<T: Clone + PartialEq>(files: &[T], file: T) -> Option<Vec<T>> {
+/// the clone. This keeps the "clone only when the field actually changes" rule
+/// in one place, shared by the ordered container updates on `Root` and
+/// `Package`. See [`with_inserted`] / [`with_discarded`] for the unordered
+/// `OrphanRoot` / `StaleRoot` sets.
+pub(crate) fn with_cow_push<T: Clone + PartialEq>(files: &[T], file: T) -> Option<Vec<T>> {
     if files.contains(&file) {
         return None;
     }
@@ -422,11 +424,40 @@ pub(crate) fn with_appended<T: Clone + PartialEq>(files: &[T], file: T) -> Optio
     Some(updated)
 }
 
-/// The container with `file` removed, or `None` if it wasn't there. `None`
-/// means nothing would change, see [`with_file_appended`].
-pub(crate) fn with_removed<T: Clone + PartialEq>(files: &[T], file: T) -> Option<Vec<T>> {
+/// The ordered container with `file` removed, or `None` if it wasn't there.
+/// `None` means nothing would change, see [`with_appended`].
+pub(crate) fn with_cow_filter<T: Clone + PartialEq>(files: &[T], file: T) -> Option<Vec<T>> {
     if !files.contains(&file) {
         return None;
     }
     Some(files.iter().filter(|f| **f != file).cloned().collect())
 }
+
+/// The set with `item` inserted, or `None` if it's already present. The
+/// unordered counterpart of [`with_appended`], used for the `OrphanRoot` /
+/// `StaleRoot` sets where membership is all that matters.
+pub(crate) fn with_cow_insert<T: Clone + Eq + Hash>(
+    set: &HashSet<T>,
+    item: T,
+) -> Option<HashSet<T>> {
+    if set.contains(&item) {
+        return None;
+    }
+    let mut updated = set.clone();
+    updated.insert(item);
+    Some(updated)
+}
+
+/// The set with `item` removed, or `None` if it wasn't present. The unordered
+/// counterpart of [`with_removed`].
+pub(crate) fn with_cow_remove<T: Clone + Eq + Hash>(
+    set: &HashSet<T>,
+    item: T,
+) -> Option<HashSet<T>> {
+    if !set.contains(&item) {
+        return None;
+    }
+    let mut updated = set.clone();
+    updated.remove(&item);
+    Some(updated)
+}

crates/oak_scan/src/stale.rs

@@ -35,6 +35,9 @@ use oak_db::Root;
 use rustc_hash::FxHashMap;
 use salsa::Setter;
 
+use crate::inputs::with_cow_filter;
+use crate::inputs::with_cow_remove;
+
 /// Drop `root` from its live container, rehoming files and packages to
 /// `OrphanRoot` / `StaleRoot` as described in the module doc.
 ///
@@ -77,22 +80,14 @@ pub(crate) fn set_root_stale<DB: Db + DbInputs>(
     if !to_orphan.is_empty() {
         let orphan = db.orphan_root();
         let mut files = orphan.files(db).clone();
-        for file in to_orphan {
-            if !files.contains(&file) {
-                files.push(file);
-            }
-        }
+        files.extend(to_orphan);
         orphan.set_files(db).to(files);
     }
 
     let stale = db.stale_root();
     if !to_stale.is_empty() {
         let mut files = stale.files(db).clone();
-        for file in to_stale {
-            if !files.contains(&file) {
-                files.push(file);
-            }
-        }
+        files.extend(to_stale);
         stale.set_files(db).to(files);
     }
 
@@ -120,22 +115,16 @@ pub(crate) fn set_root_stale<DB: Db + DbInputs>(
 
 pub(crate) fn remove_from_stale_files<DB: Db + DbInputs>(db: &mut DB, file: File) {
     let stale = db.stale_root();
-    if !stale.files(db).contains(&file) {
-        return;
+    if let Some(files) = with_cow_remove(stale.files(db), file) {
+        stale.set_files(db).to(files);
     }
-    let mut files = stale.files(db).clone();
-    files.retain(|f| *f != file);
-    stale.set_files(db).to(files);
 }
 
 pub(crate) fn remove_from_stale_packages<DB: Db + DbInputs>(db: &mut DB, pkg: Package) {
     let stale = db.stale_root();
-    if !stale.packages(db).contains(&pkg) {
-        return;
+    if let Some(packages) = with_cow_filter(stale.packages(db), pkg) {
+        stale.set_packages(db).to(packages);
     }
-    let mut packages = stale.packages(db).clone();
-    packages.retain(|p| *p != pkg);
-    stale.set_packages(db).to(packages);
 }
 
 /// Look up a stale `File` by URL. The scanner's upsert helpers call this to

crates/oak_scan/src/watch.rs

@@ -36,8 +36,8 @@ use salsa::Setter;
 use crate::inputs::remove_from_orphan;
 use crate::inputs::remove_from_pkg_files;
 use crate::inputs::upsert_root_file;
-use crate::inputs::with_appended;
-use crate::inputs::with_removed;
+use crate::inputs::with_cow_filter;
+use crate::inputs::with_cow_push;
 use crate::inputs::FileEntry;
 use crate::packages::classify_in_package;
 use crate::packages::is_r_file;
@@ -159,17 +159,17 @@ pub(crate) fn add_watched_file<DB: Db + DbInputs>(db: &mut DB, url: UrlId, conte
 fn append_to_container<DB: Db + DbInputs>(db: &mut DB, file: File, placement: Placement) {
     match placement {
         Placement::Script(root) => {
-            if let Some(scripts) = with_appended(root.scripts(db), file) {
+            if let Some(scripts) = with_cow_push(root.scripts(db), file) {
                 root.set_scripts(db).to(scripts);
             }
         },
         Placement::PackageFile(pkg) => {
-            if let Some(files) = with_appended(pkg.files(db), file) {
+            if let Some(files) = with_cow_push(pkg.files(db), file) {
                 pkg.set_files(db).to(files);
             }
         },
         Placement::PackageScript(pkg) => {
-            if let Some(scripts) = with_appended(pkg.scripts(db), file) {
+            if let Some(scripts) = with_cow_push(pkg.scripts(db), file) {
                 pkg.set_scripts(db).to(scripts);
             }
         },
@@ -192,7 +192,7 @@ pub(crate) fn remove_watched_file<DB: Db + DbInputs>(db: &mut DB, url: UrlId) {
     }
 
     for &root in &db.workspace_roots().roots(db).clone() {
-        if let Some(scripts) = with_removed(root.scripts(db), file) {
+        if let Some(scripts) = with_cow_filter(root.scripts(db), file) {
             root.set_scripts(db).to(scripts);
             return;
         }

crates/oak_scan/tests/integration/library.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::fs;
 use std::path::Path;
 use std::path::PathBuf;
@@ -378,7 +379,9 @@ fn test_upsert_re_promotes_editor_owned_file_from_orphan() {
     // Editor opens the file before any scan -> orphan.
     let r_url = file_url("/lib/pkg/R/a.R");
     let file = File::new(&db, r_url.clone(), "editor content".to_string(), None);
-    db.orphan_root().set_files(&mut db).to(vec![file]);
+    db.orphan_root()
+        .set_files(&mut db)
+        .to(HashSet::from([file]));
     assert_eq!(file.package(&db), None);
 
     // Now a library scan picks up the same URL as part of a package.