bytes icon indicating copy to clipboard operation
bytes copied to clipboard

Add specialized get_X impls for Bytes struct to reduce dead code

Open Walnut356 opened this issue 1 year ago • 3 comments

Without this change, there's quite a bit of dead code generated when using these functions with a Bytes object. This is because:

  1. These functions are nearly always inlined in their entirety (including the call to self.copy_to_slice)

  2. The entire conditional:

        if let Some(ret) = ret {
            // if the direct conversion was possible, advance and return
            $this.advance(SIZE);
            return ret;
        } else {
            // if not we copy the bytes in a temp buffer then convert
            let mut buf = [0; SIZE];
            $this.copy_to_slice(&mut buf); // (do the advance)
            return $typ::$conv(buf);
        }

seems to be meant to handle non-contiguous memory. Bytes is guaranteed to be contiguous, so whenever the else block is triggered it immediately panics on self.copy_to_slice's first assert statement: assert!(self.remaining() >= dst.len());

Due to the above, the generated assembly for multiple sequential bytes.get_x calls is pretty messy and it doesn't look like the compiler is having a fun time with it. This simple change removes the conditional and just has a raw panic on a None being returned from slice.get, cleaning up the generated code a lot and allowing for a lot more optimizations (from what i observed).

I also copied the Buf get_u16 tests over to the bytes tests to confirm the behavior still works as expected.

Walnut356 avatar Aug 16 '23 13:08 Walnut356