quinlan-lab/bedder-rs

replace Box<dyn> with enum

brentp opened this issue · 1 comments

brentp commented

We take a substantial performance hit from Boxing everything. We must Box because we are passing around Traits (VCF::Variant, Bam::Record, etc). This means that the dynamic dispatch is done twice since the Box dispatches to the underlying Trait which dispatches to the concrete struct.
https://github.com/brentp/bedder-rs/blob/d051b6520401caddc81a05364838c9065dcb8ee5/src/position.rs#L58-L75

We can use this crate: https://crates.io/crates/enum_dispatch to get static dispatch but we must provide the enum anyway.

Using enum_dispatch or the actual enum would mean that a lib user could not add a new file type (PositionedIterator) without changing the core library--updating the main enum and all code that matches on that enum. (Unless the new filed type used an existing Positioned type)

We can also implement as enums directly. More info on the problem/setup here:
https://bennett.dev/dont-use-boxed-trait-objects-for-struct-internals/

If we do have the enum, a downside is that every Positioned will take the space of the largest item in the Enum.
This is probably OK as memory will only be a problem for huuuge query intervals and we are using Rc, not copying; but it is still a consideration.

The example code shows how this appears when using the API: https://github.com/brentp/bedder-rs/blob/4f106838fb3048430c35796b641cfbc1afd48054/examples/intersect_bed_count.rs#L43-L58

brentp commented

This is implemented in main