OSD_Parallel_TBB: number of execution threads is strictly limited by the root scope

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
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.

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

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."

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.

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.

Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
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

Branch [archived branch] has been deleted by Participant.

[revision removed]