Skip to content

Commit

Permalink
Add bytes specialization
Browse files Browse the repository at this point in the history
Remove Size in favor of a BitSize and ByteSize. This allows the
Deku{Read/Write}(Endian, BitSize) and Deku{Read/Write}(Endian, ByteSize)
impls to be created and allow the ByteSize impls to be faster. Most of
the assumption I made (and the perf that is gained) is from the removal
of this branch for bytes:

let value = if pad == 0
                      && bit_slice.len() == max_type_bits
                      && bit_slice.as_raw_slice().len() * 8 == max_type_bits
                  {

See sharksforarms#44

I Added some benchmarks, in order to get a good idea of what perf this
allows. The benchmarks look pretty good, but I'm on my old thinkpad.
These results are vs the benchmarks running from master.

deku_read_vec           time:   [744.39 ns 751.13 ns 759.58 ns]
                        change: [-23.681% -21.358% -19.142%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) high mild
  4 (4.00%) high severe

 See sharksforarms#25
  • Loading branch information
wcampbell0x2a authored and sharksforarms committed Oct 6, 2022
1 parent 93e9864 commit 3e5b51e
Show file tree
Hide file tree
Showing 14 changed files with 294 additions and 126 deletions.
36 changes: 35 additions & 1 deletion benches/deku.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ enum DekuEnum {
VariantA(u8),
}

/// This is faster, because we go right to (endian, bytes)
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct DekuVecPerf {
#[deku(bytes = "1")]
count: u8,
#[deku(count = "count")]
#[deku(bytes = "1")]
data: Vec<u8>,
}

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct DekuVec {
count: u8,
Expand Down Expand Up @@ -60,6 +70,14 @@ fn deku_write_vec(input: &DekuVec) {
let _v = input.to_bytes().unwrap();
}

fn deku_read_vec_perf(input: &[u8]) {
let (_rest, _v) = DekuVecPerf::from_bytes((input, 0)).unwrap();
}

fn deku_write_vec_perf(input: &DekuVecPerf) {
let _v = input.to_bytes().unwrap();
}

fn criterion_benchmark(c: &mut Criterion) {
c.bench_function("deku_read_byte", |b| {
b.iter(|| deku_read_byte(black_box([0x01].as_ref())))
Expand All @@ -71,7 +89,12 @@ fn criterion_benchmark(c: &mut Criterion) {
b.iter(|| deku_read_bits(black_box([0xf1].as_ref())))
});
c.bench_function("deku_write_bits", |b| {
b.iter(|| deku_write_bits(black_box(&DekuBits { data_01: 0x0f, data_02: 0x01 })))
b.iter(|| {
deku_write_bits(black_box(&DekuBits {
data_01: 0x0f,
data_02: 0x01,
}))
})
});

c.bench_function("deku_read_enum", |b| {
Expand All @@ -96,6 +119,17 @@ fn criterion_benchmark(c: &mut Criterion) {
c.bench_function("deku_write_vec", |b| {
b.iter(|| deku_write_vec(black_box(&deku_write_vec_input)))
});

let deku_write_vec_input = DekuVecPerf {
count: 100,
data: vec![0xFF; 100],
};
c.bench_function("deku_read_vec_perf", |b| {
b.iter(|| deku_read_vec_perf(black_box(&deku_read_vec_input)))
});
c.bench_function("deku_write_vec_perf", |b| {
b.iter(|| deku_write_vec_perf(black_box(&deku_write_vec_input)))
});
}

criterion_group!(benches, criterion_benchmark);
Expand Down
8 changes: 4 additions & 4 deletions deku-derive/src/macros/deku_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,27 +597,27 @@ fn emit_field_read(
quote! {
{
use core::borrow::Borrow;
#type_as_deku_read::read(__deku_rest, (::#crate_::ctx::Limit::new_count(usize::try_from(*((#field_count).borrow()))?), (#read_args)))
#type_as_deku_read::::read(__deku_rest, (::#crate_::ctx::Limit::new_count(usize::try_from(*((#field_count).borrow()))?), (#read_args)))
}
}
} else if let Some(field_bits) = &f.bits_read {
quote! {
{
use core::borrow::Borrow;
#type_as_deku_read::read(__deku_rest, (::#crate_::ctx::Limit::new_size(::#crate_::ctx::Size::Bits(usize::try_from(*((#field_bits).borrow()))?)), (#read_args)))
#type_as_deku_read::::read(__deku_rest, (::#crate_::ctx::Limit::new_bit_size(::#crate_::ctx::BitSize(usize::try_from(*((#field_bits).borrow()))?)), (#read_args)))
}
}
} else if let Some(field_bytes) = &f.bytes_read {
quote! {
{
use core::borrow::Borrow;
#type_as_deku_read::read(__deku_rest, (::#crate_::ctx::Limit::new_size(::#crate_::ctx::Size::Bytes(usize::try_from(*((#field_bytes).borrow()))?)), (#read_args)))
#type_as_deku_read::::read(__deku_rest, (::#crate_::ctx::Limit::new_byte_size(::#crate_::ctx::ByteSize(usize::try_from(*((#field_bytes).borrow()))?)), (#read_args)))
}
}
} else if let Some(field_until) = &f.until {
// We wrap the input into another closure here to enforce that it is actually a callable
// Otherwise, an incorrectly passed-in integer could unexpectedly convert into a `Count` limit
quote! {#type_as_deku_read::read(__deku_rest, (::#crate_::ctx::Limit::new_until(#field_until), (#read_args)))}
quote! {#type_as_deku_read::::read(__deku_rest, (::#crate_::ctx::Limit::new_until(#field_until), (#read_args)))}
} else {
quote! {#type_as_deku_read::read(__deku_rest, (#read_args))}
}
Expand Down
14 changes: 8 additions & 6 deletions deku-derive/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,17 @@ fn gen_type_from_ctx_id(
}

/// Generate argument for `id`:
/// `#deku(endian = "big", bits = "1")` -> `Endian::Big, Size::Bits(1)`
/// `#deku(endian = "big", bits = "1")` -> `Endian::Big, BitSize(1)`
/// `#deku(endian = "big", bytes = "1")` -> `Endian::Big, ByteSize(1)`
pub(crate) fn gen_id_args(
endian: Option<&syn::LitStr>,
bits: Option<&Num>,
bytes: Option<&Num>,
) -> syn::Result<TokenStream> {
let crate_ = get_crate_name();
let endian = endian.map(gen_endian_from_str).transpose()?;
let bits = bits.map(|n| quote! {::#crate_::ctx::Size::Bits(#n)});
let bytes = bytes.map(|n| quote! {::#crate_::ctx::Size::Bytes(#n)});
let bits = bits.map(|n| quote! {::#crate_::ctx::BitSize(#n)});
let bytes = bytes.map(|n| quote! {::#crate_::ctx::ByteSize(#n)});

// FIXME: Should be `into_iter` here, see /~https://github.com/rust-lang/rust/issues/66145.
let id_args = [endian.as_ref(), bits.as_ref(), bytes.as_ref()]
Expand All @@ -229,7 +230,8 @@ pub(crate) fn gen_id_args(

/// Generate argument for fields:
///
/// `#deku(endian = "big", bits = "1", ctx = "a")` -> `Endian::Big, Size::Bits(1), a`
/// `#deku(endian = "big", bits = "1", ctx = "a")` -> `Endian::Big, BitSize(1), a`
/// `#deku(endian = "big", bytes = "1", ctx = "a")` -> `Endian::Big, ByteSize(1), a`
fn gen_field_args(
endian: Option<&syn::LitStr>,
bits: Option<&Num>,
Expand All @@ -238,8 +240,8 @@ fn gen_field_args(
) -> syn::Result<TokenStream> {
let crate_ = get_crate_name();
let endian = endian.map(gen_endian_from_str).transpose()?;
let bits = bits.map(|n| quote! {::#crate_::ctx::Size::Bits(#n)});
let bytes = bytes.map(|n| quote! {::#crate_::ctx::Size::Bytes(#n)});
let bits = bits.map(|n| quote! {::#crate_::ctx::BitSize(#n)});
let bytes = bytes.map(|n| quote! {::#crate_::ctx::ByteSize(#n)});
let ctx = ctx.map(|c| quote! {#c});

// FIXME: Should be `into_iter` here, see /~https://github.com/rust-lang/rust/issues/66145.
Expand Down
10 changes: 5 additions & 5 deletions examples/custom_reader_and_writer.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use deku::bitvec::{BitSlice, BitVec, Msb0};
use deku::ctx::Size;
use deku::ctx::BitSize;
use deku::prelude::*;
use std::convert::TryInto;

fn bit_flipper_read(
field_a: u8,
rest: &BitSlice<Msb0, u8>,
bit_size: Size,
bit_size: BitSize,
) -> Result<(&BitSlice<Msb0, u8>, u8), DekuError> {
// Access to previously read fields
println!("field_a = 0x{:X}", field_a);
Expand All @@ -30,7 +30,7 @@ fn bit_flipper_write(
field_a: u8,
field_b: u8,
output: &mut BitVec<Msb0, u8>,
bit_size: Size,
bit_size: BitSize,
) -> Result<(), DekuError> {
// Access to previously written fields
println!("field_a = 0x{:X}", field_a);
Expand All @@ -52,8 +52,8 @@ struct DekuTest {
field_a: u8,

#[deku(
reader = "bit_flipper_read(*field_a, deku::rest, Size::Bits(8))",
writer = "bit_flipper_write(*field_a, *field_b, deku::output, Size::Bits(8))"
reader = "bit_flipper_read(*field_a, deku::rest, BitSize(8))",
writer = "bit_flipper_write(*field_a, *field_b, deku::output, BitSize(8))"
)]
field_b: u8,
}
Expand Down
4 changes: 2 additions & 2 deletions src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ struct Type1 {
// is equivalent to
struct Type1 {
#[deku(ctx = "Endian::Big, Size::Bits(1)")]
#[deku(ctx = "Endian::Big, BitSize(1)")]
field: u8,
}
```
Expand All @@ -874,7 +874,7 @@ struct Type1 {
struct Type1 {
#[deku(ctx = "Endian::Big")]
field_a: u16,
#[deku(ctx = "Endian::Big, Size::Bits(5), *field_a")] // endian is prepended
#[deku(ctx = "Endian::Big, BitSize(5), *field_a")] // endian is prepended
field_b: SubType,
}
```
Expand Down
78 changes: 29 additions & 49 deletions src/ctx.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
//! Types for context representation
//! See [ctx attribute](super::attributes#ctx) for more information.
use crate::error::DekuError;
use core::marker::PhantomData;
use core::str::FromStr;

#[cfg(feature = "alloc")]
use alloc::format;

/// An endian
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub enum Endian {
Expand Down Expand Up @@ -85,8 +81,11 @@ pub enum Limit<T, Predicate: FnMut(&T) -> bool> {
/// Read until a given predicate holds true
Until(Predicate, PhantomData<T>),

/// Read until a given quantity of bytes have been read
ByteSize(ByteSize),

/// Read until a given quantity of bits have been read
Size(Size),
BitSize(BitSize),
}

impl<T> From<usize> for Limit<T, fn(&T) -> bool> {
Expand All @@ -101,9 +100,15 @@ impl<T, Predicate: for<'a> FnMut(&'a T) -> bool> From<Predicate> for Limit<T, Pr
}
}

impl<T> From<Size> for Limit<T, fn(&T) -> bool> {
fn from(size: Size) -> Self {
Limit::Size(size)
impl<T> From<ByteSize> for Limit<T, fn(&T) -> bool> {
fn from(size: ByteSize) -> Self {
Limit::ByteSize(size)
}
}

impl<T> From<BitSize> for Limit<T, fn(&T) -> bool> {
fn from(size: BitSize) -> Self {
Limit::BitSize(size)
}
}

Expand All @@ -123,35 +128,39 @@ impl<T> Limit<T, fn(&T) -> bool> {
}

/// Constructs a new Limit that reads until the given size
pub fn new_size(size: Size) -> Self {
pub fn new_bit_size(size: BitSize) -> Self {
size.into()
}

/// Constructs a new Limit that reads until the given size
pub fn new_byte_size(size: ByteSize) -> Self {
size.into()
}
}

/// The size of a field
/// The size of field in bytes
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
pub enum Size {
/// bit size
Bits(usize),
/// byte size
Bytes(usize),
}
pub struct ByteSize(pub usize);

impl Size {
/// The size of field in bits
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
pub struct BitSize(pub usize);

impl BitSize {
/// Convert the size in bytes to a bit size.
///
/// # Panic
/// Panic if `byte_size * 8` is greater than `usize::MAX`.
fn bits_from_bytes(byte_size: usize) -> Self {
Self::Bits(byte_size.checked_mul(8).expect("bit size overflow"))
Self(byte_size.checked_mul(8).expect("bit size overflow"))
}

/// Returns the bit size of a type.
/// # Examples
/// ```rust
/// # use deku::ctx::Size;
/// # use deku::ctx::BitSize;
///
/// assert_eq!(Size::of::<i32>(), Size::Bits(4 * 8));
/// assert_eq!(BitSize::of::<i32>(), BitSize(4 * 8));
/// ```
///
/// # Panics
Expand All @@ -164,33 +173,4 @@ impl Size {
pub fn of_val<T: ?Sized>(val: &T) -> Self {
Self::bits_from_bytes(core::mem::size_of_val(val))
}

/// Returns the size in bits of a Size
///
/// # Panics
/// Panic if the bit size of Size::Bytes(n) is greater than `usize::MAX`
pub fn bit_size(&self) -> usize {
match *self {
Size::Bits(size) => size,
Size::Bytes(size) => size.checked_mul(8).expect("bit size overflow"),
}
}

/// Returns the size in bytes of a Size
pub fn byte_size(&self) -> Result<usize, DekuError> {
match *self {
Size::Bits(size) => {
if size % 8 == 0 {
Ok(size / 8)
} else {
Err(DekuError::InvalidParam(format!(
"Bit size of {} is not a multiple of 8.
Cannot be represented in bytes",
size
)))
}
}
Size::Bytes(size) => Ok(size),
}
}
}
2 changes: 1 addition & 1 deletion src/impls/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod tests {
let input = &[0b01_000000];
let bit_slice = input.view_bits::<Msb0>();

let (rest, res_read) = bool::read(bit_slice, crate::ctx::Size::Bits(2)).unwrap();
let (rest, res_read) = bool::read(bit_slice, crate::ctx::BitSize(2)).unwrap();
assert_eq!(true, res_read);
assert_eq!(6, rest.len());

Expand Down
8 changes: 4 additions & 4 deletions src/impls/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ mod tests {
case::normal_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), 2.into(), vec![0xAABB, 0xCCDD].into_boxed_slice(), bits![Msb0, u8;], vec![0xAA, 0xBB, 0xCC, 0xDD]),
case::predicate_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), (|v: &u16| *v == 0xBBAA).into(), vec![0xBBAA].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
case::predicate_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), (|v: &u16| *v == 0xAABB).into(), vec![0xAABB].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
case::bytes_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), Size::Bits(16).into(), vec![0xBBAA].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
case::bytes_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), Size::Bits(16).into(), vec![0xAABB].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
case::bytes_le([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Little, Some(16), BitSize(16).into(), vec![0xBBAA].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
case::bytes_be([0xAA, 0xBB, 0xCC, 0xDD].as_ref(), Endian::Big, Some(16), BitSize(16).into(), vec![0xAABB].into_boxed_slice(), bits![Msb0, u8; 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 0, 1, 1, 1, 0, 1], vec![0xAA, 0xBB]),
)]
fn test_boxed_slice<Predicate: FnMut(&u16) -> bool>(
input: &[u8],
Expand All @@ -114,13 +114,13 @@ mod tests {
let bit_size = bit_size.unwrap();

let (rest, res_read) =
<Box<[u16]>>::read(bit_slice, (limit, (endian, Size::Bits(bit_size)))).unwrap();
<Box<[u16]>>::read(bit_slice, (limit, (endian, BitSize(bit_size)))).unwrap();
assert_eq!(expected, res_read);
assert_eq!(expected_rest, rest);

let mut res_write = bitvec![Msb0, u8;];
res_read
.write(&mut res_write, (endian, Size::Bits(bit_size)))
.write(&mut res_write, (endian, BitSize(bit_size)))
.unwrap();
assert_eq!(expected_write, res_write.into_vec());

Expand Down
Loading

0 comments on commit 3e5b51e

Please sign in to comment.