-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
warn on tbb::global_control interferences #6814
Conversation
The current solution might be too noisy in some contexts, for instance RDataFrame, where "every MT event loop should be exactly two calls to Foreach but you might have a bunch of event loops per application" (@eguiraud 📝 ). I'll give it another look. |
f113e4f
to
e25fc2b
Compare
Starting build on |
e25fc2b
to
78a2cba
Compare
Starting build on |
Hah, I thought this one was merged already. I need to add another test case in the threadExecutor test in roottest to account for this |
Build failed on ROOT-debian10-i386/cxx14. Warnings:
|
Build failed on mac11.0/cxx17. Failing tests: |
78a2cba
to
7c21c51
Compare
Starting build on |
Build failed on ROOT-fedora30/cxx14. Failing tests: |
Build failed on ROOT-debian10-i386/cxx14. Warnings:
|
Build failed on mac11.0/cxx17. Failing tests: |
Starting build on |
Build failed on ROOT-debian10-i386/cxx14. Warnings:
|
77d8cc8
to
714f75b
Compare
Starting build on |
714f75b
to
ee2c8c2
Compare
Starting build on |
ee2c8c2
to
f1cc84f
Compare
Starting build on |
@phsft-bot build please |
Starting build on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Xavi! The comment about false positives is the only required change.
"tbb::global_control is limiting the number of parallel workers." | ||
" Proceeding with %zu threads this time", | ||
tbb::global_control::active_value(tbb::global_control::max_allowed_parallelism)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic repeated 3 times could be factored out in a little helper function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these three times in such a short file, I feel creating a helper to print would be overkill, and creating a helper that includes the call to GetPoolSize() would complicate the code.
Build failed on mac11.0/cxx17. Failing tests: |
warn when the number of threads set by the user is limited at runtime by tbb::global_control. Fix for github issue root-project#6363: root-project#6363
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
f1cc84f
to
e7c1d1d
Compare
Starting build on |
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
warn when the number of threads set by the user is limited at runtime
by tbb::global_control
Addressing #6363