View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0030959 | Open CASCADE | OCCT:Foundation Classes | public | 2019-09-11 11:02 | 2019-09-15 10:55 |
Reporter | oan | Assigned To | bugmaster | ||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
Summary | 0030959: OSD_Parallel_TBB: number of execution threads is strictly limited by the root scope | ||||
Description | In 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 Reproduce | Not required | ||||
Tags | No tags attached. | ||||
Test case number | Not required | ||||
|
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. |
|
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 : 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 |
|
Branch CR30959 has been deleted by inv. SHA-1: 497e7d52eb77817a2dcd99877442ad5744a7e5aa |
occt: master 3068c3bb 2019-09-11 08:04:03 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 |
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 |
|
Note Added: 0087014 | |
2019-09-11 18:39 |
|
Assigned To | msv => oan |
2019-09-11 18:39 |
|
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 |
|
Note Added: 0087020 | |
2019-09-12 11:46 |
|
Assigned To | msv => bugmaster |
2019-09-12 11:46 |
|
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 |