Large STEP imports: no more multi-minute hang, shared STEP text, cancel and progress (#925)

crates/kernel/src/lib.rs

@@ -93,6 +93,7 @@ mod topo;
 mod tspline;
 #[cfg(test)]
 mod untouched_faces_tests;
+pub mod watch;
 
 pub use blend::{ChamferSide, chamfer, chamfer_sides, fillet, fillet_each, fillet_g2, fillet_max, fillet_max_each};
 pub use blend_corner::fillet_radii;
@@ -119,7 +120,7 @@ pub use prism_measure::certified_prism_equivalent;
 pub use sew::{EdgeSpec, FaceSpec, SurfSpec, sew};
 pub use splitface::{SplitTool, split_body, split_faces, split_faces_with_map};
 pub use step::step_export;
-pub use step_in::{ImportedBody, StepImport, StepNode, step_import, step_import_shared, step_orientation_errors, step_validate};
+pub use step_in::{ImportedBody, StepImport, StepNode, step_cache_alive, step_import, step_import_shared, step_orientation_errors, step_validate};
 pub use step_out::{ExportBody, ExportProduct, StepHeader, step_export_bodies, step_export_products};
 pub use surface_kind::{FaceSurface, face_surface};
 pub use surface_offset::offset_surface;

crates/kernel/src/step_in/mod.rs

@@ -257,6 +257,12 @@ struct Reader<'a> {
     unmeshed_checked: HashSet<u64>,
     bodies: Vec<ImportedBody>,
     workers: usize,
+    /// Whoever waits for the import (progress, Cancel).
+    watch: Option<Arc<dyn crate::watch::Watch>>,
+    /// Solid items in the file, the measure of the conversion's progress.
+    solid_items: usize,
+    /// The watch cancelled the import: the walk stops and the import fails.
+    cancelled: bool,
 }
 
 impl<'a> Reader<'a> {
@@ -280,9 +286,27 @@ impl<'a> Reader<'a> {
             unmeshed_checked: HashSet::new(),
             bodies: Vec::new(),
             workers,
+            watch: None,
+            solid_items: 0,
+            cancelled: false,
         }
     }
 
+    /// Report progress; true once the import is cancelled (the walk then unwinds).
+    fn stop(&mut self) -> bool {
+        if self.cancelled {
+            return true;
+        }
+        if let Some(w) = &self.watch {
+            if w.cancelled() {
+                self.cancelled = true;
+                return true;
+            }
+            w.progress("converting", self.bodies.len().min(self.solid_items), self.solid_items);
+        }
+        false
+    }
+
     fn warn(&mut self, msg: impl Into<String>) {
         *self.warnings.entry(msg.into()).or_default() += 1;
     }
@@ -584,6 +608,9 @@ impl<'a> Reader<'a> {
         let u = self.units_of(rep);
         let mut items = self.reps.get(&rep).map(|r| r.0.clone()).unwrap_or_default().into_iter().peekable();
         while let Some(item) = items.next() {
+            if self.stop() {
+                return;
+            }
             let Some(e) = self.ex.get(item) else { continue };
             match e.name() {
                 "MANIFOLD_SOLID_BREP" | "BREP_WITH_VOIDS" | "FACETED_BREP" | "SHELL_BASED_SURFACE_MODEL" => {
@@ -749,6 +776,9 @@ impl<'a> Reader<'a> {
     fn node(&mut self, pd: u64, local: mt::Matrix4, world: mt::Matrix4, path: &[String], depth: usize, stack: &mut Vec<u64>) -> StepNode {
         let name = self.product_name(pd);
         let mut node = StepNode { name: name.clone(), transform: to_array(&local), bodies: Vec::new(), children: Vec::new() };
+        if self.stop() {
+            return node;
+        }
         if depth > MAX_ASSEMBLY_DEPTH || stack.contains(&pd) {
             self.warn("assembly nested too deeply or recursive: some components were left out");
             return node;
@@ -933,40 +963,73 @@ pub fn step_validate(text: &str) -> std::result::Result<usize, String> {
     Ok(n)
 }
 
+/// A recent import: held weakly, so it lives only as long as someone uses it (the command that
+/// read the file, until the timeline has evaluated it), and strongly only when it is small.
+struct Recent {
+    key: (u64, usize),
+    import: std::sync::Weak<StepImport>,
+    /// Keeps a small import alive (never read).
+    _small: Option<Arc<StepImport>>,
+}
+
 /// Recent imports, so a file read for a command and then evaluated in the timeline is only
 /// read once.
-static RECENT: std::sync::Mutex<Vec<(u64, usize, std::sync::Arc<StepImport>)>> = std::sync::Mutex::new(Vec::new());
+static RECENT: std::sync::Mutex<Vec<Recent>> = std::sync::Mutex::new(Vec::new());
 
-/// [`step_import`] through a small cache of recent results (keyed by the text).
+/// Files up to this size stay cached after their last user is gone (a few cheap re-reads
+/// saved); a larger import is freed with the design that holds it.
+const KEEP_SMALL_BYTES: usize = 1 << 20;
+
+/// [`step_import`] through a small cache of recent results (keyed by the text). Keep the result
+/// while the same text is still to be read elsewhere (in the timeline): that read reuses it.
 pub fn step_import_shared(text: &str) -> Result<std::sync::Arc<StepImport>> {
     use std::hash::{Hash, Hasher};
     let mut h = std::collections::hash_map::DefaultHasher::new();
     text.hash(&mut h);
     let key = (h.finish(), text.len());
     if let Ok(cache) = RECENT.lock()
-        && let Some((_, _, imp)) = cache.iter().find(|(k, n, _)| (*k, *n) == key)
+        && let Some(imp) = cache.iter().filter(|r| r.key == key).find_map(|r| r.import.upgrade())
     {
-        return Ok(imp.clone());
+        return Ok(imp);
     }
     let imp = std::sync::Arc::new(step_import(text)?);
     if let Ok(mut cache) = RECENT.lock() {
-        cache.retain(|(k, n, _)| (*k, *n) != key);
-        cache.push((key.0, key.1, imp.clone()));
-        if cache.len() > 4 {
+        cache.retain(|r| r.key != key && r.import.strong_count() > 0);
+        let small = (text.len() <= KEEP_SMALL_BYTES).then(|| imp.clone());
+        cache.push(Recent { key, import: Arc::downgrade(&imp), _small: small });
+        while cache.len() > 4 {
             cache.remove(0);
         }
     }
     Ok(imp)
 }
 
+/// Imports the cache still holds alive, and how many STEP bytes they came from (tests and
+/// diagnostics: a closed design must not leave its import behind).
+pub fn step_cache_alive() -> (usize, usize) {
+    RECENT.lock().map(|c| c.iter().filter(|r| r.import.strong_count() > 0).fold((0, 0), |(n, b), r| (n + 1, b + r.key.1))).unwrap_or((0, 0))
+}
+
 /// Read a STEP file: its solids (as bodies, placed by the assembly structure) and the tree.
 pub fn step_import(text: &str) -> Result<StepImport> {
     step_import_with_workers(text, parallel::workers())
 }
 
 fn step_import_with_workers(text: &str, workers: usize) -> Result<StepImport> {
-    let ex = p21::parse(text).map_err(KernelError::Invalid)?;
+    let watch = crate::watch::current();
+    let cancelled = || watch.as_ref().is_some_and(|w| w.cancelled());
+    let ex = match p21::parse(text) {
+        Ok(ex) => ex,
+        Err(_) if cancelled() => return Err(crate::watch::cancelled_error("STEP import")),
+        Err(e) => return Err(KernelError::Invalid(e)),
+    };
     let mut rd = Reader::new(&ex, workers);
+    rd.solid_items = ex
+        .entities
+        .values()
+        .filter(|e| matches!(e.name(), "MANIFOLD_SOLID_BREP" | "BREP_WITH_VOIDS" | "FACETED_BREP" | "SHELL_BASED_SURFACE_MODEL"))
+        .count();
+    rd.watch = watch.clone();
     rd.index();
     // Roots: product definitions that are nobody's component.
     let child_pds: HashSet<u64> =
@@ -1006,6 +1069,10 @@ fn step_import_with_workers(text: &str, workers: usize) -> Result<StepImport> {
             tree.push(StepNode { name: "Import".into(), transform: to_array(&identity), bodies, children: Vec::new() });
         }
     }
+    // Cancelled: what was read so far is dropped here, not cached.
+    if rd.cancelled || cancelled() {
+        return Err(crate::watch::cancelled_error("STEP import"));
+    }
     if let Some(first) = ex.duplicates.first() {
         rd.warn(format!("{} entity ids are defined twice (first #{first}); the first definitions were used", ex.duplicates.len()));
     }

crates/kernel/src/step_in/p21.rs

@@ -399,6 +399,7 @@ pub fn parse(text: &str) -> Result<Exchange, String> {
     }
     lx.expect(b';')?;
     let mut ex = Exchange::default();
+    let watch = crate::watch::current();
     loop {
         lx.ws()?;
         if lx.peek().is_none() {
@@ -469,6 +470,14 @@ pub fn parse(text: &str) -> Result<Exchange, String> {
                         vec![lx.record()?]
                     };
                     lx.expect(b';')?;
+                    if ex.entities.len() % 4096 == 0
+                        && let Some(w) = &watch
+                    {
+                        if w.cancelled() {
+                            return err("STEP import cancelled");
+                        }
+                        w.progress("reading", lx.i, lx.s.len());
+                    }
                     if ex.entities.len() >= MAX_ENTITIES {
                         return err(format!("STEP file has more than {MAX_ENTITIES} entities"));
                     }

crates/kernel/src/step_in/tests.rs

@@ -915,3 +915,119 @@ fn faces_with_too_many_bounds_are_refused() {
     assert!(imp.warnings.iter().any(|w| w.contains("boundary loops")), "{:?}", imp.warnings);
     assert!(start.elapsed() < std::time::Duration::from_secs(10), "took {:?}", start.elapsed());
 }
+
+/// A board-like assembly for issue #883: `SOLIDCRAFT_STEP_PCB_FIXTURE=path` writes it (with
+/// `SOLIDCRAFT_STEP_PCB_COMPONENTS` and `SOLIDCRAFT_STEP_PCB_PINS`; 600 and 15 give 52 MB). Every component is
+/// its own product with a package body and its own pins (nothing shared), the worst case for
+/// a large import.
+#[test]
+#[ignore = "manual fixture generator for large STEP imports"]
+fn generate_board_assembly_fixture() {
+    use crate::step_out::{ExportBody, ExportProduct, StepHeader, step_export_products};
+    let count: usize = solidcraft_geom::env::var("STEP_PCB_COMPONENTS").map(|s| s.parse().unwrap()).unwrap_or(500);
+    let pins: usize = solidcraft_geom::env::var("STEP_PCB_PINS").map(|s| s.parse().unwrap()).unwrap_or(16);
+    let mut owned: Vec<Vec<Body>> = Vec::new();
+    for c in 0..count {
+        let w = 4.0 + (c % 7) as f64 * 0.37;
+        let d = 3.0 + (c % 5) as f64 * 0.29;
+        let mut bodies = vec![fillet_box(w, d, 1.2 + (c % 3) as f64 * 0.1)];
+        for p in 0..pins {
+            let side = if p % 2 == 0 { -0.6 } else { d + 0.6 };
+            let x = 0.3 + (p / 2) as f64 * (w - 0.6) / (pins / 2).max(1) as f64 + c as f64 * 1e-4;
+            bodies.push(cylinder(Vec3::new(x, side, -1.5), Vec3::Z, 0.25, 2.0 + (p % 3) as f64 * 0.01).unwrap());
+        }
+        owned.push(bodies);
+    }
+    let mut products: Vec<ExportProduct> = owned
+        .iter()
+        .enumerate()
+        .map(|(c, bodies)| ExportProduct {
+            name: format!("U{c}"),
+            bodies: bodies.iter().enumerate().map(|(k, b)| ExportBody { name: format!("U{c}_{k}"), body: b, color: Some([0.2, 0.2, 0.2]) }).collect(),
+            children: Vec::new(),
+        })
+        .collect();
+    let side = (count as f64).sqrt().ceil() as usize;
+    let children = (0..count)
+        .map(|c| {
+            let (x, y) = ((c % side) as f64 * 12.0, (c / side) as f64 * 10.0);
+            (c, [[1.0, 0.0, 0.0, 0.0], [0.0, 1.0, 0.0, 0.0], [0.0, 0.0, 1.0, 0.0], [x, y, 1.6, 1.0]], format!("U{c}"))
+        })
+        .collect();
+    products.push(ExportProduct { name: "Board".into(), bodies: Vec::new(), children });
+    let root = products.len() - 1;
+    let text = step_export_products(&products, root, &StepHeader { file_name: "board.step".into(), ..Default::default() }).unwrap();
+    let path = solidcraft_geom::env::var("STEP_PCB_FIXTURE").unwrap_or_else(|| "board.step".into());
+    std::fs::write(&path, &text).unwrap();
+    println!("wrote {path}: {} bytes, {} components", text.len(), count);
+}
+
+fn fillet_box(w: f64, d: f64, h: f64) -> Body {
+    let b = box_solid(Vec3::ZERO, Vec3::new(w, d, h)).unwrap();
+    let vertical = [Vec3::new(0.0, 0.0, h / 2.0), Vec3::new(w, 0.0, h / 2.0), Vec3::new(w, d, h / 2.0), Vec3::new(0.0, d, h / 2.0)];
+    let top = [Vec3::new(w / 2.0, 0.0, h), Vec3::new(w, d / 2.0, h), Vec3::new(w / 2.0, d, h), Vec3::new(0.0, d / 2.0, h)];
+    let b = fillet(&b, &vertical, 0.3).unwrap_or(b);
+    fillet(&b, &top, 0.1).unwrap_or(b)
+}
+
+/// Independent boxes as one STEP file (about 3 KB a box).
+fn boxes_step(count: usize, name: &str) -> String {
+    let bodies: Vec<_> = (0..count)
+        .map(|i| {
+            let at = Vec3::new((i % 32) as f64 * 12.0, (i / 32) as f64 * 12.0, 0.0);
+            box_solid(at, at + Vec3::new(10.0, 10.0, 10.0)).unwrap()
+        })
+        .collect();
+    step_export(&bodies.iter().collect::<Vec<_>>(), name).unwrap()
+}
+
+struct TestWatch {
+    cancel_after: usize,
+    reports: std::sync::atomic::AtomicUsize,
+    last: std::sync::Mutex<(&'static str, usize, usize)>,
+}
+
+impl crate::watch::Watch for TestWatch {
+    fn cancelled(&self) -> bool {
+        self.reports.load(std::sync::atomic::Ordering::Relaxed) >= self.cancel_after
+    }
+    fn progress(&self, stage: &'static str, done: usize, total: usize) {
+        self.reports.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        *self.last.lock().unwrap() = (stage, done, total);
+    }
+}
+
+#[test]
+fn watched_import_reports_progress_and_stops_on_cancel_without_caching() {
+    let text = boxes_step(40, "watched import");
+    let watch = Arc::new(TestWatch { cancel_after: usize::MAX, reports: Default::default(), last: std::sync::Mutex::new(("", 0, 0)) });
+    let imp = crate::watch::watching(watch.clone(), || step_import(&text)).unwrap();
+    assert_eq!(imp.bodies.len(), 40);
+    let (stage, done, total) = *watch.last.lock().unwrap();
+    assert_eq!((stage, total), ("converting", 40));
+    assert!(done <= total && watch.reports.load(std::sync::atomic::Ordering::Relaxed) > 1);
+
+    // Cancelled part way: an error, no partial result, and nothing cached for the text.
+    let text = boxes_step(41, "cancelled import");
+    let watch = Arc::new(TestWatch { cancel_after: 3, reports: Default::default(), last: std::sync::Mutex::new(("", 0, 0)) });
+    let err = crate::watch::watching(watch.clone(), || step_import_shared(&text).map(|_| ())).unwrap_err();
+    assert!(err.to_string().contains("cancelled"), "{err}");
+    let (_, done, _) = *watch.last.lock().unwrap();
+    assert!(done < 41, "stopped before converting everything ({done})");
+    // Unwatched, the same text imports in full (no cached failure or partial import).
+    assert_eq!(step_import_shared(&text).unwrap().bodies.len(), 41);
+    // Without a watch nothing is reported or stopped.
+    assert_eq!(step_import(&boxes_step(2, "unwatched")).unwrap().bodies.len(), 2);
+}
+
+#[test]
+fn large_imports_are_freed_with_their_last_user() {
+    let text = boxes_step(400, "large shared import");
+    assert!(text.len() > KEEP_SMALL_BYTES, "{} bytes", text.len());
+    let first = step_import_shared(&text).unwrap();
+    let again = step_import_shared(&text).unwrap();
+    assert!(Arc::ptr_eq(&first, &again), "read once while it is in use");
+    let weak = Arc::downgrade(&first);
+    drop((first, again));
+    assert!(weak.upgrade().is_none(), "the cache must not keep a large import alive after its design is gone");
+}

crates/kernel/src/watch.rs

@@ -0,0 +1,44 @@
+//! Progress and cooperative cancellation for long kernel work (reading a large STEP file) run
+//! off the window thread. The caller installs a [`Watch`] around the work with [`watching`];
+//! the work reports how far it is and stops at its next check once the watch says so. Work
+//! that nobody watches runs as before.
+
+use std::cell::RefCell;
+use std::sync::Arc;
+
+/// Someone waiting for long work: told how far it is, and able to stop it.
+pub trait Watch: Send + Sync {
+    /// Stop at the next check.
+    fn cancelled(&self) -> bool;
+    /// `done` of `total` units of the current step (`stage`, such as "reading" or "converting").
+    fn progress(&self, stage: &'static str, done: usize, total: usize);
+}
+
+thread_local! {
+    static CURRENT: RefCell<Option<Arc<dyn Watch>>> = const { RefCell::new(None) };
+}
+
+/// Run `f` with `watch` seeing the kernel work it does on this thread.
+pub fn watching<R>(watch: Arc<dyn Watch>, f: impl FnOnce() -> R) -> R {
+    let previous = CURRENT.with(|c| c.replace(Some(watch)));
+    struct Restore(Option<Option<Arc<dyn Watch>>>);
+    impl Drop for Restore {
+        fn drop(&mut self) {
+            if let Some(previous) = self.0.take() {
+                CURRENT.with(|c| *c.borrow_mut() = previous);
+            }
+        }
+    }
+    let _restore = Restore(Some(previous));
+    f()
+}
+
+/// The watch over this thread's work, to hand to the worker threads it starts.
+pub(crate) fn current() -> Option<Arc<dyn Watch>> {
+    CURRENT.with(|c| c.borrow().clone())
+}
+
+/// The error long work returns when its watch cancelled it.
+pub(crate) fn cancelled_error(what: &str) -> crate::KernelError {
+    crate::KernelError::Failed(format!("{what} cancelled"))
+}