Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop deprecated TBB components #6590

Merged
merged 1 commit into from
Jun 23, 2021
Merged

Conversation

zavorka
Copy link
Contributor

@zavorka zavorka commented Jun 3, 2021

Quite some time ago, many of the TBB components were deprecated in favor of their near-equivalents in the STL or, in the case of task_scheduler_init, were broken up and reconstituted under a less ad-hoc logic. Every time a header file marked deprecated gets included, a rather loud warning is emitted, which leads to a complete TBB's domination over the stderr stream during build time, making it harder to notice legitimate warnings.

Instead of merely muting the output with TBB_SUPPRESS_DEPRECATED_MESSAGES, perform a genuine migration away from the deprecated components with the added benefit of achieving source compatibility with oneTBB, the successor to TBB, which has dropped the deprecated API for good.


Applied mapping

Deprecated Replacement
tbb::atomic std::atomic
tbb::mutex std::mutex
tbb::mutex::scoped_lock std::scoped_lock<std::mutex>
tbb::mutex::scoped_lock (empty) std::unique_lock<std::mutex> (deferred)
tbb::task_scheduler_init tbb::global_control
tbb::this_thread std::this_thread

UPDATE

Unbeknownst to me, people at Intel released have compiled their own comprehensive replacements table.


Signed-off-by: Roman Beranek [email protected]

Quite some time ago, many of the TBB components were deprecated in favor
of their near-equivalents in the STL or, in the case of task_scheduler_init,
were broken up and reconstituted under a less ad-hoc logic. Every time a header
file marked deprecated gets included, a rather loud warning is emitted, which
leads to a complete TBB's domination over the stderr stream during build time,
making it harder to notice _legitimate_ warnings.

Instead of merely muting the output with TBB_SUPPRESS_DEPRECATED_MESSAGES,
perform a genuine migration away from the deprecated components with the added
benefit of achieving a source compatibility with oneTBB, the successor to TBB
which has dropped the deprecated API for good.

What got replaced for what?

| Deprecated				| Replacement					|
| ------------------------------------- | --------------------------------------------- |
| `tbb::atomic`				| `std::atomic`					|
| `tbb::mutex`				| `std::mutex`					|
| `tbb::mutex::scoped_lock`		| `std::scoped_lock<std::mutex>`		|
| `tbb::mutex::scoped_lock` (empty)	| `std::unique_lock<std::mutex>` (deferred)	|
| `tbb::task_scheduler_init`		| `tbb::global_control`				|
| `tbb::this_thread`			| `std::this_thread`				|

Signed-off-by: Roman Beranek <[email protected]>
@bubnikv
Copy link
Collaborator

bubnikv commented Jun 4, 2021 via email

@zavorka
Copy link
Contributor Author

zavorka commented Jun 4, 2021

Wouldn't it be weird if a C++ standard library implementation shipped with threading primitives of a suboptimal performance? (Not for Microsoft, I guess.) Moreover, these interfaces were deprecated in the TBB once they had literally become a part of the STL (with some little changes here and there) in 2011.

However, both of the links point to somewhat dated content. Nowadays, it seems like, on Windows 10 with a recent-enough (2017+) STL, std::mutex should actually perform better in the high contention conditions than tbb::mutex as Microsoft's std::mutex now uses SRWLock [1], while the CRITICAL_SECTION Win32 API now itself by default does some spinning[2]. (Possibly to improve latency? A reminder that from the user's perspective, some overhead might be good, actually.)

And as far as other platforms are concerned: On POSIX systems the Thread support library wraps around POSIX Threads and it's been this way basically since around forever (i.e. the first draft from 2007 [3]).

[1] https://github.com/microsoft/STL/blob/main/stl/src/primitives.hpp#L124
[2] https://stackoverflow.com/q/52170665/3763673
[3] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2178.html

@bubnikv
Copy link
Collaborator

bubnikv commented Jun 4, 2021 via email

@bubnikv bubnikv merged commit e13535f into prusa3d:master Jun 23, 2021
@bubnikv
Copy link
Collaborator

bubnikv commented Jun 23, 2021

Trusted, but verified.

Thanks for your contribution.

@ssill2
Copy link

ssill2 commented Jun 23, 2021

This may have broken windows build. I just cleared all my deps/build and the main build dirs and regenerated. All deps built fine and the cmake to build .sln files worked fine.

A lot of errors around mutex
image

@bubnikv
Copy link
Collaborator

bubnikv commented Jun 23, 2021

It should be fine now.

@ssill2
Copy link

ssill2 commented Jun 23, 2021

thansk, I'll sync up and try again!

@ssill2
Copy link

ssill2 commented Jun 23, 2021

That did it thanks!

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Nov 24, 2021
https://build.opensuse.org/request/show/933584
by user bnavigator + dimstar_suse
- Make compatible with TBB 2021
  * add FindTBB.cmake -- gh#prusa3d/PrusaSlicer#7332
  * add PrusaSlicer-pr6590-updateTBB.patch
    gh#prusa3d/PrusaSlicer#6590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants