Using Aligner instance with rayon threads

wdecoster commented 1 year ago

Hi,

Great work here - finally had the chance to start testing. I haven't been using Rust for long enough to fully understand the problem I'm about to describe. In my setup I create an Aligner instance, and then use rayon to test reads from a fastq file in parallel. I would prefer to create the Aligner only once, and then have it be used in each thread, but the compiler tells me:

error[E0277]: `*mut minimap2_sys::mm_idx_t` cannot be sent between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::mm_idx_t` cannot be sent between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Send` is not implemented for `*mut minimap2_sys::mm_idx_t`

Implementing Send goes beyond what I can do in Rust (for now). Is this a problem that is easy to solve or would there be a better way around this?

jguhlin commented 1 year ago

Hey, apologies for the delay. I'm having my first battle with covid (on the upswing, but tired really easily still!). I'll futz around with this today, but I have a feeling Rayon will be harder to get to work compared to other multithreading options. Right now, none work though!

wdecoster commented 1 year ago

Please don't apologize, take care of yourself!

jguhlin commented 1 year ago

Ok, try release 0.1.8.

If it works, let me know! Otherwise, you can follow along with fakeminimap's implementation of threads: https://github.com/jguhlin/minimap2-rs/blob/main/fakeminimap2/src/main.rs#L62

or we can try to find an easier way. Cheers. :)

I suspect it's going to leak a bit of memory still, so watch out for that. It's on my todo list for when I'm feeling a bit better.

wdecoster commented 1 year ago

Thanks for your efforts! This is a summary of 8 errors obtained after cargo build:

    |          `*const i8` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*const i8`
    = note: required because it appears within the type `minimap2_sys::mm_mapopt_t`

 `*mut minimap2_sys::mm_idx_seq_t` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_seq_t`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`

`*mut u32` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut u32`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`

*mut minimap2_sys::mm_idx_bucket_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_bucket_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`

`*mut minimap2_sys::mm_idx_intv_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_intv_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`

`*mut c_void` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut c_void`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`

`*mut minimap2_sys::_IO_FILE` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::_IO_FILE`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_reader_t>`

 |
    |          `*mut minimap2_sys::mm_bseq_file_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_bseq_file_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t__bindgen_ty_1`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_reader_t>`

I will have a look at fakeminimap then - rayon is just very convenient :-)

Wouter

jguhlin commented 1 year ago

Some of those errors should be fixed in the newest version, can you double check? I switched away from sharing raw pointers (*mut etc...) to using the structs directly.

wdecoster commented 1 year ago

Hmm then I don't think I understand what is going on, see full error below, using v0.1.8:

   Compiling minimap2-sys v0.1.6
    Checking minimap2 v0.1.8
    Checking nanofilt v0.1.0 (/home/wdecoster/wsl-repos/nanofiltrs)
error[E0277]: `*const i8` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*const i8` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*const i8`
    = note: required because it appears within the type `minimap2_sys::mm_mapopt_t`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut minimap2_sys::mm_idx_seq_t` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::mm_idx_seq_t` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_seq_t`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut u32` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut u32` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut u32`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut minimap2_sys::mm_idx_bucket_s` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::mm_idx_bucket_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_bucket_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut minimap2_sys::mm_idx_intv_s` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::mm_idx_intv_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_idx_intv_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut c_void` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut c_void` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut c_void`
    = note: required because it appears within the type `minimap2_sys::mm_idx_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut minimap2_sys::_IO_FILE` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::_IO_FILE` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::_IO_FILE`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_reader_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

error[E0277]: `*mut minimap2_sys::mm_bseq_file_s` cannot be shared between threads safely
   --> src/main.rs:71:10
    |
71  |         .for_each(|record| {
    |          ^^^^^^^^ -------- within this `[closure@src/main.rs:71:19: 71:27]`
    |          |
    |          `*mut minimap2_sys::mm_bseq_file_s` cannot be shared between threads safely
    |
    = help: within `[closure@src/main.rs:71:19: 71:27]`, the trait `Sync` is not implemented for `*mut minimap2_sys::mm_bseq_file_s`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t__bindgen_ty_1`
    = note: required because it appears within the type `minimap2_sys::mm_idx_reader_t`
    = note: required because it appears within the type `Option<minimap2_sys::mm_idx_reader_t>`
    = note: required because it appears within the type `minimap2::Aligner`
    = note: required because it appears within the type `Option<minimap2::Aligner>`
note: required because it's used within this closure
   --> src/main.rs:71:19
    |
71  |         .for_each(|record| {
    |                   ^^^^^^^^
note: required by a bound in `rayon::iter::ParallelIterator::for_each`
   --> /home/wdecoster/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-1.5.3/src/iter/mod.rs:367:30
    |
367 |         OP: Fn(Self::Item) + Sync + Send,
    |                              ^^^^ required by this bound in `rayon::iter::ParallelIterator::for_each`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `nanofilt` due to 8 previous errors

jguhlin commented 1 year ago

Thanks for that. Nah, it's my fault. I thought I got rid of all of those types of raw pointers, but clearly not enough for rayon! Sorry for the distraction, but this is helpful. But yeah, you were right, take a look at fakeminimap and let me know if you have any trouble implementing that.

jguhlin commented 1 year ago

Closing as this is now represented in #5

jguhlin / minimap2-rs

Using Aligner instance with rayon threads #2