diff --git a/crates/doc/src/handoff_tests.rs b/crates/doc/src/handoff_tests.rs index 79b97194..8a18151d 100644 --- a/crates/doc/src/handoff_tests.rs +++ b/crates/doc/src/handoff_tests.rs @@ -85,3 +85,23 @@ fn validate_is_linear_in_a_wide_parent() { doc.get_mut(page).unwrap().children.retain(|c| *c != kid); assert!(doc.validate().iter().any(|m| m.contains("not listed in parent"))); } + +/// Repair lists every unlisted page once, by set lookups rather than a scan of the page list per +/// page (#1078: a file with many pages made repair quadratic). +#[test] +fn repair_relists_many_unlisted_pages_once_each() { + let mut doc = Document::new("d"); + for i in 0..5_000 { + doc.add_page(&format!("P{i}")); + } + let all: Vec<_> = doc.pages.clone(); + // Half the pages fall off the list. + doc.pages.retain(|p| p.0 % 2 == 0); + doc.repair(); + let mut listed = doc.pages.clone(); + listed.sort_unstable(); + let mut want = all; + want.sort_unstable(); + assert_eq!(listed, want, "every page is listed again, once"); + assert!(doc.validate().is_empty()); +} diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index caf0c861..7d1f6928 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -1143,8 +1143,10 @@ impl Document { Arc::make_mut(&mut self.libraries).store = store; } self.pages.retain(|p| self.nodes.get(p).is_some_and(|n| n.is_page()) && Some(*p) != store); + // Unlisted pages join the list; a set keeps many pages from making this quadratic (#1078). + let mut listed: std::collections::BTreeSet = self.pages.iter().copied().collect(); for (k, n) in self.nodes.iter() { - if n.is_page() && !self.pages.contains(k) && Some(*k) != store { + if n.is_page() && Some(*k) != store && listed.insert(*k) { self.pages.push(*k); } } diff --git a/crates/format/src/library.rs b/crates/format/src/library.rs index daec05e2..12d38e94 100644 --- a/crates/format/src/library.rs +++ b/crates/format/src/library.rs @@ -90,9 +90,13 @@ pub(crate) fn read( } // Keep what the snapshot really holds; clip what people see. manifest.components.retain(|a| doc.node(a.id).is_some_and(|n| matches!(n.kind, NodeKind::Component { .. } | NodeKind::ComponentSet { .. }))); - manifest.styles.retain(|s| doc.styles.iter().any(|x| x.id == s.id)); - manifest.collections.retain(|c| doc.variables.collections.iter().any(|x| x.id == c.id)); - manifest.variables.retain(|v| doc.variables.variables.iter().any(|x| x.id == v.id && x.collection == v.collection)); + // Membership by set lookups: a long manifest against a large snapshot stays linear (#1078). + let styles: BTreeSet<&str> = doc.styles.iter().map(|x| x.id.as_str()).collect(); + let collections: BTreeSet<&str> = doc.variables.collections.iter().map(|x| x.id.as_str()).collect(); + let variables: BTreeSet<(&str, &str)> = doc.variables.variables.iter().map(|x| (x.id.as_str(), x.collection.as_str())).collect(); + manifest.styles.retain(|s| styles.contains(s.id.as_str())); + manifest.collections.retain(|c| collections.contains(c.id.as_str())); + manifest.variables.retain(|v| variables.contains(&(v.id.as_str(), v.collection.as_str()))); manifest.name = manifest.name.chars().take(MAX_NAME).collect(); manifest.description = manifest.description.chars().take(MAX_DESCRIPTION).collect(); Ok(Some(PublishedLibrary { manifest, doc: Arc::new(doc) }))