jguhlin / minimap2-rs

Rust bindings to minimap2 library
Other
60 stars 13 forks source link

Multi-threaded implementation #4

Closed Adoni5 closed 1 year ago

Adoni5 commented 1 year ago

Hi Joseph,

Thanks for the great work on this crate!

Just like #2, I am hitting the issue of Send/Sync not being implemented on the bindgen generated Structs which are fields on the Aligner. Unlike in #2, I was attempted to create a simple multi threaded example using the ThreadPool crate.

Simply something along the lines of:

for seq in seqs {
    pool.execute(|| {
        // Channel transmitter
        let tx = tx.clone()
        // The map function is altered to transmit the mapping back at the end instead of returning it
        self.map(
            tx,
            seq.as_bytes(),
            self.idx
            ....
        )
    })
}

pool.join()
re.try_iter().collect()

I have tried to mark mm_idx_t as Sync/Send - as you can't implement traits for Structs defined in other crates, I created a Type struct that wraps the fields for mm_idx_t, however I get no mappings back. I'm going to have another crack at this this afternoon and will get back to you here if there is any progress to report!

Many Thanks, Rory

jguhlin commented 1 year ago

I'm futzing around this morning too, will let you know.

I did impl Send for them, but the raw pointers themselves aren't safe, so having to change some code.

jguhlin commented 1 year ago

Send is working, I get some output back, then it crashes. Will work on debugging that next.

fish: “./target/debug/fakeminimap2 /mn…” terminated by signal SIGSEGV (Address boundary error)
jguhlin commented 1 year ago

Try it with version 0.1.8 now on crates. I believe it will work for your use case, if not I've implemented my own thread pool method here, you can follow it:

https://github.com/jguhlin/minimap2-rs/blob/main/fakeminimap2/src/main.rs#L62

If you need to alter the map function, let me know if it'd be easier / better to return a different type.

I suspect it's leaking memory, but haven't had a chance to run it long enough to prove or disprove that yet.

Adoni5 commented 1 year ago

Dude, heroic effort - definitely take it easy it until you are feeling better! I'm free in about an hour - will report back, very excited to see if this works

Adoni5 commented 1 year ago

@jguhlin - sorry for the slow reply, been very busy! I'm closing this as implementing a similar to fakeminimap approach worked excellently. I'm writing some tests and will maybe try my hand at memory profiling!

Thanks again!