diff --git a/libs/gds/src/write.rs b/libs/gds/src/write.rs index 4bc6bc3b1..88ad0df55 100644 --- a/libs/gds/src/write.rs +++ b/libs/gds/src/write.rs @@ -12,14 +12,15 @@ pub struct GdsWriter<'wr> { impl<'wr> GdsWriter<'wr> { /// Creates new [GdsWriter] with destination file `fname`. pub fn open(fname: impl AsRef) -> GdsResult { - let file = BufWriter::new(File::create(fname)?); - Ok(Self::new(file)) + Ok(Self::new(File::create(fname)?)) } /// Creates a new [GdsWriter] to destination `dest`. + /// + /// Writes to `dest` are buffered, since records are encoded a few bytes at a time. pub fn new(dest: impl Write + 'wr) -> Self { Self { - dest: Box::new(dest), + dest: Box::new(BufWriter::new(dest)), } } @@ -27,7 +28,10 @@ impl<'wr> GdsWriter<'wr> { pub fn write_lib(&mut self, lib: &GdsLibrary) -> GdsResult<()> { // `write_lib` is our typicaly entry point when writing to file. // It quickly dispatches most behavior off to our implementation of the [Encode] trait. - self.encode_lib(lib) + self.encode_lib(lib)?; + // Flush explicitly: dropping the buffer would discard any write error. + self.dest.flush()?; + Ok(()) } /// Helper to write a sequence of [GdsRecord] references. diff --git a/libs/uniquify/src/lib.rs b/libs/uniquify/src/lib.rs index 31db16240..8ac4d7579 100644 --- a/libs/uniquify/src/lib.rs +++ b/libs/uniquify/src/lib.rs @@ -9,10 +9,17 @@ use serde::{Deserialize, Serialize}; /// A set of unique names. /// /// Each key of type `K` is assigned a unique name. -#[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct Names { names: HashSet, assignments: HashMap, + /// The suffix at which [`Names::assign_name`] starts searching for a free name, + /// per base name. + /// + /// Every lower suffix is in use. This only speeds up the search, so it is + /// neither serialized nor compared. + #[serde(skip)] + next_suffix: HashMap, } impl Default for Names { @@ -20,10 +27,19 @@ impl Default for Names { Self { names: HashSet::new(), assignments: HashMap::new(), + next_suffix: HashMap::new(), } } } +impl PartialEq for Names { + fn eq(&self, other: &Self) -> bool { + self.names == other.names && self.assignments == other.assignments + } +} + +impl Eq for Names {} + impl Names { /// Creates a new, empty name set. #[inline] @@ -37,6 +53,7 @@ impl Names { Self { names: HashSet::with_capacity(capacity), assignments: HashMap::with_capacity(capacity), + next_suffix: HashMap::new(), } } @@ -65,13 +82,15 @@ impl Names { /// The name will be based on the given `base_name`. pub fn assign_name(&mut self, id: K, base_name: &str) -> ArcStr { let name = if self.names.contains(base_name) { - let mut i = 1; + // Resume where the previous search for this base name stopped. Scanning from 1 + // every time is quadratic in the number of keys sharing a base name. + let i = self.next_suffix.entry(base_name.into()).or_insert(1); loop { let new_name = arcstr::format!("{}_{}", base_name, i); + *i += 1; if !self.names.contains(&new_name) { break new_name; } - i += 1; } } else { base_name.into() @@ -89,9 +108,33 @@ impl Names { pub fn unassign(&mut self, id: &K) -> bool { if let Some(name) = self.assignments.remove(id) { self.names.remove(&name); + // If `name` has the form `{base}_{i}`, suffix `i` is free again for `base`. + if let Some((base, suffix)) = name.rsplit_once('_') + && let (Some(next), Ok(i)) = (self.next_suffix.get_mut(base), suffix.parse()) + { + *next = std::cmp::min(*next, i); + } true } else { false } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn assign_name_reuses_unassigned_suffixes() { + let mut names = Names::new(); + assert_eq!(names.assign_name(0, "a"), "a"); + assert_eq!(names.assign_name(1, "a"), "a_1"); + assert_eq!(names.assign_name(2, "a"), "a_2"); + assert!(names.reserve_name(3, "a_3")); + assert_eq!(names.assign_name(4, "a"), "a_4"); + assert!(names.unassign(&1)); + assert_eq!(names.assign_name(5, "a"), "a_1"); + assert_eq!(names.assign_name(6, "a"), "a_5"); + } +}