View Issue Details

IDProjectCategoryView StatusLast Update
0030959Open CASCADEOCCT:Foundation Classespublic2019-09-15 10:55
Reporteroan Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version7.4.0Fixed in Version7.4.0 
Summary0030959: OSD_Parallel_TBB: number of execution threads is strictly limited by the root scope
DescriptionIn case of TBB usage, number of execution threads is regulated by task_scheduler_init as minimum from number of elements to be processed and default value specfied by OSD_ThreadPool.

Given that parallel task itself performs some computations in parallel, number of threads remains the same and equal to value specified by the root scope even if several more threads could be allocated according to OSD_ThreadPool.

This leads to inconsistency between specified parameter and expected number of threads used by algorithms.
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberNot required

Relationships

child of 0030573 closedapn OSD_Parallel_TBB: limit number of execution threads using settings of OSD_ThreadPool::DefaultPool() 

Activities

git

2019-09-11 11:07

administrator   ~0086999

Branch CR30959 has been created by oan.

SHA-1: 497e7d52eb77817a2dcd99877442ad5744a7e5aa


Detailed log of new commits:

Author: oan
Date: Wed Sep 11 11:04:03 2019 +0300

    0030959: OSD_Parallel_TBB: number of execution threads is strictly limited by the root scope
    
    Do not limit number of available threads by number of items processed within the root scope due to possibility of spawning of an additional threads within the subscopes.

oan

2019-09-11 13:11

developer   ~0087007

Please review:

http://occt-tests/CR30959-master-OAN-OCCT/Windows-64-VC14/diff_summary.html
http://occt-tests/CR30959-master-OAN-OCCT/Debian80-64/diff_summary.html

msv

2019-09-11 18:39

developer   ~0087014

For tbb usage, it is better not to set number of threads explicitly. So, I would not called task_scheduler_init at all, or called it with default value (automatic).
The quote from tbb ref manual:
"It is preferable option for production code because it helps to avoid nasty surprises when several TBB based components run side-by-side or in a nested fashion inside the same process."

oan

2019-09-12 11:18

developer   ~0087018

Mikhail,

I beg your pardon, but I didn't get your remark completely.

>> For tbb usage, it is better not to set number of threads explicitly.
What does it mean given that the option to control number of threads already exists both for TBB and Native implementation of OSD_Parallel and requested about a year ago by one of our customers and already integrated in OCCT?

Moreover, TBB provides functionaltiy to set number of threads explicitly via task_scheduler_init, it is the aim of this official function provided by TBB, so it is natural feature and it is not "better" to avoid it completely, especially if such feature is needed for various projects (0030573 and 0028199).

>> So, I would not called task_scheduler_init at all, or called it with default value (automatic).
First, what do you suggest to limit number of threads in case of TBB if you removed task_scheduler_init at all?
If you don't have any constructive idea, task_scheduler_init is the only solution in this case.
Let's consider existing code and changes introduced by current patch:

master:
const Standard_Integer aNbThreads = theNbItems > 0 ?
      Min (theNbItems, aThreadPool->NbDefaultThreadsToLaunch()) : -1;

patch:
const Standard_Integer aNbThreads = theNbItems > 0 ?
      aThreadPool->NbDefaultThreadsToLaunch() : -1;

Here, the only change is not to limit scheduler by minimum of items to be processed and default number of threads, but use default number of threads explicitly (see description of the issue to figure out the reason).

This is the documentation note about theNbItems:
//! @param theNbItems number of items passed by iterator, -1 if unknown

Value -1 is exactly that "default (automatic)" value that leads to standard behaviour of TBB procedures.

msv

2019-09-12 11:46

developer   ~0087020

It was just my general remark about implementation of this functionality. It is that tbb implementation is much smarter than our, and such way we restrict tbb's smartness. One feature is supporting of affinity mask. Tbb by default restricts the number of threads taking into account affinity mask if any.

I agree that if we decide to do more smart behavior we will do it in scope of another issue.

So, this patch is reviewed.

bugmaster

2019-09-12 18:47

administrator   ~0087029

Combination -
OCCT branch : CR30959
master SHA - 497e7d52eb77817a2dcd99877442ad5744a7e5aa
5f5b1aed1c6e139bbd34314eca77ae7abcd8895c
Products branch : master SHA - f9d0bd5e3a29d6a97b3f5f6354ea2397253ab4f8
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: 16840.53000000009 / 16836.290000000165 [+0.03%]
Products
Total CPU difference: 10570.34000000003 / 10553.510000000031 [+0.16%]
Windows-64-VC14:
OCCT
Total CPU difference: 18333.484375 / 18300.0625 [+0.18%]
Products
Total CPU difference: 12402.859375 / 12227.453125 [+1.43%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2019-09-15 10:55

administrator   ~0087098

Branch CR30959 has been deleted by inv.

SHA-1: 497e7d52eb77817a2dcd99877442ad5744a7e5aa

Related Changesets

occt: master 3068c3bb

2019-09-11 08:04:03

oan


Committer: bugmaster Details Diff
0030959: OSD_Parallel_TBB: number of execution threads is strictly limited by the root scope

Do not limit number of available threads by number of items processed within the root scope due to possibility of spawning of an additional threads within the subscopes.
Affected Issues
0030959
mod - src/OSD/OSD_Parallel_TBB.cxx Diff File

Issue History

Date Modified Username Field Change
2019-09-11 11:02 oan New Issue
2019-09-11 11:02 oan Assigned To => abv
2019-09-11 11:03 oan Relationship added child of 0030573
2019-09-11 11:07 git Note Added: 0086999
2019-09-11 13:11 oan Note Added: 0087007
2019-09-11 13:11 oan Assigned To abv => msv
2019-09-11 13:11 oan Status new => resolved
2019-09-11 13:11 oan Steps to Reproduce Updated
2019-09-11 18:39 msv Note Added: 0087014
2019-09-11 18:39 msv Assigned To msv => oan
2019-09-11 18:39 msv Status resolved => assigned
2019-09-12 11:18 oan Note Added: 0087018
2019-09-12 11:18 oan Assigned To oan => msv
2019-09-12 11:18 oan Status assigned => feedback
2019-09-12 11:46 msv Note Added: 0087020
2019-09-12 11:46 msv Assigned To msv => bugmaster
2019-09-12 11:46 msv Status feedback => reviewed
2019-09-12 18:47 bugmaster Note Added: 0087029
2019-09-12 18:47 bugmaster Status reviewed => tested
2019-09-12 18:48 bugmaster Test case number => Not required
2019-09-15 10:51 bugmaster Changeset attached => occt master 3068c3bb
2019-09-15 10:51 bugmaster Status tested => verified
2019-09-15 10:51 bugmaster Resolution open => fixed
2019-09-15 10:55 git Note Added: 0087098