Better way than using from_be_bytes?

code review
#1

So here we're doing a common operation - taking a u32 from 4 bytes, big-endian. This is from a program that analyzes "tcpdump" files, so it has a lot of data to look at. Turning 4 bytes from a vector of bytes into a u32 is supported, but the code is kind of clunky.

fn processacks(&self, acks: &[u8]) -> Result<(),&str> {
    for i in (0..acks.len()).step_by(std::mem::size_of::<u32>()) {
        let seqnum = &acks[i .. i+4];                           // next sequence number
        /// In an ACK, sequence numbers also are big-endian. 
        let sequence = u32::from_be_bytes(seqnum.try_into().unwrap());  // panic here should be impossible since previous statement sets size as 4.
  self.processack(sequence);                              // ACK processor
}

Is there a better way? I'm going to have about a thousand statements like that in machine-generated code that's taking apart packets, so I want that operation to be fast.

Is the optimizer smart enough to detect that a byte array created in the previous statement with [i ++ i+4] has to have a length of 4 and avoid generating the panic?

#2

The bytes crate provides Buf::get_u32 which would do the job of incrementing the index in the source array for you. You can then just iterate over Buf::has_remaining had read all numbers.

Up to you whether that is better or not what you have.
For bounds checks - best check the compiler output (e.g. via godbolt). Things change between compiler versions. It might help the compiler if you assert the maximum length of the array upfront.

#3

What do you want if the acks has a size not a multiple of size_of::<u32>()? This code will panic on let seqnum = &acks[i..i+4]; if the acks's length is, say, 7.

If you're ok to drop excess bytes shorter than it, it would be simpler.

fn process_acks(&self, acks: &[u8]) -> Result<(), &str> {
    use std::convert::TryInto;

    for sequence in acks.chunks_exact(std::mem::size_of::<u32>()) {
        self.process_ack(u32::from_be_bytes(sequence.try_into().unwrap()));
    }
}

And yes, the compiler can optimize out those obvious bound checks.

1 Like
#4

And for well-known array lengths, we'll eventually get methods that just give &[u8; 4] directly, be that https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.array_chunks or https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.as_chunks

#5

Thanks. I've been off trying this in Godbolt. It's amusing. With optimization off, the code is terrible and there seems to be a function call for each subscript check. With optimization on (-O), everything gets inlined and there are no visible checks. As I understand it, Rust does not turn off subscript checks for safe code. So I assume it optimized those out, as it should.

#6

Yes, rust code with optimizations off is notoriously bad. There's a reason that it's not uncommon for optimized code to be 50x faster.

Correct. Safe code is always safe; there's no way to turn those checks off. (There are different unsafe things that don't have checks, but there's no way to turn off the checks in the safe APIs.)