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

feat: don't use TBB in Sequence(numThreads=1) #1444

Merged
merged 11 commits into from
Aug 19, 2022

Conversation

timadye
Copy link
Contributor

@timadye timadye commented Aug 17, 2022

  • Let's the Sequencer bypass the Intel TBB library if numThreads=1.
    • This could simplify debugging.
  • Uses some wrapper functions for most of the TBB functions that we use in Sequencer.
    • Note that only a small subset of TBB functions are implemented, and tbb::blocked_range (which doesn't require any thread setup) is still taken from the TBB library.
  • However, if NO_TBB is defined, then don't use TBB library at all (throws an exception unless numThreads=1 or -1).
    • This should allow the ACTS Examples to be built without the TBB library (and removes one dependency on ROOT).

At the moment, this doesn't have any CMake support for disabling linking with the TBB library. If someone thinks this might be useful, please let me know - or add it to this PR!

@timadye timadye added this to the next milestone Aug 17, 2022
@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #1444 (ebb044d) into main (48f2993) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1444   +/-   ##
=======================================
  Coverage   48.59%   48.59%           
=======================================
  Files         380      380           
  Lines       20589    20589           
  Branches     9431     9431           
=======================================
  Hits        10005    10005           
  Misses       4097     4097           
  Partials     6487     6487           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add one python test that runs the Sequencer in explicitly in single thread mode? I think there should be a few that do it "by accident" but I'd like to make sure that we have one test case that explicitly covers this.

@timadye
Copy link
Contributor Author

timadye commented Aug 19, 2022

Can you add one python test that runs the Sequencer in explicitly in single thread mode? I think there should be a few that do it "by accident" but I'd like to make sure that we have one test case that explicitly covers this.

I added test_sequencer_single_threaded and test_sequencer_multi_threaded.

@paulgessinger paulgessinger added automerge backport develop/v19.x Backport this PR to the v19.x series labels Aug 19, 2022
@kodiakhq kodiakhq bot merged commit 2171277 into acts-project:main Aug 19, 2022
acts-project-service pushed a commit that referenced this pull request Aug 19, 2022
* Let's the `Sequencer` bypass the Intel TBB library if `numThreads=1`.
  * This could simplify debugging.
* Uses some wrapper functions for most of the TBB functions that we use in `Sequencer`.
  * Note that only a small subset of TBB functions are implemented, and `tbb::blocked_range` (which doesn't require any thread setup) is still taken from the TBB library.
* However, if `NO_TBB` is defined, then don't use TBB library at all (throws an exception unless `numThreads=1` or `-1`).
  * This should allow the ACTS Examples to be built without the TBB library (and removes one dependency on ROOT).

At the moment, this doesn't have any CMake support for disabling linking with the TBB library. If someone thinks this might be useful, please let me know - or add it to this PR!

(cherry picked from commit 2171277)
kodiakhq bot pushed a commit that referenced this pull request Aug 22, 2022
…lop/v19.x] (#1449)

Backport 2171277 from #1444.
---
* Let's the `Sequencer` bypass the Intel TBB library if `numThreads=1`.
  * This could simplify debugging.
* Uses some wrapper functions for most of the TBB functions that we use in `Sequencer`.
  * Note that only a small subset of TBB functions are implemented, and `tbb::blocked_range` (which doesn't require any thread setup) is still taken from the TBB library.
* However, if `NO_TBB` is defined, then don't use TBB library at all (throws an exception unless `numThreads=1` or `-1`).
  * This should allow the ACTS Examples to be built without the TBB library (and removes one dependency on ROOT).

At the moment, this doesn't have any CMake support for disabling linking with the TBB library. If someone thinks this might be useful, please let me know - or add it to this PR!
@paulgessinger paulgessinger modified the milestones: next, v20.1.0 Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport develop/v19.x Backport this PR to the v19.x series
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants