diff --git a/Cargo.lock b/Cargo.lock index 04299a6cd3c899c9da0f9eb1c2d3110ceed26ef3..d25b76456c75b7b98e727fe297de25154eb37e32 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16022,6 +16022,7 @@ dependencies = [ "anyhow", "assistant_text_thread", "chrono", + "collections", "editor", "feature_flags", "fs", diff --git a/crates/collections/Cargo.toml b/crates/collections/Cargo.toml index 8675504347f171397ea7372841cb00b7959eafe3..aa3dd899a7222f38377ea5f62927eea23534d1d8 100644 --- a/crates/collections/Cargo.toml +++ b/crates/collections/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition.workspace = true publish = false license = "Apache-2.0" -description = "Standard collection type re-exports used by Zed and GPUI" +description = "Standard collection types used by Zed and GPUI" [lints] workspace = true diff --git a/crates/collections/src/collections.rs b/crates/collections/src/collections.rs index ea5ea7332fb14e5e2ac33ba2d6f957dbfdc28c7a..8e6c334d2bd5d544f36666184df5fe095c3fdbe1 100644 --- a/crates/collections/src/collections.rs +++ b/crates/collections/src/collections.rs @@ -7,3 +7,7 @@ pub use indexmap::Equivalent; pub use rustc_hash::FxHasher; pub use rustc_hash::{FxHashMap, FxHashSet}; pub use std::collections::*; + +pub mod vecmap; +#[cfg(test)] +mod vecmap_tests; diff --git a/crates/collections/src/vecmap.rs b/crates/collections/src/vecmap.rs new file mode 100644 index 0000000000000000000000000000000000000000..97972e7bf9f5ae43957648b8a44b10e3e45bc32f --- /dev/null +++ b/crates/collections/src/vecmap.rs @@ -0,0 +1,119 @@ +/// A collection that provides a map interface but is backed by vectors. +/// +/// This is suitable for small key-value stores where the item count is not +/// large enough to overcome the overhead of a more complex algorithm. +/// +/// If this meets your use cases, then [`VecMap`] should be a drop-in +/// replacement for [`std::collections::HashMap`] or [`crate::HashMap`]. Note +/// that we are adding APIs on an as-needed basis. If the API you need is not +/// present yet, please add it! +/// +/// Because it uses vectors as a backing store, the map also iterates over items +/// in insertion order, like [`crate::IndexMap`]. +/// +/// This struct uses a struct-of-arrays (SoA) representation which tends to be +/// more cache efficient and promotes autovectorization when using simple key or +/// value types. +#[derive(Default)] +pub struct VecMap { + keys: Vec, + values: Vec, +} + +impl VecMap { + pub fn new() -> Self { + Self { + keys: Vec::new(), + values: Vec::new(), + } + } + + pub fn iter(&self) -> Iter<'_, K, V> { + Iter { + iter: self.keys.iter().zip(self.values.iter()), + } + } +} + +impl VecMap { + pub fn entry(&mut self, key: K) -> Entry<'_, K, V> { + match self.keys.iter().position(|k| k == &key) { + Some(index) => Entry::Occupied(OccupiedEntry { + key: &self.keys[index], + value: &mut self.values[index], + }), + None => Entry::Vacant(VacantEntry { map: self, key }), + } + } +} + +pub struct Iter<'a, K, V> { + iter: std::iter::Zip, std::slice::Iter<'a, V>>, +} + +impl<'a, K, V> Iterator for Iter<'a, K, V> { + type Item = (&'a K, &'a V); + + fn next(&mut self) -> Option { + self.iter.next() + } +} + +pub enum Entry<'a, K, V> { + Occupied(OccupiedEntry<'a, K, V>), + Vacant(VacantEntry<'a, K, V>), +} + +impl<'a, K, V> Entry<'a, K, V> { + pub fn key(&self) -> &K { + match self { + Entry::Occupied(entry) => entry.key, + Entry::Vacant(entry) => &entry.key, + } + } + + pub fn or_insert_with_key(self, default: F) -> &'a mut V + where + F: FnOnce(&K) -> V, + { + match self { + Entry::Occupied(entry) => entry.value, + Entry::Vacant(entry) => { + entry.map.values.push(default(&entry.key)); + entry.map.keys.push(entry.key); + match entry.map.values.last_mut() { + Some(value) => value, + None => unreachable!("vec empty after pushing to it"), + } + } + } + } + + pub fn or_insert_with(self, default: F) -> &'a mut V + where + F: FnOnce() -> V, + { + self.or_insert_with_key(|_| default()) + } + + pub fn or_insert(self, value: V) -> &'a mut V { + self.or_insert_with_key(|_| value) + } + + pub fn or_insert_default(self) -> &'a mut V + where + V: Default, + { + self.or_insert_with_key(|_| Default::default()) + } +} + +pub struct OccupiedEntry<'a, K, V> { + key: &'a K, + value: &'a mut V, +} + +pub struct VacantEntry<'a, K, V> { + map: &'a mut VecMap, + key: K, +} diff --git a/crates/collections/src/vecmap_tests.rs b/crates/collections/src/vecmap_tests.rs new file mode 100644 index 0000000000000000000000000000000000000000..9fc7d430f3422374a7662bb476cbd99dd09d9a43 --- /dev/null +++ b/crates/collections/src/vecmap_tests.rs @@ -0,0 +1,96 @@ +//! Tests for the VecMap collection. +//! +//! This is in a sibling module so that the tests are guaranteed to only cover +//! states that can be created by the public API. + +use crate::vecmap::*; + +#[test] +fn test_entry_vacant_or_insert() { + let mut map: VecMap<&str, i32> = VecMap::new(); + let value = map.entry("a").or_insert(1); + assert_eq!(*value, 1); + assert_eq!(map.iter().collect::>(), vec![(&"a", &1)]); +} + +#[test] +fn test_entry_occupied_or_insert_keeps_existing() { + let mut map: VecMap<&str, i32> = VecMap::new(); + map.entry("a").or_insert(1); + let value = map.entry("a").or_insert(99); + assert_eq!(*value, 1); + assert_eq!(map.iter().collect::>(), vec![(&"a", &1)]); +} + +#[test] +fn test_entry_or_insert_with() { + let mut map: VecMap<&str, i32> = VecMap::new(); + map.entry("a").or_insert_with(|| 42); + assert_eq!(map.iter().collect::>(), vec![(&"a", &42)]); +} + +#[test] +fn test_entry_or_insert_with_not_called_when_occupied() { + let mut map: VecMap<&str, i32> = VecMap::new(); + map.entry("a").or_insert(1); + map.entry("a") + .or_insert_with(|| panic!("should not be called")); + assert_eq!(map.iter().collect::>(), vec![(&"a", &1)]); +} + +#[test] +fn test_entry_or_insert_with_key() { + let mut map: VecMap<&str, String> = VecMap::new(); + map.entry("hello").or_insert_with_key(|k| k.to_uppercase()); + assert_eq!( + map.iter().collect::>(), + vec![(&"hello", &"HELLO".to_string())] + ); +} + +#[test] +fn test_entry_or_insert_default() { + let mut map: VecMap<&str, i32> = VecMap::new(); + map.entry("a").or_insert_default(); + assert_eq!(map.iter().collect::>(), vec![(&"a", &0)]); +} + +#[test] +fn test_entry_key() { + let mut map: VecMap<&str, i32> = VecMap::new(); + assert_eq!(*map.entry("a").key(), "a"); + map.entry("a").or_insert(1); + assert_eq!(*map.entry("a").key(), "a"); +} + +#[test] +fn test_entry_mut_ref_can_be_updated() { + let mut map: VecMap<&str, i32> = VecMap::new(); + let value = map.entry("a").or_insert(0); + *value = 5; + assert_eq!(map.iter().collect::>(), vec![(&"a", &5)]); +} + +#[test] +fn test_insertion_order_preserved() { + let mut map: VecMap<&str, i32> = VecMap::new(); + map.entry("b").or_insert(2); + map.entry("a").or_insert(1); + map.entry("c").or_insert(3); + assert_eq!( + map.iter().collect::>(), + vec![(&"b", &2), (&"a", &1), (&"c", &3)] + ); +} + +#[test] +fn test_multiple_entries_independent() { + let mut map: VecMap = VecMap::new(); + map.entry(1).or_insert(10); + map.entry(2).or_insert(20); + map.entry(3).or_insert(30); + assert_eq!(map.iter().count(), 3); + // Re-inserting does not duplicate keys + map.entry(1).or_insert(99); + assert_eq!(map.iter().count(), 3); +} diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index 179d8c40135bece9d5e85142cb47bc32fe236218..a75a6b1af7a26723c1691b27676072c1869b5847 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -23,6 +23,7 @@ agent_settings.workspace = true agent_ui.workspace = true anyhow.workspace = true chrono.workspace = true +collections.workspace = true editor.workspace = true feature_flags.workspace = true fs.workspace = true diff --git a/crates/sidebar/src/project_group_builder.rs b/crates/sidebar/src/project_group_builder.rs index ae3b64d424e43b4d1712d437418ccaf5e7e81a79..bab0060186a7cae2d79aaead61946a15bf109a5a 100644 --- a/crates/sidebar/src/project_group_builder.rs +++ b/crates/sidebar/src/project_group_builder.rs @@ -8,8 +8,8 @@ //! This module is provides the functions and structures necessary to do this //! lookup and mapping. +use collections::{HashMap, HashSet, vecmap::VecMap}; use std::{ - collections::{HashMap, HashSet}, path::{Path, PathBuf}, sync::Arc, }; @@ -78,16 +78,14 @@ impl ProjectGroup { pub struct ProjectGroupBuilder { /// Maps git repositories' work_directory_abs_path to their original_repo_abs_path directory_mappings: HashMap, - project_group_names: Vec, - project_groups: Vec, + project_groups: VecMap, } impl ProjectGroupBuilder { fn new() -> Self { Self { - directory_mappings: HashMap::new(), - project_group_names: Vec::new(), - project_groups: Vec::new(), + directory_mappings: HashMap::default(), + project_groups: VecMap::new(), } } @@ -113,15 +111,7 @@ impl ProjectGroupBuilder { } fn project_group_entry(&mut self, name: &ProjectGroupName) -> &mut ProjectGroup { - match self.project_group_names.iter().position(|n| n == name) { - Some(idx) => &mut self.project_groups[idx], - None => { - let idx = self.project_group_names.len(); - self.project_group_names.push(name.clone()); - self.project_groups.push(ProjectGroup::default()); - &mut self.project_groups[idx] - } - } + self.project_groups.entry(name.clone()).or_insert_default() } fn add_mapping(&mut self, work_directory: &Path, original_repo: &Path) { @@ -204,9 +194,7 @@ impl ProjectGroupBuilder { } pub fn groups(&self) -> impl Iterator { - self.project_group_names - .iter() - .zip(self.project_groups.iter()) + self.project_groups.iter() } }