reverse_complement: Do line wrapping and reverse complement in one step

[mbrubeck]

This combines newline handling and the main "reverse complement" step into a single loop. The downside of this is that it can no longer treat the input as u16 in order to transform two bytes with one table lookup. The advantages are faster speed from making a single pass over the data, and no more unsafe code!

This runs about 18% faster than the master branch on my computer.

[mbrubeck]

Pushed some minor stylistic fixes.

[mbrubeck]

As an extra bonus, the sequence of operations in this version more closely resembles the C program, for anyone who wants to do side-by-side comparisons of the two.

[mbrubeck]

We could reintroduce the u16-based lookup inside this function... I'll try this and see what impact it has.

[mbrubeck]

I tried this and it did not change performance at all. The code is at mbrubeck@e133026.

[TeXitoi]

I like it.

I think I can write a comment with Ascii art to explain the loop, that would help the understanding. I'll try to write it today.

[cristicbz]

This is brilliant, I'd love to see it merged!

[TeXitoi]

I can't find a better explaination that what is already done. As Isaac closed the previous submission, do you prefer I submit or you submit?

And maybe we should wait some days on a bench without update before submitting it. That's too much update of the same bench for Isaac ;-)

[cristicbz]

FWIW, I think you could write this as *y = tables.cpl8(mem::replace(x, tables.cpl8(*y)))

[mbrubeck]

Done.

[cristicbz]

Do you think it'd be worth making this some multiple of 61?

[mbrubeck]

Do you think it'd be worth making this some multiple of 61?

I don't think it would make any noticeable difference.

[cristicbz]

What do you think about removing this struct and just having a table: &[u8; 256] passed around and a

fn build_table() -> [u8; 256] {
    let mut table = [0u8; 256];
    for (i, v) in table8.iter_mut().enumerate() {
        *v = match i as u8 {
             b'A' | b'a' => b'T',
             b'C' | b'c' => b'G',
             b'G' | b'g' => b'C',
             b'T' | b't' => b'A',
             b'U' | b'u' => b'A',
             b'M' | b'm' => b'K',
             b'R' | b'r' => b'Y',
             b'W' | b'w' => b'W',
             b'S' | b's' => b'S',
             b'Y' | b'y' => b'R',
             b'K' | b'k' => b'M',
             b'V' | b'v' => b'B',
             b'H' | b'h' => b'D',
             b'D' | b'd' => b'H',
             b'B' | b'b' => b'V',
             b'N' | b'n' => b'N',
             i => i,
         };
    }
}

called once in main. Or we could even go one better and do what C does:

const TABLE: &'static [u8] =
    b"                                                                 \
       TVGH  CD  M KN   YSAABW R       TVGH  CD  M KN   YSAABW R";

[TeXitoi]

The last one must generate bound check, so better having a [u8; 256] to elide bound checks.

[cristicbz]

That's true: you could fill it up all the way to 256 and do const TABLE: &[u8; 256] it's only 4 lines 64 chars. But yeah it's a bit silly.

[mbrubeck]

I used the fn build_table() -> [u8; 256] approach, because I found it the most readable.

[mbrubeck]

As Isaac closed the previous submission, do you prefer I submit or you submit?

I'll create a new submission for this after it's stabilized for at least a couple of days.

[mbrubeck]

I haven't come up with any new improvements and am no longer actively working on this benchmark, so I'll submit it later today if there are no other comments or changes posted.

[TeXitoi]

OK for me, I'll merge when you provide the link to the submission.

[mbrubeck]

Pushed my final version, which contains comment edits and a few minor style changes since the previous push.

[mbrubeck]

https://alioth.debian.org/tracker/index.php?func=detail&aid=315641&group_id=100815&atid=413122