Skip to content

Commit

Permalink
Implement from_reader io::Read support
Browse files Browse the repository at this point in the history
Add from_reader and use Read internally to improve the codegen of deku,
the performance of derived parsers. Internally, this is implemented
using the new src/reader.rs interface in deku parses. This has a waterfall
effect on DekuReader implementations and the deku-derive/deku_read Derived impl.
Previous usage with from_bytes and other API's are unchanged.
There is somewhat of a performance hit for bit-only parses,
but I find the major improvments in the bytes-wise parsing to be great enough
to warrent this change.

The following are some sample benchmarks:
> critcmp before after
group                  after                                  before
-----                  -----                                  ------
deku_read_bits         1.24   845.8±14.29ns        ? ?/sec    1.00   679.5±11.83ns        ? ?/sec
deku_read_byte         1.00     17.8±0.25ns        ? ?/sec    2.12     37.8±3.35ns        ? ?/sec
deku_read_enum         1.00     15.3±0.15ns        ? ?/sec    2.04     31.2±0.81ns        ? ?/sec
deku_read_vec          1.00    676.8±7.04ns        ? ?/sec    2.16  1459.5±40.22ns        ? ?/sec
deku_write_bits        1.00    125.3±3.10ns        ? ?/sec    1.04   130.2±11.12ns        ? ?/sec
deku_write_byte        1.00    161.6±4.86ns        ? ?/sec    1.02    165.0±5.91ns        ? ?/sec
deku_write_enum        1.00    105.6±1.06ns        ? ?/sec    1.03    109.0±7.20ns        ? ?/sec
deku_write_vec         1.00      4.6±0.04µs        ? ?/sec    1.06      4.9±0.07µs        ? ?/sec

The above change removes DekuRead, and replaces it with DekuReader. This contains
the from_reader_with_ctx. DekuContainerRead contains from_reader.

The crate no_std_io was picked to supply a Read impl
for the no_std feature. These are "re-export"ed.

Add "`Read` enabled" docs to lib.rs

Add tests/test_tuple.rs tests

Update CHANGELOG.md to reflect changes and help migration to this usage

Use llvm-cov in ci for the generation of more accurate coverage reports

Update benchmarks to test more complex parser speeds

Disable Miri CI

Update ensure_no_std to work with new Read usage. Remove wee-alloc in favour
of an updated crate for the allocator.

Add inline to small functions
  • Loading branch information
wcampbell0x2a committed Dec 13, 2023
1 parent 3ec4d10 commit cc12d18
Show file tree
Hide file tree
Showing 69 changed files with 2,732 additions and 1,794 deletions.
22 changes: 22 additions & 0 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Coverage

on: [pull_request, push]

jobs:
coverage:
runs-on: ubuntu-latest
env:
CARGO_TERM_COLOR: always
steps:
- uses: actions/checkout@v3
- name: Install Rust
run: rustup update stable
- name: Install cargo-llvm-cov
uses: taiki-e/install-action@cargo-llvm-cov
- name: Generate code coverage
run: cargo llvm-cov --workspace --codecov --output-path codecov.json
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v3
with:
files: codecov.json
fail_ci_if_error: true
69 changes: 27 additions & 42 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,31 @@ jobs:
command: test
args: --all

test_miri:
name: Miri Test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions-rs/toolchain@v1
with:
toolchain: nightly
override: true
components: miri
- run: cargo miri test

test_miri_big_endian:
name: Miri Test Big Endian
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions-rs/toolchain@v1
with:
toolchain: nightly
override: true
components: miri
target: mips64-unknown-linux-gnuabi64
- run: cargo miri test --target mips64-unknown-linux-gnuabi64
# TODO: Enable Miri
# test_miri:
# name: Miri Test
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v3
# - uses: actions-rs/toolchain@v1
# with:
# toolchain: nightly
# override: true
# components: miri
# - run: cargo miri test
#
# test_miri_big_endian:
# name: Miri Test Big Endian
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v3
# - uses: actions-rs/toolchain@v1
# with:
# toolchain: nightly
# override: true
# components: miri
# target: armebv7r-none-eabi
# - run: cargo miri test --target armebv7r-none-eabi

examples:
name: Examples
Expand Down Expand Up @@ -111,7 +112,8 @@ jobs:
with:
toolchain: nightly
override: true
- run: cd ensure_no_std && cargo run --release
target: thumbv7em-none-eabihf
- run: cd ensure_no_std && cargo build --release --target thumbv7em-none-eabihf

ensure_wasm:
name: Ensure wasm
Expand All @@ -126,20 +128,3 @@ jobs:
with:
version: 'latest'
- run: cd ensure_wasm && wasm-pack build --target web && wasm-pack test --node

coverage:
name: Coverage
runs-on: ubuntu-latest
container:
image: xd009642/tarpaulin:develop
options: --security-opt seccomp=unconfined
steps:
- name: Checkout repository
uses: actions/checkout@v3

- name: Generate code coverage
run: |
cargo tarpaulin --verbose --all-features --workspace --timeout 120 --out Xml
- name: Upload to codecov.io
uses: codecov/codecov-action@v1
129 changes: 129 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,135 @@

## [Unreleased]

## Changes
[#352](https://github.com/sharksforarms/deku/pull/352) added a new function `from_reader` that uses `io::Read`.
`io::Read` is also now used internally, bringing massive performance and usability improvements.

### New `from_reader`
```rust
use std::io::{Seek, SeekFrom, Read};
use std::fs::File;
use deku::prelude::*;

#[derive(Debug, DekuRead, DekuWrite, PartialEq, Eq, Clone, Hash)]
#[deku(endian = "big")]
struct EcHdr {
magic: [u8; 4],
version: u8,
padding1: [u8; 3],
}

let mut file = File::options().read(true).open("file").unwrap();
let ec = EcHdr::from_reader((&mut file, 0)).unwrap();
```

- The more internal (with context) `read(..)` was replaced with `from_reader_with_ctx(..)`.
With the switch to internal streaming, the variables `deku::input`, `deku::input_bits`, and `deku::rest` are now not possible and were removed.
`deku::reader` is a replacement for some of the functionality.
See [examples/deku_input.rs](examples/deku_input.rs) for a new example of caching all reads.

old:
```rust
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct DekuTest {
field_a: u8,

#[deku(
reader = "bit_flipper_read(*field_a, deku::rest, BitSize(8))",
)]
field_b: u8,
}

fn custom_read(
field_a: u8,
rest: &BitSlice<u8, Msb0>,
bit_size: BitSize,
) -> Result<(&BitSlice<u8, Msb0>, u8), DekuError> {

// read field_b, calling original func
let (rest, value) = u8::read(rest, bit_size)?;

Ok((rest, value))
}
```

new:
```rust
#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct DekuTest {
field_a: u8,

#[deku(
reader = "bit_flipper_read(*field_a, deku::reader, BitSize(8))",
)]
field_b: u8,
}

fn custom_read<R: std::io::Read>(
field_a: u8,
reader: &mut Reader<R>,
bit_size: BitSize,
) -> Result<u8, DekuError> {

// read field_b, calling original func
let value = u8::from_reader_with_ctx(reader, bit_size)?;

Ok(value)
}
```

- With the addition of using `Read`, containing a byte slice with a reference is not supported:

old
```rust
#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct<'a> {
bytes: u8,

#[deku(bytes_read = "bytes")]
data: &'a [u8],
}
```

new
```rust
#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct {
bytes: u8,

#[deku(bytes_read = "bytes")]
data: Vec<u8>,
}
```

- `id_pat` is now required to be the same type as stored id.
This also disallows using tuples for storing the id:

old:
```rust
#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
#[deku(type = "u8")]
enum DekuTest {
#[deku(id_pat = "_")]
VariantC((u8, u8)),
}
```

new:
```rust
#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
#[deku(type = "u8")]
enum DekuTest {
#[deku(id_pat = "_")]
VariantC {
id: u8,
other: u8,
},
}
```

- The feature `const_generics` was removed and is enabled by default.

## [0.16.0] - 2023-02-28

### Changes
Expand Down
7 changes: 4 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ keywords = ["deku", "bits", "serialization", "deserialization", "struct"]
categories = ["encoding", "parsing", "no-std"]
description = "bit level serialization/deserialization proc-macro for structs"
readme = "README.md"
rust-version = "1.65.0"

[lib]
bench = false
Expand All @@ -19,16 +20,16 @@ members = [
]

[features]
default = ["std", "const_generics"]
std = ["deku_derive/std", "bitvec/std", "alloc"]
default = ["std"]
std = ["deku_derive/std", "bitvec/std", "alloc", "no_std_io/std"]
alloc = ["bitvec/alloc"]
logging = ["deku_derive/logging", "log"]
const_generics = []

[dependencies]
deku_derive = { version = "^0.16.0", path = "deku-derive", default-features = false}
bitvec = { version = "1.0.1", default-features = false }
log = { version = "0.4.17", optional = true }
no_std_io = { version = "0.5.0", default-features = false, features = ["alloc"] }

[dev-dependencies]
rstest = "0.16.0"
Expand Down
Loading

0 comments on commit cc12d18

Please sign in to comment.