updated benchmarks? - Githubissues

ostnam / rust-wfa

MIT License

11 stars 1 forks source link

updated benchmarks? #11

Closed jguhlin closed 2 years ago

jguhlin commented 2 years ago

Wondering if you could update the benchmarks to see how it compares to wfa2-lib. Cheers

ostnam commented 2 years ago

Good idea, it doesn't make sense to bench against an implementation that isn't actively developed anymore.

ostnam commented 2 years ago

Done in bd96db0.

I ran the benchmarks as follows:

The benchmark cases (scores and sequences) are those hard-coded in bench/bench_wfa.rs.
For rust-wfa, I ran the benchmarks from the main crate, with the -b flag. The results differ significantly compared to running them with cargo bench, with the latter being much faster. I don't know why.
For wfa2-lib, I used the align_benchmark binary from the latest commit, with the same scores and the gap-affine-wfa or gap-affine-swg algorithms. The results listed are those provided by Time.Alignment.

Comparing the results for the longest, most different sequences, it's clear that my implementation can be significantly improved.

I'll let you close the issue unless you want to add something.

jguhlin commented 2 years ago

Hey, thanks for the really fast turnaround, it's appreciated. I got a speed boost on your library by using mimalloc (with secure mode off). I may poke around a bit more as I'd like to use this with some nanopore reads, so speeding up the longer sequences with high error rate is priority.

Cheers

Edit: Just thinking on it a library shouldn't set a memory allocator, but a binary should (and maybe benches, if possible). But it's still a good improvement.

ostnam commented 2 years ago

After reading your comment I tried using reserve when resizing the wavefront vectors, hoping to reduce the number of allocations performed. There was no clear cut benefit on benchmarks, with some increasing and others decreasing by a couple percents.

I also tried to initialize the vecs using Vec::with_capacity(), set to a large value which had pretty much the same result.

jguhlin commented 2 years ago

Strange. I hope I'm not bothering you too much, just trying to squeeze out some performance. I doubt I can do much though. You've got the algorithm pretty fast already. I'll close this for now but I'll open a new thread if I get anything new in.