MantisBT - Open CASCADE
View Issue Details
0026365Open CASCADE[OCCT] OCCT:Foundation Classespublic2015-06-23 14:282019-04-02 12:39
nbv 
kgv 
normalminor 
assignedopen 
 
[OCCT] 7.4.0* 
Not required
0026365: Optimization of work of OSD_Parallel class members for GeomLib_CheckCurveOnSurface
Existing OCCT parallelization algorithm (without TBB) requires using many system resources (it creates a thread for every parallel process) and much time (time spent on thread creation sometimes is much more than time for execution this thread).

It is not optimal and we need to recode this algorithm.
Not required
1. See OSD_Parallel class. Especially, OSD_Parallel::For(...) member.

2. After the fix, please change the following fragment of BRepLib_CheckCurveOnSurface::Compute(...) method:

#ifdef HAVE_TBB
  OSD_Parallel::For(anIntervals.Lower(), anIntervals.Upper(), aComp);
#else
  ...
  OSD_Parallel::For(anIntervals.Lower(), anIntervals.Upper(), aComp, Standard_True);
#endif

should be replaced with
OSD_Parallel::For(anIntervals.Lower(), anIntervals.Upper(), aComp);
No tags attached.
related to 0026506closed bugmaster Open CASCADE Change class BRepLib_CheckCurveOnSurface 
related to 0028931closed bugmaster Open CASCADE Eliminate dependency from TBB in OSD_Parallel header 
related to 0028199assigned ski Open CASCADE Add possibility to set number of threads for parallel execution 
related to 0022146assigned abv Open CASCADE Integration of OCC in-house parallelization tool 
related to 0029935verified bugmaster Open CASCADE Foundation Classes - introduce OSD_ThreadPool class defining a thread pool 
Issue History
2015-06-23 14:28nbvNew Issue
2015-06-23 14:28nbvAssigned To => msv
2015-06-23 14:32nbvRelationship addedrelated to 0025613
2015-06-23 15:41msvNote Added: 0042363
2015-06-23 15:41msvAssigned Tomsv => abv
2015-07-31 16:02nbvRelationship addedrelated to 0026506
2016-11-03 17:07abvTarget Version7.1.0 => 7.2.0
2017-04-02 11:03kgvRelationship addedrelated to 0028531
2017-04-15 18:10kgvRelationship addedrelated to 0028657
2017-04-15 18:48kgvRelationship deletedrelated to 0028657
2017-07-20 12:34kgvRelationship addedrelated to 0028199
2017-07-20 12:37kgvRelationship addedrelated to 0022146
2017-07-26 16:41abvTarget Version7.2.0 => 7.4.0*
2017-08-11 13:05apvTest case number => Not required
2017-08-11 13:07oanRelationship addedrelated to 0028931
2018-07-07 02:32kgvRelationship addedrelated to 0029935
2019-01-10 16:38kgvSummaryOptimization of work of OSD_Parallel class members => Optimization of work of OSD_Parallel class members for GeomLib_CheckCurveOnSurface
2019-01-10 16:39gitNote Added: 0081722
2019-01-10 20:44kgvNote Added: 0081733
2019-01-10 20:44kgvAssigned Toabv => msv
2019-01-10 20:44kgvStatusnew => feedback
2019-01-11 09:31msvAssigned Tomsv => nbv
2019-01-11 09:32msvNote Added: 0081743
2019-01-11 10:04msvAssigned Tonbv => msv
2019-01-11 10:11nbvNote Added: 0081744
2019-01-11 10:48msvNote Added: 0081749
2019-01-11 10:48msvNote Edited: 0081749bug_revision_view_page.php?bugnote_id=81749#r20543
2019-01-11 10:53nbvNote Edited: 0081744bug_revision_view_page.php?bugnote_id=81744#r20545
2019-01-11 11:10msvNote Added: 0081750
2019-01-11 11:14msvNote Added: 0081752
2019-01-11 11:14msvAssigned Tomsv => kgv
2019-01-11 11:14msvStatusfeedback => assigned
2019-04-02 12:39gitNote Added: 0083357

Notes
(0042363)
msv   
2015-06-23 15:41   
Dear Andrey, please include this task in planning.
(0081722)
git   
2019-01-10 16:39   
Branch CR26365 has been created by kgv.

SHA-1: 1984a15e669e7f8567d13956ed65a71966b72809


Detailed log of new commits:

Author: kgv
Date: Thu Jan 10 16:35:25 2019 +0300

    0026365: Optimization of work of OSD_Parallel class members for GeomLib_CheckCurveOnSurface
    
    Removed workaround within GeomLib_CheckCurveOnSurface.
(0081733)
kgv   
2019-01-10 20:44   
The issue with threads re-creation has been handled within patch for 0029935.

This bug describes the problem, but does not provide any reproducer nor numbers, so that it is difficult to check how parallelization affects performance in this particular case.

Patch CR26365 removes HAVE_TBB check; through it is desired verifying performance - hints / test cases are welcome.
(0081743)
msv   
2019-01-11 09:32   
Dear Nikolay, could you recommend the test cases to check performance of the considered code?
(0081744)
nbv   
2019-01-11 10:11   
(edited on: 2019-01-11 10:53)
Unfortunately I do not remember all problematic test cases. However, you can find the tests where following DRAW commands are used: "xdistef", "checkcurveonsurf".

Additionally, this algorithm has already been implemented in "bopargcheck" algorithm and in FACE/FACE intersection algorithm called from Boolean operation.

Please try to switch on Parallel mode option in corresponding test cases and check their performance.

(0081749)
msv   
2019-01-11 10:48   
Actually, this algorithm works each time when intersection of 2 faces is performed inside PaveFiller. I.e., during each Boolean operation. So, any BO test can be impacted by this change if HAVE_TBB is not defined.
There is another simple way to call this code from Draw, it is to run the command:

checkcurveonsurf shape

It will call this code for each pcurve of the shape.

I have another concern about this patch. BO has the option IsParallel. If the user does not set it to true he expects all the code will run in one thread. But the code of GeomLib_CheckCurveOnSurface is always called in multi-thread mode (the argument of Perform() method isTheMultyTheradDisabled = Standard_False).

So, in order to make consistent behavior of the code regarding the flag myRunParallel of BO, it is needed to pass this flag through the chain of calls BOPAlgo_PaveFiller::PerformFF() -> IntTools_FaceFace -> IntTools_Tools::ComputeTolerance() -> GeomLib_CheckCurveOnSurface::Perform().

I propose to include this change in the current patch.

(0081750)
msv   
2019-01-11 11:10   
My proposed above change can be done later in scope of another bug, e.g. linked 0028199. And the current patch is worth to be integrated as is.
(0081752)
msv   
2019-01-11 11:14   
For simplicity, please just run the standard tests to check the patch if there is no regressions in no HAVE_TBB mode.
(0083357)
git   
2019-04-02 12:39   
Branch CR26365_1 has been created by kgv.

SHA-1: 35785ecaa27eee8a7e7a9b1cc466bf6fa4132655


Detailed log of new commits:

Author: kgv
Date: Thu Jan 10 16:35:25 2019 +0300

    0026365: Optimization of work of OSD_Parallel class members for GeomLib_CheckCurveOnSurface
    
    Removed workaround within GeomLib_CheckCurveOnSurface.