oneTBB: port dispatcher, thread limits and add build option #2466
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This now modifies global_control::max_allowed_parallelism instead of task_scheduler_init, which leads to changes in API behavior. * Increasing number of threads beyond the number of cores now additionally requires creating a task_arena with higher max concurrency. * All application threads are affected when setting the concurrency limit, not just the current thread. * The winning call to set the number of threads may now be different due to global_control always changing the number, while with task_scheduler_init the first created instance determines the number of threads. Also, in the existing implementation task_scheduler_init is never freed, not on shutdown or on changing the concurrently limit back to the default. This seems unideal, but the new code does the same to keep the same behavior.
To make concurrent wait thread safe this is accessing the internals of TBB, since the TBB wait implementation has a comment saying it is not thread safe.
And update FindTBB to support it. This also changes OpenVDB 7 to 10 when using oneTBB since earlier versions do not support it.
|
Filed as internal issue #USD-8384 |
2 tasks
loqs
reviewed
Jun 23, 2023
1 task
|
@brechtvl I noticed this pull request now has merge conflicts. |
7 tasks
pixar-oss
added a commit
that referenced
this pull request
May 29, 2024
…support for oneTBB Fixes #2466 (Internal change: 2328321)
pixar-oss
added a commit
that referenced
this pull request
May 29, 2024
… for imposing const on dispatcher lambas for support of oneTBB Fixes #2466 (Internal change: 2328322)
pixar-oss
added a commit
that referenced
this pull request
May 29, 2024
…on for adding suppoort for oneTBB. Fixes #2466 (Internal change: 2328323)
|
This merge left out the build system changes. But I guess that was intentional, and there are more changes coming before it's considered ready. |
|
That's right - we're landing the last few changes now, @brechtvl - thanks so much for your giant contributions, here! |
pixar-oss
pushed a commit
that referenced
this pull request
Jun 11, 2024
See #2466 (Internal change: 2329967)
pixar-oss
pushed a commit
that referenced
this pull request
Jul 23, 2024
This is based on changes in PR #2466 from @brechtvl. However, there are some notable differences: - The script will error out of Embree and oneTBB are both requested, since the version of Embree currently used is incompatible with oneTBB. - The original PR bumped OpenVDB to v10.0.1 if oneTBB was requested, stating that this was the first version to support oneTBB. However, this doesn't appear to be accurate; the current version (v9.1.0) builds successfully with oneTBB. v10.0.1 has a fix for a build issue with TBB 2021.5 (0a9e2177 in the OpenVDB repo) but I'm not sure why it's needed. On Windows, TBB 2021.5 does install its .dll with a version suffix, e.g. tbb12.dll instead of tbb.dll. However, it also installs the corresponding .lib files without the version suffix, which should cause the MSVC linker to pick it up correctly. In my testing, this does work as expected. I tested builds with OpenVDB v9.1.0 and oneTBB on Linux/MacOS/Window and didn't run into any issues. So I'm opting to leave that in place for now. (Internal change: 2334872)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change(s)
After the various simple changes submitted as separate pull requests, this actually does the bulk of the work of adding TBB 2021 support. It is optional, older TBB version should still work exactly the same.
build_usd.pyoption--onetbb, disabled by default.The impact of the thread limits changes in complex applications like DCCs is not easy to understand. One would hope that such applications do not use the USD API or environment variables to set thread limits.
If there are multiple parts of the application using and configuring TBB thread limits, here are some potential differences when using oneTBB. This is from my understanding of the docs and code, not testing in practice.
With older TBB, the first created
task_scheduler_initwins and determines the number of threads. USD can create one as part of static initialization, which can happen before a host application creates one, and so override the number of threads for the entire application. With oneTBB, everyglobal_controlcall changes the number of threads instead of just the first.With older TBB,
task_scheduler_initaffects one thread spawing tasks. It may commonly be used early on in the main thread and affect the entire application, but not necessarily. With oneTBB,global_controlalways affects the entire application regardless of the thread it is called from.After these changes, the thread limit API can still be used to decrease the number of threads used by TBB. But it can no longer increase them beyond the number of cores just by itself. To increase the number of threads, the parallel code now needs to be run inside a task arena with increased max concurrency as well. I could not find a mechanism in oneTBB that allows changing the max concurrency of the current existing task arena.
Fixes Issue(s)
Part of #1471, adding oneTBB support.