View Issue Details

IDProjectCategoryView StatusLast Update
0030618Open CASCADEOCCT:Modeling Algorithmspublic2019-04-21 11:06
Reporterkgv Assigned Toapn  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.4.0 
Target Version7.4.0Fixed in Version7.4.0 
Summary0030618: Modeling Algorithms, BOPTools_Parallel - avoid using map for thread-local contexts without TBB
DescriptionBOPTools_ContextFunctor allocates a map of per-thread contexts using a global thread id as map key:
template <class TypeSolver,  class TypeSolverVector,
          class TypeContext, typename TN>
class BOPTools_ContextFunctor
{
  typedef NCollection_DataMap<Standard_ThreadId, TypeContext, Hasher> ContextMap;

public:

  //! Constructor
  explicit BOPTools_ContextFunctor( TypeSolverVector& theVector )
  : mySolverVector(theVector) {}

  //! Binds main thread context
  void SetContext( TypeContext& theContext )
  {
    myContexts.Bind(OSD_Thread::Current(), theContext);
  }

  //! Returns current thread context
  TypeContext& GetThreadContext() const
  {
    const Standard_ThreadId aThreadID = OSD_Thread::Current();
    if ( myContexts.IsBound(aThreadID) )
    {
      TypeContext& aContext = myContexts(aThreadID);
      if ( aContext.IsNull() == Standard_False )
        return aContext;
    }

    // Create new context
    TypeContext aContext = new TN
      ( NCollection_BaseAllocator::CommonBaseAllocator() );

    Standard_Mutex::Sentry aLocker(myMutex);
    myContexts.Bind(aThreadID, aContext);

    return myContexts(aThreadID);
  }

  //! Defines functor interface
  void operator()( const Standard_Integer theIndex ) const
  {
    TypeContext& aContext = GetThreadContext();
    TypeSolver&  aSolver  = mySolverVector(theIndex);

    aSolver.SetContext(aContext);
    aSolver.Perform();
  }

private:
  TypeSolverVector&      mySolverVector;
  mutable ContextMap     myContexts;
  mutable Standard_Mutex myMutex;
};


Supposedly, this is done to overcome TBB interface limitations.

When using OSD_ThreadPool instead of TBB, it is desired using an array instead of a map and implement functor taking thread index within thread pool as input:
  void operator() (int theThreadIndex,
                   int theElemIndex) const
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0029935 closedbugmaster Foundation Classes - introduce OSD_ThreadPool class defining a thread pool 
related to 0030620 closedbugmaster Tests - perf/bop/buildfaces does not match description (broken) 

Activities

git

2019-03-29 00:23

administrator   ~0083286

Branch CR30618 has been created by kgv.

SHA-1: d8ee52cf0066ca7121f2970a2c7bfd46da7e03a5


Detailed log of new commits:

Author: kgv
Date: Fri Mar 29 00:14:36 2019 +0300

    0030618: Modeling Algorithms, BOPTools_Parallel - avoid using map for thread-local contexts without TBB
    
    OSD_Parallel::ToUseOcctThreads() - new flag allowing to use OCCT threads implementation even when compiled with TBB (for testing).
    Added new command dparallel for managing default Thread Pool.
    
    BOPTools_Parallel - eliminated redundant typedefs/explicit instantiations of templates.
    Added functor using array of per-thread context instead of a map.

git

2019-03-29 01:29

administrator   ~0083287

Branch CR30618 has been updated forcibly by kgv.

SHA-1: 89a11facd731363e9112a622200c8f711bbc28fd

git

2019-03-29 01:31

administrator   ~0083288

Branch CR30618_no_tbb has been created by kgv.

SHA-1: f8a97e9b056ed380e6ef806e36e016a856c195c7


Detailed log of new commits:

Author: kgv
Date: Fri Mar 29 01:25:17 2019 +0300

    no TBB [for testing]

git

2019-03-29 01:41

administrator   ~0083289

Branch CR30618_no_tbb has been updated forcibly by kgv.

SHA-1: 5ed378d64c8229c55afef02a0018a4ea0ee2ccca

kgv

2019-03-29 08:28

developer   ~0083291

Patch is ready for review.
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR30618-master-KGV/

Avoiding maps shows subtle performance difference on checked couple of use cases,
so that patch should be more considered more as general small code clean up of BOPTools_Parallel.

kgv

2019-03-29 22:23

developer   ~0083301

Last edited: 2019-03-29 22:24

I was curious how the magic with mutex-less map within BOPTools_ContextFunctor::GetThreadContext() actually works and does not boom (mutex is used only with Bind(), but not for reads).

But now I get it:
BOPTools_ContextFunctor::SetContext() once before starting algorithm, which resizes the map to the first prime number 101.
This is relatively safe on usual modern workstations having amount of cores far beyond this limit (up to 8 cores / 16 threads),
but server and future high-end configurations are much closer to 101 limit
(AMD Ryzen Threadripper 2990WX has 32 cores / 64 threads nowadays, with some promises about 64 cores / 128 threads in next generations).

git

2019-04-02 11:56

administrator   ~0083355

Branch CR30618 has been updated forcibly by kgv.

SHA-1: f07057e9b2c3eb0f6047449ac8f4a3b6cae43341

msv

2019-04-05 13:41

developer   ~0083431

src/Draw/Draw_BasicCommands.cxx
- 868: the option -tbb is not documented.

git

2019-04-05 14:52

administrator   ~0083434

Branch CR30618 has been updated by kgv.

SHA-1: 1de4bb5149f78793a4e1df2bf1a0e29e3cb292be


Detailed log of new commits:

Author: kgv
Date: Fri Apr 5 14:46:54 2019 +0300

    # correct help

kgv

2019-04-05 14:53

developer   ~0083435

Patch has been updated.

msv

2019-04-05 16:14

developer   ~0083438

Help "[-occt|-tbb]" is not consistent with implementation where boolean argument is expected.

kgv

2019-04-05 16:37

developer   ~0083439

> Help "[-occt|-tbb]" is not consistent with implementation
> where boolean argument is expected.
This is usual syntax of on/off argument, when no extra value means 1.

git

2019-04-05 16:41

administrator   ~0083440

Branch CR30618 has been updated by kgv.

SHA-1: 6d6d8459599ee19175341aa624316741745c78bd


Detailed log of new commits:

Author: kgv
Date: Fri Apr 5 16:35:12 2019 +0300

    # correction

msv

2019-04-05 17:34

developer   ~0083443

It is better to return to commit SHA-1: f07057e9b2c3eb0f6047449ac8f4a3b6cae43341.

git

2019-04-05 17:35

administrator   ~0083444

Branch CR30618 has been updated forcibly by kgv.

SHA-1: f07057e9b2c3eb0f6047449ac8f4a3b6cae43341

kgv

2019-04-05 17:36

developer   ~0083445

Reverted to previous state.

msv

2019-04-05 17:38

developer   ~0083446

Reviewed.

apn

2019-04-08 15:48

administrator   ~0083522

Combination -
OCCT branch : CR30618
master SHA - 89a11facd731363e9112a622200c8f711bbc28fd
d67d4b811012eef8913d3c535c29654d0acf3c4c
Products branch : master SHA - 63ea473ef04f4f06e4baccecde3b9b8fd4a069a3
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: 16593.659999999934 / 16549.040000000037 [+0.27%]
Products
Total CPU difference: 9133.78000000005 / 9115.580000000047 [+0.20%]
Windows-64-VC14:
OCCT
Total CPU difference: 17918.5625 / 17926.078125 [-0.04%]
Products
Total CPU difference: 10568.515625 / 10583.1875 [-0.14%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2019-04-14 15:41

administrator   ~0083719

Branch CR30618 has been deleted by inv.

SHA-1: f07057e9b2c3eb0f6047449ac8f4a3b6cae43341

git

2019-04-21 11:06

administrator   ~0083908

Branch CR30618_no_tbb has been deleted by inv.

SHA-1: 5ed378d64c8229c55afef02a0018a4ea0ee2ccca

Related Changesets

occt: master fc867b96

2019-03-28 21:14:36

kgv


Committer: apn Details Diff
0030618: Modeling Algorithms, BOPTools_Parallel - avoid using map for thread-local contexts without TBB

OSD_Parallel::ToUseOcctThreads() - new flag allowing to use OCCT threads implementation even when compiled with TBB (for testing).
Added new command dparallel for managing default Thread Pool.
OSD_Parallel::For() now avoid creation of universal iterator in simplest case.

BOPTools_Parallel - eliminated redundant typedefs/explicit instantiations of templates.
Added functor using array of per-thread context instead of a map.
Affected Issues
0030618
mod - src/BOPAlgo/BOPAlgo_Builder_2.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_Builder_3.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_CheckerSI.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_CheckerSI_1.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_2.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_3.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_4.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_5.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_6.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_PaveFiller_9.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_RemoveFeatures.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_ShellSplitter.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_Tools.cxx Diff File
mod - src/BOPAlgo/BOPAlgo_WireSplitter.cxx Diff File
mod - src/BOPDS/BOPDS_Iterator.cxx Diff File
mod - src/BOPTools/BOPTools_AlgoTools_1.cxx Diff File
mod - src/BOPTools/BOPTools_Parallel.hxx Diff File
mod - src/Draw/Draw_BasicCommands.cxx Diff File
mod - src/OSD/OSD_Parallel.cxx Diff File
mod - src/OSD/OSD_Parallel.hxx Diff File
mod - src/OSD/OSD_Parallel_TBB.cxx Diff File
mod - src/OSD/OSD_Parallel_Threads.cxx Diff File
mod - src/OSD/OSD_ThreadPool.cxx Diff File
mod - src/OSD/OSD_ThreadPool.hxx Diff File

Issue History

Date Modified Username Field Change
2019-03-28 17:34 kgv New Issue
2019-03-28 17:34 kgv Assigned To => msv
2019-03-29 00:23 kgv Summary Modeling Algorithms - BOPTools_ContextFunctor should use array for thread-local contexts instead of a map without TBB => Modeling Algorithms, BOPTools_Parallel - avoid using map for thread-local contexts without TBB
2019-03-29 00:23 git Note Added: 0083286
2019-03-29 01:29 git Note Added: 0083287
2019-03-29 01:31 git Note Added: 0083288
2019-03-29 01:41 git Note Added: 0083289
2019-03-29 08:28 kgv Note Added: 0083291
2019-03-29 08:28 kgv Status new => resolved
2019-03-29 11:27 kgv Relationship added related to 0029935
2019-03-29 11:34 kgv Relationship added related to 0030620
2019-03-29 22:23 kgv Note Added: 0083301
2019-03-29 22:24 kgv Note Edited: 0083301
2019-03-29 22:24 kgv Note Edited: 0083301
2019-04-02 11:56 git Note Added: 0083355
2019-04-05 13:41 msv Note Added: 0083431
2019-04-05 13:42 msv Assigned To msv => kgv
2019-04-05 13:42 msv Status resolved => assigned
2019-04-05 14:52 git Note Added: 0083434
2019-04-05 14:53 kgv Note Added: 0083435
2019-04-05 14:53 kgv Assigned To kgv => msv
2019-04-05 14:53 kgv Status assigned => resolved
2019-04-05 16:14 msv Note Added: 0083438
2019-04-05 16:14 msv Assigned To msv => kgv
2019-04-05 16:14 msv Status resolved => assigned
2019-04-05 16:37 kgv Note Added: 0083439
2019-04-05 16:41 git Note Added: 0083440
2019-04-05 16:41 kgv Assigned To kgv => msv
2019-04-05 16:41 kgv Status assigned => resolved
2019-04-05 17:34 msv Note Added: 0083443
2019-04-05 17:35 git Note Added: 0083444
2019-04-05 17:36 kgv Note Added: 0083445
2019-04-05 17:38 msv Note Added: 0083446
2019-04-05 17:38 msv Assigned To msv => bugmaster
2019-04-05 17:38 msv Status resolved => reviewed
2019-04-08 15:48 apn Test case number => Not needed
2019-04-08 15:48 apn Note Added: 0083522
2019-04-08 15:48 apn Status reviewed => tested
2019-04-14 15:36 apn Changeset attached => occt master fc867b96
2019-04-14 15:36 apn Assigned To bugmaster => apn
2019-04-14 15:36 apn Status tested => verified
2019-04-14 15:36 apn Resolution open => fixed
2019-04-14 15:41 git Note Added: 0083719
2019-04-21 11:06 git Note Added: 0083908