Skip to content

Commit

Permalink
[FIX] make a bgzf_thread_count a single variable
Browse files Browse the repository at this point in the history
bgzf_thread_count was declared `inline static` outside of the scope of a
class.
In this case the `inline` allows the variable to break the one
definition rule. Meaning there can be multiple symbols across different
translation that all declare `bgzf_thread_count`. In this case the
linker will choose one of the defined symbols. (Without inline it would
throw a multiple definition error).

The `static` keyword (in this context) means that the created symbol is
only visible inside a current translation unit. Meaning, there are no symbols
for the linker to work with. This will cause every translation unit to have there
own `bgzf_thread_count` variable.

This combination leads to every translation unit having there own
`bgzf_thread_count` variable, which is not what a user of seqan3 expects.
The fix is simple, we remove the `static` keyword. This should have the
intended behavior.

Thought: maybe it should be declared `thread_local`.
  • Loading branch information
SGSSGene committed Jan 12, 2022
1 parent 0f5fb9d commit 1ec1bba
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ If possible, provide tooling that performs the changes, e.g. a shell-script.
#### I/O
* Changed default of `output_options::fasta_blank_before_id` to `false`
([\#2769](https://github.com/seqan/seqan3/pull/2769)).
* Changed default of `bgzf_thread_count` to `4` instead of determining the numbers of cores of the
machine it is running on ([\#2911](https://github.com/seqan/seqan3/pull/2911)).
* `bgzf_thread_count` is now a single global variable instead of having an instance per translation unit
([\#2752](https://github.com/seqan/seqan3/pull/2752)).

# 3.1.0

Expand Down
9 changes: 7 additions & 2 deletions doc/cookbook/compression_threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@
//![example]
#include <seqan3/io/all.hpp>


// The `bgzf_thread_count` is a variable that can only be changed during run time of the program.
// This does not work, the value must be overwritten within a function.
// seqan3::contrib::bgzf_thread_count = 1u;
// seqan3::contrib::bgzf_thread_count = 1u; // Doesn't work

int main()
{
// Use one thread for (de-)compression.
// Here we change the number of threads to `1`. This is a global
// change and will effect the every future bgzf decompression call
// which doesn't specify an explicit thread count parameter.
// Already running decompression calls are unaffected by this
seqan3::contrib::bgzf_thread_count = 1u;

// Read/Write compressed files.
Expand Down
2 changes: 1 addition & 1 deletion include/seqan3/contrib/stream/bgzf_stream_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace seqan3::contrib

/*!\brief A static variable indicating the number of threads to use for the bgzf-streams. Defaults to 4.
*/
[[maybe_unused]] inline static uint64_t bgzf_thread_count = 4;
[[maybe_unused]] inline uint64_t bgzf_thread_count = 4;

// ============================================================================
// Forwards
Expand Down

0 comments on commit 1ec1bba

Please sign in to comment.