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.

Sign up for GitHub

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

Jump to bottom

Migrate from tbb to onetbb #41

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Migrate from tbb to onetbb #41

wants to merge 4 commits into from

Conversation

lkeegan
Copy link

@lkeegan lkeegan commented Feb 9, 2023

  • add FlexbarAtomic
    • a simple wrapper around std::atomic which adds a copy constructor
    • to replace tbb::atomic member variables in classes with default copy constructors
  • replace removed tbb::atomic
    • with FlexbarAtomic if copy constructor was assumed
    • with std::atomic otherwise
  • filters (PairedAlign, PairedInput, PairedOutput)
    • no longer inherit from tbb::filter
    • take and return pointers to actual type instead of void*
    • operator() is now const
  • pipeline
    • use parallel_pipeline and make_filter
  • use global_control to set max threads
  • use oneapi::tbb namespace
  • add find_package for TBB to CMakeLists.txt

@lkeegan
Migrate from tbb to onetbb
195a1ab 
- add `FlexbarAtomic`
  - a simple wrapper around `std::atomic` which adds a copy constructor
  - to replace `tbb::atomic` member variables in classes with default copy constructors
- replace removed `tbb::atomic`
  - with `FlexbarAtomic` if copy constructor was assumed
  - with `std::atomic` otherwise
- filters (PairedAlign, PairedInput, PairedOutput)
  - no longer inherit from tbb::filter
  - take and return pointers to actual type instead of void*
  - operator() is now const
- pipeline
  - use parallel_pipeline and make_filter
- use global_control to set max threads
- use `oneapi::tbb` namespace
- add find_package for TBB to CMakeLists.txt
@lkeegan lkeegan mentioned this pull request Feb 9, 2023
Open
@tillea
Copy link

tillea commented Feb 10, 2023

I've tested this patch with the Debian package and confirm flexbar builds that way. Unfortunately the test seems to end in some endless loop:

 cd test ; \
 export PATH="/build/flexbar-3.5.0/obj-x86_64-linux-gnu:$PATH" ; \
 echo $PATH ; \
 export HOME=/tmp ; \
 which flexbar ; \
 ./flexbar_test.sh
 /build/flexbar-3.5.0/obj-x86_64-linux-gnu:/usr/sbin:/usr/bin:/sbin:/bin
 /build/flexbar-3.5.0/obj-x86_64-linux-gnu/flexbar

 Testing fasta:

Hopefully someone will be able to fire up gdb and find a fix.
Kind regards, Andreas.

@lkeegan
Fix parallel_pipeline issues
1c872fa 
- pass a lambda with a reference to the filter instead of directly passing the filter to parallel_pipeline
  - passed filters may be copied and/or deleted by tbb
  - flexbar continues to use the filter objects after the pipeline is finished
- call fc.stop() in PairedInput when there is no more input
  - required control flow for first filter in parallel_pipeline that was previously missing
@lkeegan
Copy link
Author

lkeegan commented Feb 10, 2023

@tillea thanks for testing - I didn't notice the tests folder!
1c872fa10 should fix the endless loop and another issue that came up with the pipeline changes, and now the tests pass for me

lkeegan added a commit to lkeegan/bioconda-recipes that referenced this pull request Feb 10, 2023
@lkeegan
fix onetbb flexbar issue
0173d33 
- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
lkeegan added a commit to lkeegan/bioconda-recipes that referenced this pull request Feb 10, 2023
@lkeegan
fix onetbb flexbar issue
826165f 
- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
lkeegan added a commit to lkeegan/bioconda-recipes that referenced this pull request Feb 10, 2023
@lkeegan
fix onetbb flexbar issue
3f17ef7 
- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
lkeegan added a commit to lkeegan/bioconda-recipes that referenced this pull request Feb 10, 2023
@lkeegan
fix onetbb flexbar issue
24a5578 
- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
@lkeegan lkeegan mentioned this pull request Feb 10, 2023
Merged
@tillea
Copy link

tillea commented Feb 10, 2023

This works for me now and I'll upload the Debian package with these patches. Thanks a lot, Andreas.

BiocondaBot pushed a commit to bioconda/bioconda-recipes that referenced this pull request Feb 11, 2023
@lkeegan
[ci skip] Merge PR 39296
90560d0 
Merge PR #39296, commits were: 
 * fix onetbb flexbar issue

- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
cokelaer pushed a commit to cokelaer/bioconda-recipes that referenced this pull request Apr 28, 2023
@lkeegan @cokelaer
[ci skip] Merge PR 39296
371575e 
Merge PR bioconda#39296, commits were: 
 * fix onetbb flexbar issue

- add patch with onetbb support
  - migrates flexbar from tbb to onetbb to allow building against current conda-forge tbb version
  - upstream PR for these changes: seqan/flexbar#41 (but repo doesn't appear to be maintained)
- remove flexbar from build blacklist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lkeegan @tillea