switch BGZFStreams for BGZFLib - #81
Conversation
|
This is very exciting! I'll look at it one of the following days when I get time. Some changes might need to be upstreamed to BufferIO.jl or BGZFLib.jl (I'll handle that). I would prefer the PR is split into one which solely switches the BGZF library. |
|
Done, it was easy to separate. I added these to try to reduce allocations to a minimum, I think the BGZFLib version allocates a bit more than BGZFStreams. |
| matrix: | ||
| julia-version: | ||
| - '1.6' # LTS | ||
| - '1.11' # lowest compatible release |
There was a problem hiding this comment.
Not ideal to lose the current LTS, but I don't think it's a dealbreaker
There was a problem hiding this comment.
I think it's MemoryViews that requires 1.11
|
Let me know if you need any help with the PR or implementation |
|
I did another pass on this, benchmark still looks good, it allocates more but it's because I'm using the sequence and qualities : 12.025843 seconds (196.71 M allocations: 18.897 GiB, 24.67% gc time, 1953 lock conflicts, 0.26% compilation time) |
jakobnissen
left a comment
There was a problem hiding this comment.
I think you're almost done! Just two more bugs I found - then it can be merged.
|
Thank you! |
I had some time so I've tried to replace BGZFStreams the new libdefalte based BGZFLib. I used Claude for this but it was struggling quite a bit so I also had to contribute as well... The code look ok I think and all tests pass. There's maybe some rough edges (e.g.
_to_virtual_offsetshould be a method in BGZFLib?) but maybe that can be tackled later on, using this PR as a base.I also added to methods for allocation free sequence and quality extraction.
The performance are pretty good, computing the mean base quality on a 3.0G BAM file, 3x faster, same result :
Before :
After :