MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027814Community[OCCT] OCCT:Modeling Algorithmspublic2016-08-26 16:502021-08-29 19:22
ReporterMarkus 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.0.0 
Target Version[OCCT] 7.6.0*Fixed in Version 
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? file icon cc.brep (6,006,463 bytes) 2016-08-26 16:50
xlsx file icon bug27814 performance.xlsx (19,541 bytes) 2021-07-01 13:13

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

-  Notes
(0101409)
asuraven (developer)
2021-05-26 18:50

(0101910)
asuraven (developer)
2021-06-18 10:00

(0101971)
asuraven (developer)
2021-06-21 17:50

(0101976)
git (administrator)
2021-06-21 19:14

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
(0101978)
git (administrator)
2021-06-21 19:17

Branch CR27814 has been deleted by asuraven.

SHA-1: c1a2703a28262570b1931941a373af2a85abc329
(0101979)
git (administrator)
2021-06-21 19:22

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
(0102010)
asuraven (developer)
2021-06-23 14:18

(0102011)
asuraven (developer)
2021-06-23 14:18

(0102012)
git (administrator)
2021-06-23 15:23

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

(0102036)
git (administrator)
2021-06-24 17:53

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

(0102042)
git (administrator)
2021-06-24 19:50

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

(0102049)
asuraven (developer)
2021-06-25 12:48

(0102136)
git (administrator)
2021-06-28 19:12

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

(0102137)
git (administrator)
2021-06-28 19:16

Branch CR27814 has been deleted by asuraven.

SHA-1: 66ba96acdc32ef840bd32b5aba8cb670135b586a
(0102138)
git (administrator)
2021-06-28 19:16

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
(0102139)
asuraven (developer)
2021-06-28 19:16

(0102164)
asuraven (developer)
2021-06-29 15:45

(0102187)
asuraven (developer)
2021-06-30 16:27

(0102189)
git (administrator)
2021-06-30 17:50

Branch CR27814 has been deleted by asuraven.

SHA-1: f2a8facb543eb0ff7828518098fbd95e4572095d
(0102191)
git (administrator)
2021-06-30 18:33

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.
(0102197)
asuraven (developer)
2021-06-30 21:38

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/ [^]
(0102198)
kgv (developer)
2021-06-30 22:21
edited on: 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.

(0102199)
kgv (developer)
2021-06-30 22:46
edited on: 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?

(0102204)
msv (developer)
2021-07-01 12:25

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
(0102205)
asuraven (developer)
2021-07-01 12:26

Using TBB has same performance as built-in thread pool. See results in updated attached table
(0102210)
msv (developer)
2021-07-01 15:55

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'.
(0102211)
msv (developer)
2021-07-01 15:58
edited on: 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);


(0102224)
asuraven (developer)
2021-07-02 09:41

(0102279)
git (administrator)
2021-07-05 11:24

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

(0102301)
asuraven (developer)
2021-07-06 13:18

(0102311)
git (administrator)
2021-07-06 18:39

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.
(0102313)
git (administrator)
2021-07-06 20:25

Branch CR27814_1 has been deleted by asuraven.

SHA-1: efe79cecd037f15ca0fc5bcb1bd10e33dfbeb6ef
(0102314)
git (administrator)
2021-07-06 20:26

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.
(0102315)
asuraven (developer)
2021-07-06 20:49

(0102324)
git (administrator)
2021-07-07 14:19

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

(0102328)
git (administrator)
2021-07-07 15:42

Branch CR27814 has been deleted by asuraven.

SHA-1: 6481f3e43931047a0ddc98243af5a4f316cd1293
(0102332)
git (administrator)
2021-07-07 16:19

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.
(0102345)
git (administrator)
2021-07-07 18:59

Branch CR27814_1 has been deleted by asuraven.

SHA-1: 2f7dad9028edf0021b7fc2ea98b500f32915f25f
(0102346)
git (administrator)
2021-07-07 19:00

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.
(0102378)
asuraven (developer)
2021-07-08 17:11

(0102393)
msv (developer)
2021-07-09 13:24

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.
(0102395)
msv (developer)
2021-07-09 14:18

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.
(0102396)
msv (developer)
2021-07-09 14:35

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.
(0102397)
msv (developer)
2021-07-09 14:39

Review.
(0102407)
asuraven (developer)
2021-07-09 17:50

(0102473)
git (administrator)
2021-07-12 17:03

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.
(0102477)
asuraven (developer)
2021-07-12 18:14
edited on: 2021-07-12 18:41



(0102490)
git (administrator)
2021-07-13 13:48

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 94cac8ceb16b72f476a46c6288f0047ebeb4b911
(0102491)
git (administrator)
2021-07-13 13:48

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.
(0102496)
asuraven (developer)
2021-07-13 15:55
edited on: 2021-07-13 17:25



(0102506)
git (administrator)
2021-07-13 20:17

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 5d5a6cadf3b088d26e275ffd60bf0a2bf1fa946b
(0102507)
git (administrator)
2021-07-13 20:18

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.
(0102521)
asuraven (developer)
2021-07-14 14:52

@msv see changes in branch CR27814_2
http://jenkins-test-occt/view/CR27814_2-master-ASURAVEN/view/COMPARE/ [^]
(0102522)
msv (developer)
2021-07-14 16:41

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".
(0102523)
git (administrator)
2021-07-14 16:51

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
(0102529)
git (administrator)
2021-07-14 19:38

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 67f8beb8541ece6f520ec63a2490901a98dfe45c
(0102530)
git (administrator)
2021-07-14 19:47

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.
(0102541)
git (administrator)
2021-07-15 11:52

Branch CR27814_2 has been deleted by asuraven.

SHA-1: a34acf3287bcf1b8a7c342b2444eedae5c35fde7
(0102542)
git (administrator)
2021-07-15 11:52

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.
(0102545)
asuraven (developer)
2021-07-15 15:04

(0102550)
asuraven (developer)
2021-07-15 16:46

(0102556)
asuraven (developer)
2021-07-15 18:36
edited on: 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

(0102558)
msv (developer)
2021-07-15 20:53

Please correct compilation warnings.
(0102563)
git (administrator)
2021-07-16 12:19

Branch CR27814_2 has been deleted by asuraven.

SHA-1: 5423230987b9c27b73f428ce7d40af1779886c72
(0102564)
git (administrator)
2021-07-16 12:19

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.
(0102565)
asuraven (developer)
2021-07-16 12:20

fixed in CR27814_2
(0102580)
msv (developer)
2021-07-16 15:58

OCCT - CR27814_2
Products - none.
(0102583)
kgv (developer)
2021-07-16 17:50

+ 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") {}
(0102584)
msv (developer)
2021-07-16 18:02

Agree with Kirill's remarks.
(0102585)
git (administrator)
2021-07-16 18:10

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

(0102586)
git (administrator)
2021-07-16 18:23

Branch CR27814_2 has been updated forcibly by asuraven.

SHA-1: 288077188e24fce51b942cc69340160edfad293e
(0102629)
git (administrator)
2021-07-19 12:11

Branch CR27814_2 has been updated forcibly by asuraven.

SHA-1: ae3e6594bceb9b59a46a3f2c60f02aa35c9fc94c
(0102630)
git (administrator)
2021-07-19 12:14

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.
(0102631)
git (administrator)
2021-07-19 12:15

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.
(0102645)
kgv (developer)
2021-07-19 14:31

Cosmetic changes have been pushed to CR27814_4.
(0102646)
kgv (developer)
2021-07-19 14:37

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
  }
(0102656)
git (administrator)
2021-07-19 17:11

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

(0102657)
git (administrator)
2021-07-19 18:05

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: c2a3684122f8a12eb9dfe6204818f264bcd534e4
(0102658)
asuraven (developer)
2021-07-19 19:20

(0102661)
kgv (developer)
2021-07-19 20:01

(0102672)
git (administrator)
2021-07-20 10:45

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 2b0e1e89254428041607ba24b1daa15a8dbc2ef6
(0102676)
git (administrator)
2021-07-20 13:03

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 6eb57d1922b1cf575dea0f924853d40c1f8475d6
(0102677)
asuraven (developer)
2021-07-20 14:13

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/ [^]
(0102678)
kgv (developer)
2021-07-20 14:25

--- 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?
(0102679)
git (administrator)
2021-07-20 15:02

Branch CR27814_4 has been updated forcibly by asuraven.

SHA-1: 228024e3384dff4ba43f19dc8564b5fabc834c61
(0102680)
asuraven (developer)
2021-07-20 15:04

myMutex was deleted
(0102761)
bugmaster (administrator)
2021-07-24 12:09

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
(0102772)
git (administrator)
2021-07-24 13:27

Branch CR27814 has been deleted by mnt.

SHA-1: c9ddc4ee06bfb8ecfb03f237c4657262e369aba4
(0102773)
git (administrator)
2021-07-24 13:27

Branch CR27814_1 has been deleted by mnt.

SHA-1: 6dd600f91426aa59c9204715bfc43beb4576d4dc
(0102774)
git (administrator)
2021-07-24 13:27

Branch CR27814_2 has been deleted by mnt.

SHA-1: ae3e6594bceb9b59a46a3f2c60f02aa35c9fc94c
(0102775)
git (administrator)
2021-07-24 13:28

Branch CR27814_2_kgv has been deleted by mnt.

SHA-1: 6b3acca765c05f45d65e01918013f826cd6ecf97
(0102776)
git (administrator)
2021-07-24 13:28

Branch CR27814_3 has been deleted by mnt.

SHA-1: 69e5a92648c9fdb823069c8905d688b4514f936a
(0102777)
git (administrator)
2021-07-24 13:28

Branch CR27814_4 has been deleted by mnt.

SHA-1: 228024e3384dff4ba43f19dc8564b5fabc834c61

- Related Changesets
occt: master 000c21fa
Timestamp: 2021-06-21 16:15:09
Author: 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
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 View Revisions
2021-06-30 22:49 kgv Note Edited: 0102199 View Revisions
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 View Revisions
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 View Revisions
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 View Revisions
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 View Revisions
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 View Revisions
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 14:39 kgv Note Added: 0102576
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:38 kgv Note Deleted: 0102576
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


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker