Repository navigation
Conversation
…ication snapshot curve_on_classify clones the whole B-rep (clone_sharing_surface_caches) for every curve it classifies and drops it afterwards; on a 269-face boolean this was about half of the cut, and the edge and edgeuse arenas were 72% of each copy. The snapshot now shares those two arenas through an Arc and copies one only when something writes to it (Arc::make_mut in DerefMut), so every writer sees exactly what the deep copy gave it. Plain Brep::clone stays deep.
… intersections CurveClassification intersected every edge through the uncached global_curve_intersect, so each edge curve was tessellated again for every classified curve: about half of a 269-face boolean cut. Upstream keeps a curve's cache on the curve object (SmCacheMgr::GetOrCreateObjectCache in SmCurve::GlobalCurveIntersect). The target's CurveCache now lives in a slot on the Curve with the tolerances it was built under; every &mut edit of the geometry clears it and a clone starts without one.
|
Machine-generated (OpenAI Codex coordinator and agents) on behalf of @tdamsma. I checked the conflicts against current main. The classification changes merge cleanly; the collision is in Curve storage, construction/copy and edit invalidation. Main now has CurveMemo, with context-dependent keys, a memoization switch, and exclusions for surface/volume-dependent curves. This PR’s separate target cache checks only tolerances and accepts those dependent kinds too. A mechanical union would bypass those protections. A useful refresh would first measure main’s memo on the same workload, then retain any additional target-cache gain through the existing memo eligibility and complete key. Regression controls should cover context changes, disabled memoization, dependent-surface changes, and clone/edit independence. I have not changed your branch or claimed the performance gain is already redundant. |
|
Thanks, agreed on both points. Measured on the same battery workload: main with #1037 and the memo takes 5.6–6.3 s for eight cuts, this PR on its earlier base 6.7–6.9 s, so there is no extra gain to keep. And you're right that the tolerance-only key would have accepted surface-dependent curves the memo excludes. That's why it stays closed. |
Depends on #1037 (its commit is the first one here; review only the second). Without #1037 every classification snapshot deep-copies the edge curves, and a copy starts without a cache, so this change has nothing to reuse.
CurveClassification::intersect_edgesand the edgeuse path called the uncachedglobal_curve_intersect, so every edge curve was tessellated again (CurveCache::tessellate/compute_constants) for every curve thatcurve_on_classifyclassified. On a 269-face boolean from a battery model that was about half of the cut's samples. Upstream keeps a curve's cache on the curve object:SmCurve::GlobalCurveIntersecttakes both caches fromSmCacheMgr::GetOrCreateObjectCache(SM_OC_CURVE, ...).The target curve's
CurveCachenow lives in a slot onCurve, together with the kernel tolerances it was built under, and is reused only under equal tolerances, through the existingglobal_curve_intersect_cached. Every&mutedit of the curve's geometry (kind_mutand the editing methods) clears the slot, and every clone or constructor starts with an empty one, so a cache never outlives the geometry it was built from. The source curve's cache is still built per call, as before.Measured on that model: eight sequential
boolean_differencecuts of a 269-face body by 14-face tools (macOS arm64, release, base = #1037 on main 329e22c, 3 interleaved runs each):Against main without #1037 the eight cuts take 22.1–24.5 s.
cargo test -p solidcore --lib --release: 2199 passed, 0 failed. Locally on Linux arm64: fmt, deny and both clippy runs (including--features difftest/cpp) pass. C++ parity is left to this PR's CI. No tolerances or cases changed.