From e3ee1b359e0e549865a92bf0634fd71df4ff3b9f Mon Sep 17 00:00:00 2001 From: bherbruck Date: Sat, 10 Oct 2026 19:04:19 -0400 Subject: [PATCH] Fix cubic input-name repair when loading large designs --- apps/solvecraft-cli/src/main.rs | 34 ++++++++++++++++ crates/doc/src/format.rs | 69 +++++++++++++++++++++++++++++++-- crates/doc/src/params.rs | 49 +++++++++++++++++++++++ docs/perf.md | 32 +++++++++++++++ 4 files changed, 180 insertions(+), 4 deletions(-) diff --git a/apps/solvecraft-cli/src/main.rs b/apps/solvecraft-cli/src/main.rs index a3225871..093ef200 100644 --- a/apps/solvecraft-cli/src/main.rs +++ b/apps/solvecraft-cli/src/main.rs @@ -3,6 +3,7 @@ //! ```text //! solvecraft-cli run [--in design|part.step|mesh.3mf] [--out FILE]... [--save FILE] [--quiet] //! solvecraft-cli eval (alias: inspect) measurements as JSON +//! solvecraft-cli bench-open [--parse-only] loading timings as JSON lines //! solvecraft-cli snapshot --out shot.png [--width W] [--height H] [--view iso|front|top|…] //! solvecraft-cli exec [json-params] run one command on an empty design //! solvecraft-cli commands the command registry as JSON @@ -31,6 +32,7 @@ const USAGE: &str = "usage: solvecraft-cli run [--in design.solvecraft|part.step] [--out FILE]... [--save FILE] [--quiet] [--results] solvecraft-cli run --in [--out FILE]... solvecraft-cli eval (alias: inspect; or --in FILE) + solvecraft-cli bench-open [--parse-only] solvecraft-cli snapshot --out shot.png [--width W] [--height H] [--view iso|front|back|top|bottom|left|right] solvecraft-cli exec [json-params] solvecraft-cli commands @@ -74,6 +76,7 @@ fn main() -> ExitCode { Some("save-stress") => cmd_save_stress(&args[1..]), // Hidden: time recomputes after parameter edits (docs/perf.md). Some("bench-edit") => cmd_bench_edit(&args[1..]), + Some("bench-open") => cmd_bench_open(&args[1..]), Some("--version" | "-V") => { println!("solvecraft-cli {}", env!("CARGO_PKG_VERSION")); Ok(()) @@ -351,3 +354,34 @@ fn cmd_bench_edit(args: &[String]) -> Result<(), String> { println!("{}", pretty(&json!(out))); Ok(()) } + +/// Separate file reading, JSON decoding/repair, timeline evaluation and display meshing. +fn cmd_bench_open(args: &[String]) -> Result<(), String> { + let path = args.first().ok_or("usage: solvecraft-cli bench-open [--parse-only]")?; + let t = std::time::Instant::now(); + let len = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?.len(); + if len > solvecraft_engine::io::MAX_DESIGN_BYTES as u64 { + return Err(format!("{path}: file too large")); + } + let bytes = std::fs::read(path).map_err(|e| format!("{path}: {e}"))?; + let read_ms = t.elapsed().as_secs_f64() * 1000.0; + println!("{}", json!({"step": "read", "ms": read_ms, "bytes": bytes.len()})); + let t = std::time::Instant::now(); + let doc = solvecraft_engine::io::read_design(&bytes).map_err(|e| e.to_string())?; + let parse_ms = t.elapsed().as_secs_f64() * 1000.0; + println!("{}", json!({"step": "parse", "ms": parse_ms, "features": doc.features.len()})); + if args.iter().any(|a| a == "--parse-only") { + return Ok(()); + } + let t = std::time::Instant::now(); + let s = Session::new(doc); + println!("{}", json!({"step": "evaluate", "ms": t.elapsed().as_secs_f64() * 1000.0})); + for r in &s.model.results { + println!("{}", json!({"step": "feature", "id": r.id, "name": r.name, "ms": r.ms, "error": r.error})); + } + let t = std::time::Instant::now(); + let st = s.world_state(); + let triangles: usize = st.bodies.iter().map(|b| b.mesh().triangles.len()).sum(); + println!("{}", json!({"step": "mesh", "ms": t.elapsed().as_secs_f64() * 1000.0, "bodies": st.bodies.len(), "triangles": triangles})); + Ok(()) +} diff --git a/crates/doc/src/format.rs b/crates/doc/src/format.rs index cf0328ec..4b94a488 100644 --- a/crates/doc/src/format.rs +++ b/crates/doc/src/format.rs @@ -197,10 +197,7 @@ fn rigid(m: &crate::Mat) -> bool { /// Feature input names and component occurrences, which format 1 files may predate. fn normalise(d: &mut Document) -> Result<()> { - let ids: Vec = d.features.iter().map(|f| f.id).collect(); - for id in ids { - d.name_feature_inputs(id); - } + d.name_all_feature_inputs(); let missing: Vec<(u64, u64)> = d.components.iter().filter(|c| !d.occurrences.iter().any(|o| o.component == c.id)).map(|c| (c.id, c.parent)).collect(); for (c, p) in missing { @@ -213,6 +210,70 @@ fn normalise(d: &mut Document) -> Result<()> { mod tests { use super::*; + #[test] + fn bulk_input_repair_preserves_existing_names_and_reuses_surplus_names() { + let mut d = Document::new("input repair"); + d.set_param("d2", "1", None, None).unwrap(); + for i in 0..5 { + d.add_feature( + crate::FeatureKind::Box { + corner: ["0".into(), "0".into(), "0".into()], + length: "10".into(), + width: "10".into(), + height: "10".into(), + operation: crate::Operation::NewBody, + }, + Some(&format!("Box{i}")), + ) + .unwrap(); + } + d.features[0].param_names = vec!["".into(), "custom".into(), "".into(), "d1".into(), "d2".into()]; + d.features[1].param_names.clear(); + d.features[2].param_names = vec!["d7".into(), "".into(), "d9".into()]; + d.features[3].param_names = vec!["d7".into(), "d11".into(), "d12".into(), "d1".into()]; + d.features[4].param_names.clear(); + let mut expected = d.clone(); + for id in expected.features.iter().map(|f| f.id).collect::>() { + expected.name_feature_inputs(id); + } + d.name_all_feature_inputs(); + assert_eq!(d, expected); + assert_eq!(read(&d.to_json()).unwrap(), d); + } + + #[test] + fn large_unnamed_timeline_loads_with_unique_inputs() { + let mut d = Document::new("large input repair"); + let id = d + .add_feature( + crate::FeatureKind::Box { + corner: ["0".into(), "0".into(), "0".into()], + length: "10".into(), + width: "10".into(), + height: "10".into(), + operation: crate::Operation::NewBody, + }, + None, + ) + .unwrap(); + let mut f = d.feature(id).unwrap().clone(); + f.param_names.clear(); + d.features = (1..=5000) + .map(|id| { + let mut f = f.clone(); + f.id = id; + f + }) + .collect(); + d.next_id = 5001; + let back = read(&d.to_json()).unwrap(); + let names: Vec<_> = back.features.iter().flat_map(|f| &f.param_names).collect(); + assert_eq!(names.len(), 15000); + assert_eq!(names.first().unwrap().as_str(), "d1"); + assert_eq!(names.last().unwrap().as_str(), "d15000"); + assert_eq!(names.iter().collect::>().len(), names.len()); + } + #[test] fn versions() { assert_eq!(version_of("solvecraft/1"), Some(1)); diff --git a/crates/doc/src/params.rs b/crates/doc/src/params.rs index 03d791de..73e7c69a 100644 --- a/crates/doc/src/params.rs +++ b/crates/doc/src/params.rs @@ -398,6 +398,55 @@ impl Document { } } + /// Repair a loaded timeline in one pass. Reserve even later features' names, just as + /// `name_feature_inputs` does, without restarting a document scan for every `dN` candidate. + pub(crate) fn name_all_feature_inputs(&mut self) { + let mut taken: BTreeMap = BTreeMap::new(); + for name in self.params.iter().map(|p| &p.name).chain(self.features.iter().flat_map(|f| &f.param_names)) { + *taken.entry(name.clone()).or_default() += 1; + } + let mut next = 1usize; + for f in &mut self.features { + let n = f.kind.inputs().len(); + for k in 0..n { + if f.param_names.get(k).is_some_and(|name| !name.is_empty()) { + continue; + } + let name = loop { + let name = format!("d{next}"); + next += 1; + if !taken.contains_key(&name) { + break name; + } + }; + if f.param_names.len() <= k { + f.param_names.resize(k + 1, String::new()); + } + // Empty slots cannot collide with a generated name. + if let Some(slot) = f.param_names.get_mut(k) { + *slot = name.clone(); + } + taken.insert(name, 1); + } + // Match the single-feature repair: surplus names become available only after + // this feature has been repaired. Counts preserve names shared with parameters. + for name in f.param_names.drain(n..) { + if let Some(count) = taken.get_mut(&name) { + *count -= 1; + if *count == 0 { + taken.remove(&name); + if let Some(i) = name.strip_prefix('d').and_then(|s| s.parse::().ok()) + && name == format!("d{i}") + && i > 0 + { + next = next.min(i); + } + } + } + } + } + } + /// Change a parameter: a stored one, or a feature input by its name. /// A typed length that is a bare number means the design's units: it gets that unit written /// in (feature inputs are stored in mm otherwise). Anything else is returned unchanged. diff --git a/docs/perf.md b/docs/perf.md index ee864ceb..b1d29fea 100644 --- a/docs/perf.md +++ b/docs/perf.md @@ -1,3 +1,35 @@ +# Design loading + +`solvecraft-cli bench-open design.solvecraft` reports JSON lines for file reading, +JSON decoding and migrations (including input-name repair), timeline evaluation, individual +features, and world placement/display meshing. `--parse-only` stops before geometry evaluation. +The parse timer includes document validation and repair, not just the JSON parser. + +Windows x64, optimized `ci` profile, measured during other builds: + +| Design | Parse/repair | Evaluate | Display meshes | +|---|---:|---:|---:| +| 19-feature enclosure, 34 KB | 1.2 ms | 14.9 s | 6 ms | +| 25-feature assembly, 15.2 MB, 2,990 bodies | 21 ms | 12.5 s | 2.1 s | +| Generated 1,000 independent boxes, 185 KB | 5 ms | 0.82 s | 0.15 s | +| Generated 5,000 boxes in five grids, 10 features | 0.45 ms | 17.6 s | 0.81 s | + +The enclosures spend most of evaluation in joins and cuts, not parsing. Designs save feature +definitions; opening them rebuilds their geometry. Those times remain after the repair fix. + +A separate generated timeline with missing input names reproduced minutes in the reader: + +| Timeline (geometry rolled back) | Parse/repair before | After | +|---|---:|---:| +| 2,000 boxes, 6,000 unnamed inputs | 78.1 s | 10–20 ms | +| 5,000 boxes, 15,000 unnamed inputs | interrupted after 131 s, still repairing | 26–46 ms | + +Previously, each missing input restarted the `d1`, `d2`, … search, scanning the document for +every candidate. This approaches cubic work as unnamed inputs accumulate. Loading now reserves +names once and assigns missing names with an advancing cursor. Existing names, names on later +features, parameter collisions and reuse of surplus names retain the previous behavior. +Regression tests compare the old and bulk repairs and load a 5,000-feature unnamed timeline. + # Viewport performance Measured with the control channel (`ui.inspect` `frame_ms`: CPU time of one UI frame, which