View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0027814CommunityOCCT:Modeling Algorithmspublic2016-08-26 16:502022-01-11 12:26
ReporterMarkus Assigned Tobugmaster  
PrioritynormalSeverityfeature 
Status closedResolutionfixed 
Product Version7.0.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0027814: Modeling Algorithms - Parallelize BRepCheck_Analyzer
DescriptionWould it be easily possible to parallelize BRepCheck_Analyzer?

For more complex solids, checking the shape's validity can become time-consuming, especially if also the geometry should be checked.

How big is the potential to use multi-threading in BRepCheck_Analyzer and could it be easily implemented?

Steps To Reproducerestore cc.brep c
checkshape c
TagsNo tags attached.
Test case numbertests/heal/checkshape/bug27814_1,bug27814_10,bug27814_11,bug27814_2,bug27814_3,bug27814_4,bug27814_5,bug27814_6,bug27814_7,bug27814_8,bug27814_9

Attached Files

  • cc.brep (6,006,463 bytes)
  • bug27814 performance.xlsx (19,541 bytes)

Relationships

parent of 0032492 closedasuraven Open CASCADE Coding - New warnings after integration fix for 27814 
has duplicate 0024524 closedbugmaster Open CASCADE Parallelization of 'checkshape' algorithm. 

Activities

Timo

2016-08-26 16:50

developer  

cc.brep (6,006,463 bytes)

asuraven

2021-05-26 18:50

developer   ~0101409

asuraven

2021-06-18 10:00

developer   ~0101910

asuraven

2021-06-21 17:50

developer   ~0101971

git

2021-06-21 19:14

administrator   ~0101976

Branch CR27814 has been created by asuraven.

SHA-1: c1a2703a28262570b1931941a373af2a85abc329


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer

git

2021-06-21 19:17

administrator   ~0101978

Branch CR27814 has been deleted by asuraven.

SHA-1: c1a2703a28262570b1931941a373af2a85abc329

git

2021-06-21 19:22

administrator   ~0101979

Branch CR27814 has been created by asuraven.

SHA-1: 5044dce08137ea5dbb9635193152d3c99f7a931b


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer

asuraven

2021-06-23 14:18

developer   ~0102010

asuraven

2021-06-23 14:18

developer   ~0102011

git

2021-06-23 15:23

administrator   ~0102012

Branch CR27814 has been updated by asuraven.

SHA-1: 9cd355b8be606be379870b28736b2aa6bd36b5e7


Detailed log of new commits:

Author: asuraven
Date: Wed Jun 23 15:23:45 2021 +0300

    Parallel for the whole map

Author: asuraven
Date: Wed Jun 23 14:32:25 2021 +0300

    chrone for Perform

git

2021-06-24 17:53

administrator   ~0102036

Branch CR27814 has been updated by asuraven.

SHA-1: 9c61390ca204b1833c8c7982812d51f5d4df0921


Detailed log of new commits:

Author: asuraven
Date: Thu Jun 24 15:26:47 2021 +0300

    IndexedDataMap

git

2021-06-24 19:50

administrator   ~0102042

Branch CR27814 has been updated by asuraven.

SHA-1: 55d9a229ef47692508bb1583a93b5a5e5f0f9852


Detailed log of new commits:

Author: asuraven
Date: Thu Jun 24 19:51:04 2021 +0300

    force linear

asuraven

2021-06-25 12:48

developer   ~0102049

git

2021-06-28 19:12

administrator   ~0102136

Branch CR27814 has been updated by asuraven.

SHA-1: 66ba96acdc32ef840bd32b5aba8cb670135b586a


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 28 19:12:59 2021 +0300

    arrays for tasks + mutexes + refactor

git

2021-06-28 19:16

administrator   ~0102137

Branch CR27814 has been deleted by asuraven.

SHA-1: 66ba96acdc32ef840bd32b5aba8cb670135b586a

git

2021-06-28 19:16

administrator   ~0102138

Branch CR27814 has been created by asuraven.

SHA-1: f2a8facb543eb0ff7828518098fbd95e4572095d


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 28 19:12:59 2021 +0300

    arrays for tasks + mutexes + refactor

Author: asuraven
Date: Thu Jun 24 15:26:47 2021 +0300

    IndexedDataMap

Author: asuraven
Date: Thu Jun 24 19:51:04 2021 +0300

    force linear

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer

Author: asuraven
Date: Wed Jun 23 14:32:25 2021 +0300

    chrone for Perform

Author: asuraven
Date: Wed Jun 23 15:23:45 2021 +0300

    Parallel for the whole map

asuraven

2021-06-28 19:16

developer   ~0102139

asuraven

2021-06-29 15:45

developer   ~0102164

asuraven

2021-06-30 16:27

developer   ~0102187

git

2021-06-30 17:50

administrator   ~0102189

Branch CR27814 has been deleted by asuraven.

SHA-1: f2a8facb543eb0ff7828518098fbd95e4572095d

git

2021-06-30 18:33

administrator   ~0102191

Branch CR27814 has been created by asuraven.

SHA-1: 84da46b47ab0ea43f71ac97107e35e5bdb49b912


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-06-30 21:38

developer   ~0102197

Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
Add parallelization to BRepCheck_Analyzer::Perform
Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

Performance increase in parallel mode is about 2.32x
Performance in single-threaded mode relative to master is about 0.97
See attached bug27814 performance.xlsx

Tests passed: http://jenkins-test-occt.nnov.opencascade.com/view/CR27814-master-ASURAVEN/view/COMPARE/

kgv

2021-06-30 22:21

developer   ~0102198

Last edited: 2021-07-01 13:40

 Performance became up to 6% slower in single-threaded execution, which doesn't look good to me.

Performance tables are expected to specify units, not just numbers.

In addition, it is also important providing information about tested CPU and number of threads within multi-threaded implementation used in tests.

OCCT has option to use TBB and built-in thread pool - please put information about OCCT building options (HAVE_TBB) and consider performing comparison.

kgv

2021-06-30 22:46

developer   ~0102199

Last edited: 2021-06-30 22:49

 
   if (myMap.IsBound(S)) {
     return;
   }

-  BRepCheck_ListOfStatus thelist;
-  myMap.Bind(S, thelist);
-
-  BRepCheck_ListOfStatus& lst = myMap(S);
+  BRepCheck_ListOfStatus aList;
...
+  Standard_Mutex::Sentry aLock(myMutex);
+  myMap.Bind(S, aList);

This logic is confusing - myMap.IsBound(S) is done without mutex lock, while adding to the map is protected by mutex.
Note that IsBound() is also not thread-safe - it may lead to data races even if concurrent thread will try adding another shape to the map.

If existing logic ensures that function will be called with the same S only from one thread, then changes below could be considered redundant and list could be bound to map at the beginning without extra copy just with a mutex lock (but BRepCheck_ListOfStatus have to be replaced by a handle to keep pointer after mutex release).

+  Standard_Mutex myMutex;

Consider avoiding myMutex allocation in single-threaded scenario (Standard_Mutex::Sentry supports interface for passing NULL / Standard_Mutex pointer).

+  for (BRepCheck_DataMapIteratorOfDataMapOfShapeResult aMapIterator(myMap); aMapIterator.More(); aMapIterator.Next())
+  {
+    Standard_Integer aMapIndex = myMap.FindIndex(aMapIterator.Key())-1;

Why using Iterator and lookup index from key instead of straightforward iteration by index within indexed data map?

+  NCollection_Vector< NCollection_Vector<TopoDS_Shape> > aVectOfVect;

NCollection_Vector doesn't look like a best choice for this logic - it seems that arrays size can be calculated in advance making NCollection_Array1 or std::vector a better choice.

+  Handle(NCollection_Shared< NCollection_Vector< NCollection_Vector<TopoDS_Shape> > >) aSharedVect =
+    new NCollection_Shared< NCollection_Vector< NCollection_Vector<TopoDS_Shape> > >(aVectOfVect);
+  Handle(NCollection_Shared< BRepCheck_DataMapOfShapeResult >) aSharedMap = 
+    new NCollection_Shared< BRepCheck_DataMapOfShapeResult >(myMap);

Is it really necessary making these extra copies?

msv

2021-07-01 12:25

developer   ~0102204

src/BRepCheck/BRepCheck_Analyzer.hxx
Make indentation 2 in lines 48-67.

src/BRepCheck/BRepCheck_DataMapOfShapeResult.hxx
Please rename BRepCheck_DataMapOfShapeResult to BRepCheck_IndexedDataMapOfShapeResult (both the type and the file) to avoid misinterpretation.

src/BRepCheck/BRepCheck_Analyzer.cxx
  class ParallelAnalizer

Misprint -> ParallelAnalyzer

asuraven

2021-07-01 12:26

developer   ~0102205

Using TBB has same performance as built-in thread pool. See results in updated attached table

asuraven

2021-07-01 13:13

developer  

bug27814 performance.xlsx (19,541 bytes)

msv

2021-07-01 15:55

developer   ~0102210

Split the test bug27814 on several scripts, one per shape. Put the common procedure in "begin".

Add in the test the check that the results of single-thread and multi-thread are the same.

Correct the help:
checkshape [-top] shape [result] [-short | -parallel]
to
checkshape [-top] shape [result] [-short] [-parallel]
Also correct the logic of treatment of cmd arguments. The variable aPref must be set along with setting of the flag IsContextDump, as now the argument 'result' is allowed being located anywhere after 'shape'.

msv

2021-07-01 15:58

developer   ~0102211

Last edited: 2021-07-01 15:59

 In order to balance loading, it is needed to provide each thread with more than one task. I propose the following logic to compute task size:
  const Standard_Integer aMapSize = myMap.Size();
  const Standard_Integer aMinTaskSize = 10;
  const Handle(OSD_ThreadPool)& aThreadPool = OSD_ThreadPool::DefaultPool();
  const Standard_Integer aNbThreads = aThreadPool->NbThreads();
  Standard_Integer aNbTasks = aNbThreads * 10;
  Standard_Integer aTaskSize = Max ((Standard_Integer)Ceiling(aMapSize/aNbTasks), aMinTaskSize);


asuraven

2021-07-02 09:41

developer   ~0102224

git

2021-07-05 11:24

administrator   ~0102279

Branch CR27814 has been updated by asuraven.

SHA-1: 6481f3e43931047a0ddc98243af5a4f316cd1293


Detailed log of new commits:

Author: asuraven
Date: Mon Jul 5 11:24:38 2021 +0300

    fix

asuraven

2021-07-06 13:18

developer   ~0102301

git

2021-07-06 18:39

administrator   ~0102311

Branch CR27814_1 has been created by asuraven.

SHA-1: efe79cecd037f15ca0fc5bcb1bd10e33dfbeb6ef


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

git

2021-07-06 20:25

administrator   ~0102313

Branch CR27814_1 has been deleted by asuraven.

SHA-1: efe79cecd037f15ca0fc5bcb1bd10e33dfbeb6ef

git

2021-07-06 20:26

administrator   ~0102314

Branch CR27814_1 has been created by asuraven.

SHA-1: 80c07be8d702cd78d5ca0df79a2a001705551dc3


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-06 20:49

developer   ~0102315

git

2021-07-07 14:19

administrator   ~0102324

Branch CR27814_1 has been updated by asuraven.

SHA-1: 2f7dad9028edf0021b7fc2ea98b500f32915f25f


Detailed log of new commits:

Author: asuraven
Date: Wed Jul 7 14:19:06 2021 +0300

    more mutexes

git

2021-07-07 15:42

administrator   ~0102328

Branch CR27814 has been deleted by asuraven.

SHA-1: 6481f3e43931047a0ddc98243af5a4f316cd1293

git

2021-07-07 16:19

administrator   ~0102332

Branch CR27814 has been created by asuraven.

SHA-1: c9ddc4ee06bfb8ecfb03f237c4657262e369aba4


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

git

2021-07-07 18:59

administrator   ~0102345

Branch CR27814_1 has been deleted by asuraven.

SHA-1: 2f7dad9028edf0021b7fc2ea98b500f32915f25f

git

2021-07-07 19:00

administrator   ~0102346

Branch CR27814_1 has been created by asuraven.

SHA-1: 6dd600f91426aa59c9204715bfc43beb4576d4dc


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-08 17:11

developer   ~0102378

msv

2021-07-09 13:24

developer   ~0102393

Make a definition of an alias in the file BRepCheck_ListOfStatus.hxx and use it:
typedef Handle(NCollection_Shared<BRepCheck_ListOfStatus>) BRepCheck_HListOfStatus;


src/BRepCheck/BRepCheck.hxx
Do not change the API of the method Add(). Instead, dereference handle of the list at each call of Add().

In the methods Minimum():
    Handle(NCollection_Shared<BRepCheck_ListOfStatus>) lst = myMap(myShape);

Instead, use this:
    BRepCheck_ListOfStatus& aList = *myMap(myShape);


In the methods InContext():
After the handle 'lst' is ready define a reference to the list:
BRepCheck_ListOfStatus& aList = *lst;

And pass it later to BRepCheck::Add().

src/BRepCheck/BRepCheck_Analyzer.cxx 
  Standard_Integer aTaskSize = (Standard_Integer)Ceiling((double)aMapSize / aNbTasks);
  if (aTaskSize < aMinTaskSize)
  {
    aTaskSize = aMinTaskSize;
  }
  NCollection_Array1< NCollection_Array1<TopoDS_Shape> >  aArrayOfArray(0, aNbTasks);

If aTaskSize has been adjusted you need to adjust also aNbTasks.
Why aArrayOfArray is initializes with (0, aNbTasks)? Then now the number of items is aNbTasks+1. And the last item will be an uninitialized array!

                    { // critical section
                      Standard_Mutex::Sentry aLock(myIsParallel ? &myMutex : NULL);

This mutex is common for all execution. It will stop treatment of all other shapes in critical sections. It is better to use the mutex of the particular result.

src/BRepCheck/BRepCheck_Analyzer.hxx 
  BRepCheck_Analyzer(const TopoDS_Shape& S,
                      const Standard_Boolean GeomControls = Standard_True,
                      const Standard_Boolean theIsParallel = Standard_False);

Correct indentation of the tailing lines.

typedef NCollection_IndexedDataMap<TopoDS_Shape,Handle(BRepCheck_Result),TopTools_OrientedShapeMapHasher>::Iterator BRepCheck_IndexedDataMapIteratorOfDataMapOfShapeResult;

This type is not used and can be removed. Also remove the redundant files BRepCheck_DataMapIteratorOfDataMapOfShapeResult.hxx and BRepCheck_DataMapIteratorOfDataMapOfShapeListOfStatus.hxx.

src/BRepCheck/BRepCheck_Result.hxx
The two methods:
  Standard_EXPORT const Handle(NCollection_Shared<BRepCheck_ListOfStatus>)& StatusOnShape (const TopoDS_Shape& S);
    const Handle(NCollection_Shared<BRepCheck_ListOfStatus>)& StatusOnShape() const;

are ambiguous, but have very different sense. I propose to remove non-const version, as it seems it is not used by the algorithm logic, but it can be used by compiler on some platform by mistake.

About the usage of BRepCheck_Result::InitContextIterator(). I consider it is useless, because anywhere it is used only for finding a shape that is a key in the map. It is better to use the general functions of the map to find the needed shape by key instead of iterating all the map to find that key. Please consider making a small refactoring concerning this thing. After that the type BRepCheck_DataMapIteratorOfDataMapOfShapeListOfStatus will be also useless and can be removed.

msv

2021-07-09 14:18

developer   ~0102395

There are still many places where BRepCheck::Add() is called without locking a mutex. This can cause data races.

src/BRepCheck/BRepCheck_Result.hxx 
    const Handle(NCollection_Shared <BRepCheck_ListOfStatus>)& Status() const;

This change of API is unjustified. And it can lead to incompatibility of many user applications (and it is proved by your change of other files in this patch). Please restore this function return value type.

I see that mechanism of InitContextIterator() can be used in a user code. So, I propose to make this method obsolete, and provide another more efficient method to get access to contextual results.

msv

2021-07-09 14:35

developer   ~0102396

src/BRepTest/BRepTest_CheckCommands.cxx
Why the lines 969, 1004, 1009-1021, 1028 and others are changed in contradiction with coding rules?

          Standard_CString aPref = a[aCurInd + 1];
          StructuralDump(theCommands, anAna, aShapeName, aPref, aShape);

The local aPref hides the same from the upper level.

tests/heal/grids.list 
014 same_parameter_locked
015 update_tolerance_locked
024 checkshape

Please numerate grids uniquely. The numbers 14 and 15 are busy above. It was a mistake by someone earlier.

msv

2021-07-09 14:39

developer   ~0102397

Review.

asuraven

2021-07-09 17:50

developer   ~0102407

git

2021-07-12 17:03

administrator   ~0102473

Branch CR27814_2 has been created by asuraven.

SHA-1: 94cac8ceb16b72f476a46c6288f0047ebeb4b911


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-12 18:14

developer   ~0102477

Last edited: 2021-07-12 18:41

git

2021-07-13 13:48

administrator   ~0102490

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 94cac8ceb16b72f476a46c6288f0047ebeb4b911

git

2021-07-13 13:48

administrator   ~0102491

Branch CR27814_2 has been created by asuraven.

SHA-1: 5d5a6cadf3b088d26e275ffd60bf0a2bf1fa946b


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-13 15:55

developer   ~0102496

Last edited: 2021-07-13 17:25

git

2021-07-13 20:17

administrator   ~0102506

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 5d5a6cadf3b088d26e275ffd60bf0a2bf1fa946b

git

2021-07-13 20:18

administrator   ~0102507

Branch CR27814_2 has been created by asuraven.

SHA-1: 67f8beb8541ece6f520ec63a2490901a98dfe45c


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-14 14:52

developer   ~0102521

@msv see changes in branch CR27814_2
http://jenkins-test-occt/view/CR27814_2-master-ASURAVEN/view/COMPARE/

msv

2021-07-14 16:41

developer   ~0102522

src/BRepCheck/BRepCheck_Result.hxx 
    const BRepCheck_HListOfStatus& StatusOnShape() const;

Return const BRepCheck_ListOfStatus& to preserve compatibility.

Rename IsContextualShape to IsStatusOnShape.
Rename ContextualShapeStatuses to StatusOnShape.
It is just for more logical naming.

src/BRepCheck/BRepCheck_Result.cxx
src/BRepCheck/BRepCheck_Result.lxx
Remove word 'obsolete' in comments.

Sorry, in my previous remark I was wrong. Please replace the type
typedef Handle(NCollection_Shared<BRepCheck_ListOfStatus>) BRepCheck_HListOfStatus;

with
typedef NCollection_Shared<BRepCheck_ListOfStatus> BRepCheck_HListOfStatus;

And then replace the lines like
    BRepCheck_HListOfStatus aNewList = new NCollection_Shared<BRepCheck_ListOfStatus>();

with
    Handle(BRepCheck_HListOfStatus) aNewList = new BRepCheck_HListOfStatus();

Otherwise, it is unclear in the code that the type BRepCheck_HListOfStatus indeed is a handle.

To avoid double search in the map, instead of 
    myMap.Bind(S, aNewList);
    aHList = myMap(S);

use this:
    aHList = *myMap.Bound(S, aNewList);


Remove the redundant files BRepCheck_DataMapIteratorOfDataMapOfShapeResult.hxx and BRepCheck_DataMapIteratorOfDataMapOfShapeListOfStatus.hxx.

Add protection by mutex in the method BRepCheck_Shell::SetUnorientable.

In BRepCheck_Solid::Minimum(), replace "BRepCheck::Add(*myMap(myShape)" with "BRepCheck::Add(aLST".

git

2021-07-14 16:51

administrator   ~0102523

Branch CR27814_3 has been created by asuraven.

SHA-1: 69e5a92648c9fdb823069c8905d688b4514f936a


Detailed log of new commits:

Author: asuraven
Date: Wed Jul 14 16:51:20 2021 +0300

    0027814: [tests only] Modeling Algorithms - Parallelize BRepCheck_Analyzer

git

2021-07-14 19:38

administrator   ~0102529

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 67f8beb8541ece6f520ec63a2490901a98dfe45c

git

2021-07-14 19:47

administrator   ~0102530

Branch CR27814_2 has been created by asuraven.

SHA-1: a34acf3287bcf1b8a7c342b2444eedae5c35fde7


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

git

2021-07-15 11:52

administrator   ~0102541

Branch CR27814_2 has been deleted by asuraven.

SHA-1: a34acf3287bcf1b8a7c342b2444eedae5c35fde7

git

2021-07-15 11:52

administrator   ~0102542

Branch CR27814_2 has been created by asuraven.

SHA-1: 5423230987b9c27b73f428ce7d40af1779886c72


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-15 15:04

developer   ~0102545

asuraven

2021-07-15 16:46

developer   ~0102550

asuraven

2021-07-15 18:36

developer   ~0102556

Last edited: 2021-07-16 11:32

 tests are ok:
http://jenkins-test-occt.nnov.opencascade.com/view/CR27814_2-master-ASURAVEN/view/COMPARE/
See branch CR27814_2.
Tests on branch CR27814_3 (http://jenkins-test-occt.nnov.opencascade.com/view/CR27814_3-master-ASURAVEN/view/COMPARE/) demonstrate that a test bug27814_6 problem with unstability numbers of
 faulty in checkshape isn't a parallelization problem

msv

2021-07-15 20:53

developer   ~0102558

Please correct compilation warnings.

git

2021-07-16 12:19

administrator   ~0102563

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 5423230987b9c27b73f428ce7d40af1779886c72

git

2021-07-16 12:19

administrator   ~0102564

Branch CR27814_2 has been created by asuraven.

SHA-1: 97f637ddefc30ead86441ca7c145791d4fd2d341


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

asuraven

2021-07-16 12:20

developer   ~0102565

fixed in CR27814_2

msv

2021-07-16 15:58

developer   ~0102580

OCCT - CR27814_2
Products - none.

kgv

2021-07-16 17:50

developer   ~0102583 

+ const Handle(BRepCheck_Face)& aFaceRes = Handle(BRepCheck_Face)::DownCast(aResult);

DownCast() returns a copy - please avoid defining a variable as & reference.

+    Standard_Mutex::Sentry aLock(myIsParallel ? &myMutex : NULL);

What I've suggested is to define myMutex as Handle(NCollection_Shared<Standard_Mutex>) and allocate mutex in sync with IsParallel flag, but OK.

+    return myMap.IsBound(theShape) ? Standard_True : Standard_False;

Could be just "return myMap.IsBound(theShape);".

+    else if (!strcmp(a[anAI], "-parallel"))

Case-insensitive compares in preferred for Draw Harness command arguments.
TCollection_AsciiString anArg(a[anAI]);
anArg.LowerCase();
if (anArg == "-parallel") {}

msv

2021-07-16 18:02

developer   ~0102584

Agree with Kirill's remarks.

git

2021-07-16 18:10

administrator   ~0102585

Branch CR27814_2 has been updated by asuraven.

SHA-1: 81030f91a7a7c8aa72ae214b9477cddc3245ad48


Detailed log of new commits:

Author: asuraven
Date: Fri Jul 16 18:09:23 2021 +0300

    fix

git

2021-07-16 18:23

administrator   ~0102586

Branch CR27814_2 has been updated forcibly by asuraven.

SHA-1: 288077188e24fce51b942cc69340160edfad293e

git

2021-07-19 12:11

administrator   ~0102629

Branch CR27814_2 has been updated forcibly by asuraven.

SHA-1: ae3e6594bceb9b59a46a3f2c60f02aa35c9fc94c

git

2021-07-19 12:14

administrator   ~0102630

Branch CR27814_2_kgv has been created by kgv.

SHA-1: 6b3acca765c05f45d65e01918013f826cd6ecf97


Detailed log of new commits:

Author: kgv
Date: Mon Jul 19 12:12:31 2021 +0300

    # cosmetics

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

git

2021-07-19 12:15

administrator   ~0102631

Branch CR27814_4 has been created by kgv.

SHA-1: b48be4bd8e9e46bf7369d0891a0766e73a80c803


Detailed log of new commits:

Author: asuraven
Date: Mon Jun 21 19:15:09 2021 +0300

    0027814: Parallelize BRepCheck_Analyzer
    
    Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
    Add parallelization to BRepCheck_Analyzer::Perform
    Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

kgv

2021-07-19 14:31

developer   ~0102645

Cosmetic changes have been pushed to CR27814_4.

kgv

2021-07-19 14:37

developer   ~0102646

After taking a deeper look into code, it seems that every single sub-shape associated with BRepCheck_Result creates its own Mutex object - e.g. it might be thousands and more (recursivelly).
So I withdraw my early thought - it doesn't look OK to me creating that much of mutexes, especially in case of a single-threaded execution.

  if (!HR.IsNull())
  {
    HR->SetParallel (theIsParallel);
  }
  myMap.Add (theShape, HR);

  for (TopoDS_Iterator theIterator (theShape); theIterator.More(); theIterator.Next())
  {
    Put (theIterator.Value(), B, theIsParallel); // performs minimum on each shape
  }

git

2021-07-19 17:11

administrator   ~0102656

Branch CR27814_4 has been updated by asuraven.

SHA-1: 4c0aac21dfc46efc2213e8ec1b1427e38d96aea1


Detailed log of new commits:

Author: asuraven
Date: Mon Jul 19 17:11:08 2021 +0300

    mutex as Handle

git

2021-07-19 18:05

administrator   ~0102657

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: c2a3684122f8a12eb9dfe6204818f264bcd534e4

asuraven

2021-07-19 19:20

developer   ~0102658

kgv

2021-07-19 20:01

developer   ~0102661

git

2021-07-20 10:45

administrator   ~0102672

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 2b0e1e89254428041607ba24b1daa15a8dbc2ef6

git

2021-07-20 13:03

administrator   ~0102676

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 6eb57d1922b1cf575dea0f924853d40c1f8475d6

asuraven

2021-07-20 14:13

developer   ~0102677

Mutexes are wrapped in handles.
Flag isParallel excluded.
After consulting with msv, it was decided not to exclude the mutex from BRepCheck_Result.
Tests results: http://jenkins-test-occt.nnov.opencascade.com/view/CR27814_4-master-KGV/view/COMPARE/

kgv

2021-07-20 14:25

developer   ~0102678 

--- a/src/BRepCheck/BRepCheck_Analyzer.cxx
+++ b/src/BRepCheck/BRepCheck_Analyzer.cxx
@@ -44,11 +44,9 @@ class BRepCheck_ParallelAnalyzer
...
   mutable Standard_Mutex myMutex;
-  Standard_Boolean myIsParallel;

myMutex is never used and can be removed.

+void BRepCheck_Result::SetParallel(Standard_Boolean theIsParallel)
+{
+  if (theIsParallel && myMutex.IsNull())
+  {
+    myMutex.reset(new Standard_HMutex());
+  }
...
+  if (!HR.IsNull())
+  {
+    HR->SetParallel (theIsParallel);
+  }

SetParallel() method looks confusing.
Wouldn't it be simpler passing theIsParallel to constructor of statuses?

git

2021-07-20 15:02

administrator   ~0102679

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 228024e3384dff4ba43f19dc8564b5fabc834c61

asuraven

2021-07-20 15:04

developer   ~0102680

myMutex was deleted

bugmaster

2021-07-24 12:09

administrator   ~0102761

Combination -
OCCT branch : IR-2021-07-23
master SHA - 4e993e4d0df118716a2ccca02a5735fc4dec54ef
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-07-23 SHA - 5beb1e287b2273f9e4c222f042380bc3c0fa91dc
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17248.180000000266 / 17229.680000000302 [+0.11%]
Products
Total CPU difference: 11453.40000000009 / 11477.160000000118 [-0.21%]
Windows-64-VC14:
OCCT
Total CPU difference: 19038.78125 / 19020.171875 [+0.10%]
Products
Total CPU difference: 12762.65625 / 12734.125 [+0.22%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-07-24 13:27

administrator   ~0102772

Branch CR27814 has been deleted by mnt.

SHA-1: c9ddc4ee06bfb8ecfb03f237c4657262e369aba4

git

2021-07-24 13:27

administrator   ~0102773

Branch CR27814_1 has been deleted by mnt.

SHA-1: 6dd600f91426aa59c9204715bfc43beb4576d4dc

git

2021-07-24 13:27

administrator   ~0102774

Branch CR27814_2 has been deleted by mnt.

SHA-1: ae3e6594bceb9b59a46a3f2c60f02aa35c9fc94c

git

2021-07-24 13:28

administrator   ~0102775

Branch CR27814_2_kgv has been deleted by mnt.

SHA-1: 6b3acca765c05f45d65e01918013f826cd6ecf97

git

2021-07-24 13:28

administrator   ~0102776

Branch CR27814_3 has been deleted by mnt.

SHA-1: 69e5a92648c9fdb823069c8905d688b4514f936a

git

2021-07-24 13:28

administrator   ~0102777

Branch CR27814_4 has been deleted by mnt.

SHA-1: 228024e3384dff4ba43f19dc8564b5fabc834c61

Related Changesets

occt: master 000c21fa

2021-06-21 16:15:09

asuraven


Committer: bugmaster Details Diff		 0027814: Parallelize BRepCheck_Analyzer

Change BRepCheck_Analyzer::Perform algorithm from recursion to 'for' loop
Add parallelization to BRepCheck_Analyzer::Perform
Add '-parallel' option to checkshape command to use parallelization. Default mode is single-thread.

mutex as Handle		 Affected Issues
0027814
mod - dox/user_guides/draw_test_harness/draw_test_harness.md Diff File
mod - src/BRepCheck/BRepCheck_Analyzer.cxx Diff File
mod - src/BRepCheck/BRepCheck_Analyzer.hxx Diff File
rm - src/BRepCheck/BRepCheck_Analyzer.lxx Diff File
rm - src/BRepCheck/BRepCheck_DataMapIteratorOfDataMapOfShapeListOfStatus.hxx Diff File
rm - src/BRepCheck/BRepCheck_DataMapIteratorOfDataMapOfShapeResult.hxx Diff File
mod - src/BRepCheck/BRepCheck_DataMapOfShapeListOfStatus.hxx Diff File
rm - src/BRepCheck/BRepCheck_DataMapOfShapeResult.hxx Diff File
mod - src/BRepCheck/BRepCheck_Edge.cxx Diff File
mod - src/BRepCheck/BRepCheck_Face.cxx Diff File
add - src/BRepCheck/BRepCheck_IndexedDataMapOfShapeResult.hxx Diff File
mod - src/BRepCheck/BRepCheck_ListOfStatus.hxx Diff File
mod - src/BRepCheck/BRepCheck_Result.cxx Diff File
mod - src/BRepCheck/BRepCheck_Result.hxx Diff File
rm - src/BRepCheck/BRepCheck_Result.lxx Diff File
mod - src/BRepCheck/BRepCheck_Shell.cxx Diff File
mod - src/BRepCheck/BRepCheck_Solid.cxx Diff File
mod - src/BRepCheck/BRepCheck_Vertex.cxx Diff File
mod - src/BRepCheck/BRepCheck_Wire.cxx Diff File
mod - src/BRepCheck/FILES Diff File
mod - src/BRepTest/BRepTest_CheckCommands.cxx Diff File
mod - src/Graphic3d/Graphic3d_MediaTexture.cxx Diff File
mod - src/Graphic3d/Graphic3d_MediaTexture.hxx Diff File
mod - src/Graphic3d/Graphic3d_MediaTextureSet.cxx Diff File
mod - src/Graphic3d/Graphic3d_MediaTextureSet.hxx Diff File
mod - src/Standard/Standard_Mutex.hxx Diff File
add - tests/heal/checkshape/begin Diff File
add - tests/heal/checkshape/bug27814_1 Diff File
add - tests/heal/checkshape/bug27814_10 Diff File
add - tests/heal/checkshape/bug27814_11 Diff File
add - tests/heal/checkshape/bug27814_2 Diff File
add - tests/heal/checkshape/bug27814_3 Diff File
add - tests/heal/checkshape/bug27814_4 Diff File
add - tests/heal/checkshape/bug27814_5 Diff File
add - tests/heal/checkshape/bug27814_6 Diff File
add - tests/heal/checkshape/bug27814_7 Diff File
add - tests/heal/checkshape/bug27814_8 Diff File
add - tests/heal/checkshape/bug27814_9 Diff File
mod - tests/heal/end Diff File
mod - tests/heal/grids.list Diff File

Issue History

Date Modified Username Field Change
2016-08-26 16:50 Timo New Issue
2016-08-26 16:50 Timo Assigned To => msv
2016-08-26 16:50 Timo File Added: cc.brep
2016-10-28 12:16 msv Target Version 7.1.0 => 7.2.0
2017-05-31 15:39 Timo Reporter Timo => Markus
2017-07-21 11:16 msv Target Version 7.2.0 => 7.3.0
2017-12-05 17:01 msv Target Version 7.3.0 => 7.4.0
2019-08-12 16:37 msv Target Version 7.4.0 => 7.5.0
2020-09-14 22:56 msv Target Version 7.5.0 => 7.6.0
2021-05-24 16:17 szy Assigned To msv => asuraven
2021-05-24 16:17 szy Status new => assigned
2021-05-25 12:49 asuraven Relationship added related to 0024524
2021-05-26 18:50 asuraven Note Added: 0101409
2021-06-18 10:00 asuraven Note Added: 0101910
2021-06-21 17:50 asuraven Note Added: 0101971
2021-06-21 19:14 git Note Added: 0101976
2021-06-21 19:17 git Note Added: 0101978
2021-06-21 19:22 git Note Added: 0101979
2021-06-23 14:18 asuraven Note Added: 0102010
2021-06-23 14:18 asuraven Note Added: 0102011
2021-06-23 15:23 git Note Added: 0102012
2021-06-24 17:53 git Note Added: 0102036
2021-06-24 19:50 git Note Added: 0102042
2021-06-24 21:15 kgv Summary Parallelize BRepCheck_Analyzer => Modeling Algorithms - Parallelize BRepCheck_Analyzer
2021-06-25 12:48 asuraven Note Added: 0102049
2021-06-28 19:12 git Note Added: 0102136
2021-06-28 19:16 git Note Added: 0102137
2021-06-28 19:16 git Note Added: 0102138
2021-06-28 19:16 asuraven Note Added: 0102139
2021-06-29 15:45 asuraven Note Added: 0102164
2021-06-30 16:27 asuraven Note Added: 0102187
2021-06-30 17:50 git Note Added: 0102189
2021-06-30 18:33 git Note Added: 0102191
2021-06-30 21:38 asuraven Note Added: 0102197
2021-06-30 21:38 asuraven Assigned To asuraven => msv
2021-06-30 21:38 asuraven Status assigned => resolved
2021-06-30 21:39 asuraven File Added: bug27814 performance.xlsx
2021-06-30 22:21 kgv Note Added: 0102198
2021-06-30 22:46 kgv Note Added: 0102199
2021-06-30 22:47 kgv Note Edited: 0102199
2021-06-30 22:49 kgv Note Edited: 0102199
2021-07-01 12:23 asuraven File Deleted: bug27814 performance.xlsx
2021-07-01 12:24 asuraven File Added: bug27814 performance.xlsx
2021-07-01 12:25 msv Note Added: 0102204
2021-07-01 12:26 asuraven Note Added: 0102205
2021-07-01 13:12 asuraven File Deleted: bug27814 performance.xlsx
2021-07-01 13:13 asuraven File Added: bug27814 performance.xlsx
2021-07-01 13:40 kgv Note Edited: 0102198
2021-07-01 15:55 msv Note Added: 0102210
2021-07-01 15:58 msv Note Added: 0102211
2021-07-01 15:59 msv Note Edited: 0102211
2021-07-02 09:41 asuraven Note Added: 0102224
2021-07-05 11:24 git Note Added: 0102279
2021-07-06 13:18 asuraven Note Added: 0102301
2021-07-06 18:39 git Note Added: 0102311
2021-07-06 20:25 git Note Added: 0102313
2021-07-06 20:26 git Note Added: 0102314
2021-07-06 20:49 asuraven Note Added: 0102315
2021-07-07 14:19 git Note Added: 0102324
2021-07-07 15:29 kgv Assigned To msv => asuraven
2021-07-07 15:29 kgv Status resolved => assigned
2021-07-07 15:42 git Note Added: 0102328
2021-07-07 16:19 git Note Added: 0102332
2021-07-07 18:59 git Note Added: 0102345
2021-07-07 19:00 git Note Added: 0102346
2021-07-08 17:11 asuraven Note Added: 0102378
2021-07-09 13:24 msv Note Added: 0102393
2021-07-09 14:18 msv Note Added: 0102395
2021-07-09 14:35 msv Note Added: 0102396
2021-07-09 14:39 msv Note Added: 0102397
2021-07-09 17:50 asuraven Note Added: 0102407
2021-07-12 17:03 git Note Added: 0102473
2021-07-12 18:14 asuraven Note Added: 0102477
2021-07-12 18:41 asuraven Note Edited: 0102477
2021-07-13 13:48 git Note Added: 0102490
2021-07-13 13:48 git Note Added: 0102491
2021-07-13 15:55 asuraven Note Added: 0102496
2021-07-13 17:25 asuraven Note Edited: 0102496
2021-07-13 20:17 git Note Added: 0102506
2021-07-13 20:18 git Note Added: 0102507
2021-07-14 14:51 asuraven Assigned To asuraven => msv
2021-07-14 14:52 asuraven Note Added: 0102521
2021-07-14 16:41 msv Note Added: 0102522
2021-07-14 16:41 msv Assigned To msv => asuraven
2021-07-14 16:51 git Note Added: 0102523
2021-07-14 19:38 git Note Added: 0102529
2021-07-14 19:47 git Note Added: 0102530
2021-07-15 11:52 git Note Added: 0102541
2021-07-15 11:52 git Note Added: 0102542
2021-07-15 15:04 asuraven Note Added: 0102545
2021-07-15 16:46 asuraven Note Added: 0102550
2021-07-15 18:36 asuraven Note Added: 0102556
2021-07-15 18:36 asuraven Assigned To asuraven => msv
2021-07-15 18:36 asuraven Status assigned => resolved
2021-07-15 20:53 msv Note Added: 0102558
2021-07-15 20:53 msv Assigned To msv => asuraven
2021-07-15 20:53 msv Status resolved => assigned
2021-07-16 11:32 asuraven Note Edited: 0102556
2021-07-16 12:19 git Note Added: 0102563
2021-07-16 12:19 git Note Added: 0102564
2021-07-16 12:20 asuraven Note Added: 0102565
2021-07-16 12:20 asuraven Assigned To asuraven => msv
2021-07-16 12:20 asuraven Status assigned => resolved
2021-07-16 15:58 msv Note Added: 0102580
2021-07-16 15:58 msv Assigned To msv => bugmaster
2021-07-16 15:58 msv Status resolved => reviewed
2021-07-16 17:50 kgv Note Added: 0102583
2021-07-16 18:02 msv Note Added: 0102584
2021-07-16 18:02 msv Assigned To bugmaster => asuraven
2021-07-16 18:02 msv Status reviewed => assigned
2021-07-16 18:10 git Note Added: 0102585
2021-07-16 18:23 git Note Added: 0102586
2021-07-16 18:24 asuraven Assigned To asuraven => kgv
2021-07-16 18:24 asuraven Status assigned => resolved
2021-07-19 12:11 git Note Added: 0102629
2021-07-19 12:14 git Note Added: 0102630
2021-07-19 12:15 git Note Added: 0102631
2021-07-19 14:31 kgv Note Added: 0102645
2021-07-19 14:37 kgv Note Added: 0102646
2021-07-19 14:37 kgv Assigned To kgv => asuraven
2021-07-19 14:37 kgv Status resolved => feedback
2021-07-19 17:11 git Note Added: 0102656
2021-07-19 18:05 git Note Added: 0102657
2021-07-19 19:20 asuraven Note Added: 0102658
2021-07-19 20:01 kgv Note Added: 0102661
2021-07-20 10:45 git Note Added: 0102672
2021-07-20 13:03 git Note Added: 0102676
2021-07-20 14:13 asuraven Note Added: 0102677
2021-07-20 14:13 asuraven Assigned To asuraven => kgv
2021-07-20 14:13 asuraven Status feedback => resolved
2021-07-20 14:25 kgv Note Added: 0102678
2021-07-20 14:25 kgv Assigned To kgv => asuraven
2021-07-20 14:25 kgv Status resolved => assigned
2021-07-20 15:02 git Note Added: 0102679
2021-07-20 15:04 asuraven Note Added: 0102680
2021-07-20 15:04 asuraven Assigned To asuraven => kgv
2021-07-20 15:04 asuraven Status assigned => resolved
2021-07-20 15:26 kgv Assigned To kgv => bugmaster
2021-07-20 15:26 kgv Status resolved => reviewed
2021-07-24 12:09 bugmaster Note Added: 0102761
2021-07-24 12:09 bugmaster Status reviewed => tested
2021-07-24 12:16 bugmaster Test case number => tests/heal/checkshape/bug27814_1,bug27814_10,bug27814_11,bug27814_2,bug27814_3,bug27814_4,bug27814_5,bug27814_6,bug27814_7,bug27814_8,bug27814_9
2021-07-24 12:30 kgv Relationship added parent of 0032492
2021-07-24 13:23 bugmaster Changeset attached => occt master 000c21fa
2021-07-24 13:23 bugmaster Status tested => verified
2021-07-24 13:23 bugmaster Resolution open => fixed
2021-07-24 13:27 git Note Added: 0102772
2021-07-24 13:27 git Note Added: 0102773
2021-07-24 13:27 git Note Added: 0102774
2021-07-24 13:28 git Note Added: 0102775
2021-07-24 13:28 git Note Added: 0102776
2021-07-24 13:28 git Note Added: 0102777
2021-08-29 19:22 msv Relationship replaced has duplicate 0024524