Improving performance of Occ::get, adding FMIndex benchmark #74

Merged
merged 2 commits into from Jul 23, 2016

Conversation

Projects
None yet
2 participants
@anp
Contributor

anp commented Jul 22, 2016

In the process of profiling my application, I noticed quite a lot of time spent in Occ::get, so I decided to add a new benchmark and see if I could further optimize that function. The new benchmark uses the FM index to look for exact matches of 20-mers in the text (similar to exact-first-fuzzy-later read mappers). I found in profiling that the optimizer did not do well with the map/fold approach for counting characters in the BWT, so I tried an approach with filter/count.

Bench time of map/fold on my machine:

$ cargo bench search
...
     Running target/release/fmindex-3d950d5fe462c0d1

running 1 test
test search_index_seeds ... bench:      52,239 ns/iter (+/- 403)

Bench time of filter/count on my machine:

$ cargo bench search
...
     Running target/release/fmindex-3d950d5fe462c0d1

running 1 test
test search_index_seeds ... bench:      34,485 ns/iter (+/- 947)

In this microbenchmark, this provides ~34% improved runtime speeds, and I've noticed a similar speedup in the benchmarks for my application.

I also tried versions with manually counting items in a loop, and while the bench time of those was consistently within the margin of error of the filter/count version, the average time across runs was just a touch higher. Looking at the disassembly, the bounds checks add a few percentage points of overhead when manually counting the items, and using unsafe get_unchecked completely destroys the optimizer. It may be worth revisiting this optimization (and others) whenever the new MIR trans is enabled for stable Rust, but for now I think this should provide a decent speed boost to users of Occ.

@johanneskoester

This comment has been minimized.

Show comment
Hide comment
@johanneskoester

johanneskoester Jul 23, 2016

Contributor

Good catch!! Thanks!

Contributor

johanneskoester commented Jul 23, 2016

Good catch!! Thanks!

@johanneskoester johanneskoester merged commit 524d10f into rust-bio:master Jul 23, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment