MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0031757Open CASCADE[OCCT] OCCT:Visualizationpublic2020-09-08 16:332020-09-20 11:14
Reporterage 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.5.0Fixed in Version 
Summary0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads
DescriptionBVH for Select3D_SensitiveEntity are built when SelectMgr_ViewerSelector::TraverseSensitives() is called. In case of big model this building step can take a lot of time, so the first selection will be slow. It is suggested to build these BVH in parallel with the main thread to decrease time of the first selection.
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberv3d/mesh/C1
Attached Files

- Relationships

-  Notes
(0094276)
git (administrator)
2020-09-09 09:19

Branch CR31757 has been created by age.

SHA-1: 6c427a792155d143385cf96fbde036744cf236c0


Detailed log of new commits:

Author: age
Date: Tue Sep 8 14:44:21 2020 +0300

    0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads
(0094277)
age (developer)
2020-09-09 09:23

Please review the first version. Tests will be added after approval.
(0094377)
kgv (developer)
2020-09-10 15:10

+Standard_Address SelectMgr_BVHThreadPool::buildBVHThreadFunc (Standard_Address data)

Please add OSD::SetThreadLocalSignal() at the begginning of thread execution and put algorithm execution into try/catch.

+  Standard_Integer aBVHThreadsNum = theNbThreads > 0 ? theNbThreads : OSD_Parallel::NbLogicalProcessors() 
> 1 ? OSD_Parallel::NbLogicalProcessors() - 1 : 1;

I would expect the number of background thread to be 1 for this task by default.

+    aPool->myBVHListMutex.Lock();

Please prefer using Standard_Mutex::Sentry when possible.

+  for()
...
+    if (aPool->myToStopBVHThread)
+    {
+      break;
...
+  return (Standard_Address)(0);

Tip: single `return` without `break` would be clearer.

+//! Class defining a thread pool for building BVH for Select3D_SensitiveEntity in multi-threaded mode.

+class SelectMgr_BVHThreadPool : public Standard_Transient

> Class defining a thread pool for building BVH for the list of Select3D_SensitiveEntity within background thread(s).

+  Standard_EXPORT void BuildBVH (const Handle(Select3D_SensitiveEntity)& theEntity);

void AddEntity()

+protected:
+  static Standard_Address buildBVHThreadFunc (Standard_Address data);

private

+  //! Structure that will be passed to a separate thread
+  struct BVHBuild_Data
+  {
+    SelectMgr_BVHThreadPool* Pool;
+    Standard_Mutex Mutex;
+  };

+  NCollection_Array1<OSD_Thread> myBVHThreads;                         //!< threads to build 
BVH
+  NCollection_Array1<BVHBuild_Data> myBVHBuildData;                    //!< list of mutexes 
for building BVH

It would be clearer subclassing OSD_Thread instead of managing several arrays.

+void SelectMgr_BVHThreadPool::BuildBVH (const Handle(Select3D_SensitiveEntity)& theEntity)
+{

Makes sense to check if BVH tree is already build in this method.

+          NCollection_Vector<Handle(SelectMgr_SensitiveEntity)>::Iterator anIter(aNewSel->Entities());

+          for (; anIter.More(); anIter.Next())

Please declare iterator within for(;;) header.

+  if (myToPrebuildBVH)
+  {
+    myBVHThreadPool->LockBVHBuildMutex();
+  }

Please implement SelectMgr_BVHThreadPool::Sentry object that would care about this locking and unlocking in similar manner as Standard_Mutex::Sentry does.

+  Standard_Boolean ToPrebuildBVH()

const method.


StdSelect_BRepSelectionTool::PreBuildBVH() will still perform synchronous BVH build for large Trees within the patch, which I suppose should be avoided while enabling a background thread.
(0094482)
git (administrator)
2020-09-11 16:50

Branch CR31757 has been updated by age.

SHA-1: 0656cd43c4b72990b6f55b07cf8767a2284e6abd


Detailed log of new commits:

Author: age
Date: Fri Sep 11 16:45:39 2020 +0300

    Fixed remarks

(0094483)
git (administrator)
2020-09-11 16:51

Branch CR31757_1 has been created by age.

SHA-1: 5801e9bb0bfaf9c36382cf4e56f6bd196ba36f24


Detailed log of new commits:

Author: age
Date: Tue Sep 8 14:44:21 2020 +0300

    0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads
(0094487)
git (administrator)
2020-09-11 17:13

Branch CR31757_1 has been updated forcibly by age.

SHA-1: e10662c7dea5c3e6b0767f01cdf79f0380cd552a
(0094495)
git (administrator)
2020-09-11 17:42

Branch CR31757_1 has been updated forcibly by age.

SHA-1: e31def80317f2d3168b67a92ea248c37aaaafde8
(0094695)
git (administrator)
2020-09-14 10:26

Branch CR31757_1 has been updated forcibly by age.

SHA-1: 062e6d45fd2c0c0b17a64cbb20ee2b7c60e07898
(0094705)
git (administrator)
2020-09-14 11:24

Branch CR31757_1 has been updated forcibly by age.

SHA-1: 631ab66ad9e7d170db29fd1e5240166ed6a1e64d
(0094737)
git (administrator)
2020-09-14 15:24

Branch CR31757_1 has been updated forcibly by age.

SHA-1: 4c3ff79d2ac2e3588444774bf9b9367f7bf286dc
(0094738)
age (developer)
2020-09-14 15:26

Remarks have been fixed, please review branch CR31757_1

http://occt-tests/CR31757_1-master-AGE-OCCT/Windows-64-VC14/diff_summary.html [^]
(0094755)
kgv (developer)
2020-09-14 16:45

+  myBVHThreads = NCollection_Array1<BVHThread>(1, aBVHThreadsNum);

NCollection_Array1::Resize().

+void SelectMgr_BVHThreadPool::AddEntity (const Handle(Select3D_SensitiveEntity)& theEntity)
+{
+  myBVHListMutex.Lock();
+  myBVHToBuildList.Append (theEntity);

Method still doesn't check if BVH tree of entity has been invalidated or not.

+  Standard_Boolean CheckAndResetFailures (TCollection_AsciiString& theFailures, Standard_Boolean 
theToRaise);

Current design does not force application to call CheckAndResetFailures()/WaitForBVHBuild() methods, so that failures list might potentially grow without limits.
So far we do not expect BVH builders to throw any exceptions, so that it sounds reasonable simplifying this logic and just print error messages to Messenger within working threads rather than introducing mechanism propagating them to main thread.

+  theCommands.Add("vbvhprebuild",

vselbvhbuild


+ theCommands.Add("vbvhprebuildwait",

vselbvhbuild -wait

+                  "vbvhprebuild {0|1} [-threadsNb value]"

-nbThreads

+      if (!anEntity->IsInstance (STANDARD_TYPE(Select3D_SensitiveGroup)))
+      {
+        continue;
+      }
+
+      Handle(Select3D_SensitiveGroup) aGroup = Handle(Select3D_SensitiveGroup)::DownCast (anEntity);


Redundant IsInstance() check - DownCast() should be enough.
> if (Handle(Select3D_SensitiveGroup) aGroup = Handle(Select3D_SensitiveGroup)::DownCast (anEntity)) {}

+{
+  myBVHListMutex.Lock();
+  myBVHToBuildList.Append (theEntity);
+  myWakeEvent.Set();
+  myIdleEvent.Reset();
+  myBVHListMutex.Unlock();
+

Standard_Mutex::Sentry for myBVHListMutex.

--- /dev/null
+++ b/src/SelectMgr/SelectMgr_BVHThreadPool.hxx
@@ -0,0 +1,148 @@
+#ifndef _SelectMgr_BVHThreadPool_HeaderFile
+#define _SelectMgr_BVHThreadPool_HeaderFile
...
--- /dev/null
+++ b/src/SelectMgr/SelectMgr_BVHThreadPool.cxx
@@ -0,0 +1,199 @@
+#include <SelectMgr_BVHThreadPool.hxx>

Missing file header.
(0094904)
git (administrator)
2020-09-16 12:50

Branch CR31757_1 has been updated by age.

SHA-1: 917c93eb4ed78e2c66f1b4ea4919d6d9f8882d95


Detailed log of new commits:

Author: age
Date: Wed Sep 16 12:53:19 2020 +0300

    Fixed remarks

(0094909)
git (administrator)
2020-09-16 14:23

Branch CR31757_1 has been updated by age.

SHA-1: d4f4e33449391feb6ca4f120cb17cb3b0ada2a94


Detailed log of new commits:

Author: age
Date: Wed Sep 16 14:26:26 2020 +0300

    Fix remarks 2

(0094910)
git (administrator)
2020-09-16 14:24

Branch CR31757_2 has been created by age.

SHA-1: 9769cc1e438e00d97758352ffaef53e696fa9e93


Detailed log of new commits:

Author: age
Date: Tue Sep 8 14:44:21 2020 +0300

    0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads
(0094911)
age (developer)
2020-09-16 14:25

Remarks have been fixed, please review

http://occt-tests/CR31757_1-master-AGE-OCCT/Windows-64-VC14/diff_summary.html [^]
(0094913)
kgv (developer)
2020-09-16 14:45

0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads CR31757_2
author	age <age@opencascade.com>	
    Tue, 8 Sep 2020 11:44:21 +0000 (14:44 +0300)
committer	age <age@opencascade.com>	
    Wed, 16 Sep 2020 11:27:08 +0000 (14:27 +0300)

Please add description of changes into commit message shortly describing:
- Addition of new method to Select3D_SensitiveEntity interface.
- Addition of new public method in selector for computing BVH in background and default behavior (remains the same).
- Addition of new class for managing background tasks.
(0094914)
git (administrator)
2020-09-16 15:03

Branch CR31757_2 has been updated forcibly by age.

SHA-1: 6fbb61cc2da1608d48ee87094a0a0d520c983137
(0094916)
age (developer)
2020-09-16 15:04

Done
(0094920)
git (administrator)
2020-09-16 15:20

Branch CR31757_2 has been updated forcibly by age.

SHA-1: 3fb1070d2cbc9e7e3f700dac31522ab398338d73
(0094922)
kgv (developer)
2020-09-16 15:21
edited on: 2020-09-16 15:21

Please raise the patch
- OCCT branch: CR31757_2.

(0095058)
abv (manager)
2020-09-19 09:30

The declared purpose of this change is to improve performance. Are there any measurable improvements actually achieved?
(0095065)
bugmaster (administrator)
2020-09-19 17:19

Combination -
OCCT branch : IR-2020-09-18
master SHA - b0b766826118f74b9857a932b8cec8c52a25c492
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-09-18 SHA - a6486d839da1ba1383ef6cc1a1a446a172f494c7
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: 17528.97000000011 / 17378.390000000145 [+0.87%]
Products
Total CPU difference: 12127.32000000009 / 12079.540000000095 [+0.40%]
Windows-64-VC14:
OCCT
Total CPU difference: 18862.703125 / 18898.921875 [-0.19%]
Products
Total CPU difference: 13314.828125 / 13329.21875 [-0.11%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0095111)
git (administrator)
2020-09-20 11:13

Branch CR31757_2 has been deleted by inv.

SHA-1: 3fb1070d2cbc9e7e3f700dac31522ab398338d73
(0095112)
git (administrator)
2020-09-20 11:13

Branch CR31757_1 has been deleted by inv.

SHA-1: d4f4e33449391feb6ca4f120cb17cb3b0ada2a94
(0095133)
git (administrator)
2020-09-20 11:14

Branch CR31757 has been deleted by inv.

SHA-1: 0656cd43c4b72990b6f55b07cf8767a2284e6abd

- Related Changesets
occt: master 6a2fb7a1
Timestamp: 2020-09-08 11:44:21
Author: age
Committer: bugmaster
Details ] Diff ]
0031757: Visualization - Prebuild BVH for Select3D_SensitiveEntity in separate threads

- Added a new mode in SelectMgr_ViewerSelector for computing BVH for Select3D_SensitiveEntity in background which can be activated via method SelectMgr_ViewerSelector::SetToPrebuildBVH(). Default behavior has not been changed.
- New class SelectMgr_BVHThreadPool manages background processing of BVH building queue.
- Added Select3D_SensitiveEntity::ToBuildBVH() method that checks if BVH (if it used) is in invalidated state. Defined this method for all standard classes inherited from Select3D_SensitiveEntity.
mod - src/AIS/AIS_ColoredShape.cxx Diff ] File ]
mod - src/MeshVS/MeshVS_DummySensitiveEntity.hxx Diff ] File ]
mod - src/MeshVS/MeshVS_Mesh.cxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveBox.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveCircle.cxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveCircle.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveEntity.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveFace.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitivePoint.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveSegment.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveSet.hxx Diff ] File ]
mod - src/Select3D/Select3D_SensitiveTriangle.hxx Diff ] File ]
mod - src/SelectMgr/FILES Diff ] File ]
add - src/SelectMgr/SelectMgr_BVHThreadPool.cxx Diff ] File ]
add - src/SelectMgr/SelectMgr_BVHThreadPool.hxx Diff ] File ]
mod - src/SelectMgr/SelectMgr_SelectionManager.cxx Diff ] File ]
mod - src/SelectMgr/SelectMgr_SelectionManager.hxx Diff ] File ]
mod - src/SelectMgr/SelectMgr_ViewerSelector.cxx Diff ] File ]
mod - src/SelectMgr/SelectMgr_ViewerSelector.hxx Diff ] File ]
mod - src/StdSelect/StdSelect_BRepSelectionTool.cxx Diff ] File ]
mod - src/ViewerTest/ViewerTest_ViewerCommands.cxx Diff ] File ]
add - tests/v3d/mesh/C1 Diff ] File ]

- Issue History
Date Modified Username Field Change
2020-09-08 16:33 age New Issue
2020-09-08 16:33 age Assigned To => age
2020-09-09 09:19 git Note Added: 0094276
2020-09-09 09:20 age Status new => assigned
2020-09-09 09:23 age Note Added: 0094277
2020-09-09 09:23 age Assigned To age => kgv
2020-09-09 09:23 age Status assigned => resolved
2020-09-09 09:23 age Steps to Reproduce Updated View Revisions
2020-09-10 15:10 kgv Note Added: 0094377
2020-09-11 16:50 git Note Added: 0094482
2020-09-11 16:51 git Note Added: 0094483
2020-09-11 17:13 git Note Added: 0094487
2020-09-11 17:42 git Note Added: 0094495
2020-09-14 10:26 git Note Added: 0094695
2020-09-14 11:24 git Note Added: 0094705
2020-09-14 15:24 git Note Added: 0094737
2020-09-14 15:26 age Note Added: 0094738
2020-09-14 16:45 kgv Note Added: 0094755
2020-09-16 12:50 git Note Added: 0094904
2020-09-16 14:06 kgv Assigned To kgv => age
2020-09-16 14:06 kgv Status resolved => assigned
2020-09-16 14:06 kgv Severity minor => feature
2020-09-16 14:06 kgv Product Version 7.5.0 =>
2020-09-16 14:23 git Note Added: 0094909
2020-09-16 14:24 git Note Added: 0094910
2020-09-16 14:25 age Note Added: 0094911
2020-09-16 14:25 age Assigned To age => kgv
2020-09-16 14:25 age Status assigned => resolved
2020-09-16 14:45 kgv Note Added: 0094913
2020-09-16 14:45 kgv Assigned To kgv => age
2020-09-16 14:45 kgv Status resolved => assigned
2020-09-16 15:03 git Note Added: 0094914
2020-09-16 15:04 age Note Added: 0094916
2020-09-16 15:04 age Assigned To age => kgv
2020-09-16 15:04 age Status assigned => resolved
2020-09-16 15:20 git Note Added: 0094920
2020-09-16 15:21 kgv Note Added: 0094922
2020-09-16 15:21 kgv Assigned To kgv => bugmaster
2020-09-16 15:21 kgv Status resolved => reviewed
2020-09-16 15:21 kgv Note Edited: 0094922 View Revisions
2020-09-19 09:30 abv Note Added: 0095058
2020-09-19 15:41 bugmaster Test case number => v3d/mesh/C1
2020-09-19 17:19 bugmaster Note Added: 0095065
2020-09-19 17:19 bugmaster Status reviewed => tested
2020-09-20 10:55 bugmaster Changeset attached => occt master 6a2fb7a1
2020-09-20 10:55 bugmaster Status tested => verified
2020-09-20 10:55 bugmaster Resolution open => fixed
2020-09-20 11:13 git Note Added: 0095111
2020-09-20 11:13 git Note Added: 0095112
2020-09-20 11:14 git Note Added: 0095133


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker