Modeling Algorithms - fix unbounded self-interference: O(1) tangent-zone point access + a checkpointed breaker - #1386
Open
gsdali wants to merge 1 commit into
Conversation
point access + a checkpointed breaker Fixes Open-Cascade-SAS#1385. 1. Intf_Interference::Insert calls Intf_TangentZone::GetPoint(Index) inside a nested comparison loop. GetPoint is O(n) per call (the backing NCollection_Sequence has no O(1) indexed access), so every comparison paid that cost again -- measured ~80% of runtime in NCollection_BaseSequence::Find on a degenerate self-intersecting surface. Intf_TangentZone::Points() caches a true random-access array per zone (invalidated on any mutation): same comparisons, same result, O(1) lookup. 2. The self-interference phase never polled its cooperative progress indicator below BOPAlgo_CheckerSI::CheckFaceSelfIntersection, so a caller's timeout could only fire between whole-face checks, not within one -- on the artifact in Open-Cascade-SAS#1385, 619s CPU against a 30s deadline, never returning. Intf_Interference::SetBreaker (RAII-scoped via Intf_InterferenceBreakerScope) lets Insert() poll every 256 calls and abort by throwing Standard_Failure. Wired up in BOPAlgo_CheckerSI's self-intersect functor, only when execution is guaranteed single-threaded (an exception from a worker thread of OSD_Parallel::For's parallel path would be unsafe). Verified on the artifact linked in Open-Cascade-SAS#1385: with both fixes, a 0.5s deadline returns in 0.547s and a 30s deadline returns in 30.1s (vs. 619s+/never on stock), with correct HasFaulty() results throughout. Zero regression on existing self-intersection GTest cases plus 76 related Intf_*/IntPatch_*/IntTools_*/BOPAlgo_*/BRepAlgoAPI_* tests. New GTests: Intf_TangentZone_Test.cxx (Points() correctness and cache invalidation), Intf_Interference_Test.cxx (breaker aborts Insert() promptly; a non-tripping or absent breaker leaves behavior unchanged).
gsdali
marked this pull request as ready for review
July 20, 2026 11:10
gsdali
added a commit
to SecondMouseAU/OCCTSwift
that referenced
this pull request
Jul 20, 2026
…aker patch Local carry of Open-Cascade-SAS/OCCT#1386 (CI green, ready for review), fixing the isSelfIntersecting hardTimeout: hang on pathological self-interference artifacts. Xcframework rebuild + release to follow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
dpasukhi
self-requested a review
July 22, 2026 14:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1385:
BOPAlgo_ArgumentAnalyzer's self-interference check (SelfInterMode()) can run effectively unbounded on a degenerate self-intersecting surface.Intf_Interference::InsertcalledIntf_TangentZone::GetPoint(Index)repeatedly inside a nested comparison loop.GetPointis O(n) per call (the backingNCollection_Sequencehas no O(1) indexed access), so every comparison paid that cost again -- measured ~80% of runtime inNCollection_BaseSequence::Findon the artifact in BOPAlgo_ArgumentAnalyzer self-interference: unbounded runtime (no checkpoint) + O(n) tangent-zone lookup #1385.Intf_TangentZone::Points()now caches a true random-access array per zone (invalidated on any mutation): same comparisons, same result, O(1) lookup.The self-interference phase never polled its cooperative progress indicator below
BOPAlgo_CheckerSI::CheckFaceSelfIntersection, so a caller's timeout could only fire between whole-face checks, not within one -- 619s CPU against a 30s deadline on the artifact in BOPAlgo_ArgumentAnalyzer self-interference: unbounded runtime (no checkpoint) + O(n) tangent-zone lookup #1385, never returning.Intf_Interference::SetBreaker(RAII-scoped viaIntf_InterferenceBreakerScope) letsInsert()poll every 256 calls and abort by throwingStandard_Failure. Wired up inBOPAlgo_CheckerSI's self-intersect functor, only when execution is guaranteed single-threaded -- an exception thrown from a worker thread ofOSD_Parallel::For's parallel path would be unsafe (std::terminate).Verification
On the artifact linked in #1385: with both fixes, a 0.5s deadline returns in 0.547s and a 30s deadline returns in 30.1s (vs. 619s+/never on stock), with correct
HasFaulty()results throughout (the 0.5s case correctly reports "not yet determined" rather than a false negative, since the breaker trips before any fault is recorded).Locally (minimal-module
RelWithDebInfobuild,FoundationClasses+ModelingData+ModelingAlgorithmsonly):Intf_TangentZone_Test.cxx,Intf_Interference_Test.cxx): 8/8 pass.Intf_*,IntPatch_*,IntTools_*,BOPAlgo_*,BRepAlgoAPI_*): 76/76 pass.OpenCascadeGTest: 7241/7249 pass. The 8 failures are allBRepMesh_DiscretAlgoFactoryTest(mesh discretization plugin self-registration), an artifact of this being a minimal-module local build -- unrelated to this change and not touched by it.Test plan
clang-format --style=fileapplied to all changed files.