You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Reading a #[deku(bits = N)] field currently goes through an intermediate
bit-slice: deku copies the bits out of the input one slice operation at a time,
assembles them into a BitSlice, and only then turns that into an integer.
Writing does the mirror image. That intermediate is where nearly all the time
goes.
This PR skips it. For big-endian, Order::Msb0 fields only, the value is
read straight into an integer and written straight out of one. No bit-slice is
built at all.
The two changes optimise the same idea on two different public traits, and
both stay reachable:
perf: zero-extend big-endian bit fields with load_be #666 speeds up DekuRead::read(&BitSlice, ctx), the slice-level trait.
That is the entry point for anyone who already holds a BitSlice, and it is
what deku's own delegating impls in src/impls/primitive.rs call internally
(the ByteSize and signed-integer wrappers all funnel into it).
This PR speeds up DekuReader::from_reader_with_ctx, the reader-level
trait. Derived from_readerand derived from_bytes both go through it,
since from_bytes builds a Cursor and calls from_reader_with_ctx.
Where both apply, this one returns first, so a derived big-endian Msb0 read no
longer reaches #666's line. But DekuRead::read is public and still reached by
direct callers, hand-written impls, and deku's internal delegating impls, so #666 keeps doing its job there. Same optimisation, two layers, neither made
dead by the other.
Given the input bytes [0x2A, 0xB5], that is the bit stream 00 1010101011 0101, so version = 0 and id = 683.
Reading version consumes byte 0x2A, keeps its top 2 bits, and leaves the
other 6 behind as the reader's "leftover". So both traces below start from:
leftover = 101010 (6 bits)
next byte in the stream = 0xB5
Reading id today
1. allocate a 16-bit BitArray, take dst = &mut bits[..10]
bits = 0000000000 000000
dst ^^^^^^^^^^
2. copy the 6 leftover bits into dst[..6] (copy_from_bitslice)
dst = 101010 ???? 4 bits still missing
3. dst[6..10] is 4 bits, less than a whole byte, so the whole-byte loop
(chunks_exact_mut(8)) runs zero times and hands back a 4-bit remainder
4. read byte 0xB5, view it as a BitSlice, split_at_mut(4)
0xB5 = 1011 0101
used rest
copy `used` into the remainder (copy_from_bitslice)
dst = 101010 1011 complete
5. store `rest` as the new leftover, rebuilt as a BoundedBitVec
leftover = 0101 (4 bits)
6. hand dst, a 10-bit BitSlice, to u16::read, which zero-extends it to 16
bits and reads it big-endian
000000 1010101011 = 683
Six steps, three bit-slice copies, and two BoundedBitVecs built and dropped,
to move 10 bits. (Step 6 is the one #666 already fixed: before that PR the
zero-extension was 6 separate insert(0, false) calls, each shifting the whole
16-bit array. #666 made it a single load_be.)
Reading id with this PR
The whole read is one u128 accumulator, acc, and a count of how many bits
are in it, have.
1. start from the leftover. It is stored as one byte, top-aligned, plus a
length, so shifting it down is all it takes:
leftover byte = 10101000, length 6
acc = 10101000 >> (8 - 6) = 101010
have = 6
2. we need 10 bits and have 6, so pull one byte and shift it in:
read 0xB5 = 10110101
acc = (101010 << 8) | 10110101 = 101010 10110101
have = 14
3. we have 4 bits more than we asked for, so the value is `acc` shifted
right by those 4, masked to 10 bits:
rest = 14 - 10 = 4
value = acc >> 4 = 1010101011 = 683
^^^^^^^^^^ the 10 bits we wanted
4. the 4 bits we shifted off become the new leftover, kept as a byte with a
length rather than a bit-vector:
tail = acc & 0b1111 = 0101
leftover = 01010000, length 4
One byte fetched, one shift, one mask. Steps 1 to 5 of the old path are gone,
and no BitSlice or BoundedBitVec is ever constructed.
Writing id = 683, after version has queued 2 bits
Same accumulator, run backwards. Writing version emitted nothing, it only left
2 pending bits (00) waiting for the rest of the byte.
1. does 683 fit in the 10 bits the field declares?
683 as a big-endian u16: 0000 0010 1010 1011
^^^^^^ six leading zeros
significant bits = 16 - 6 = 10, and 10 <= 10, so yes
2. shift the 2 pending bits up and OR the value underneath:
acc = (00 << 10) | 1010101011 = 00 1010101011
have = 2 + 10 = 12
3. 12 bits is one whole byte plus 4. Emit the whole byte:
whole = 12 / 8 = 1, rest = 12 % 8 = 4
byte = acc >> 4 = 00101010 = 0x2A (one write_all)
4. the 4 bits that did not fill a byte stay pending:
tail = acc & 0b1111 = 1011
leftover = 10110000, length 4
That trailing 1011 is the top half of 0xB5, which is exactly where the next
field continues. Read and write are mirror images of each other.
Step 1 is worth calling out on its own. Today that check scans a bit-slice with first_one to locate the highest set bit:
today: first_one over the 16-bit slice -> index 6
error if (16 - 10) > 6 -> 6 > 6 -> false, fits
now: significant = 16 - leading_zeros = 16 - 6 = 10
error if 10 > 10 -> false, fits
Identical test. One walks a bit-slice, the other is a single leading_zeros
instruction.
Why this matters more than it looks
The cost removed here is per field and flat. It does not depend on how many
bits the field holds, because it is the price of building and tearing down the
bit-slice machinery, not of moving the bits.
The measurements say exactly that. The CCSDS header has 11 bit fields and costs
723 ns, which is 66 ns per field. A single 1-bit field in a u64 costs 69 ns.
Same number. A field that carries one bit pays what a field carrying eleven
does.
After this PR those become 23.8 ns for 11 fields (2.2 ns each) and 1.66 ns for
the single field. Still flat, just roughly 30 times smaller.
Concretely, per file
src/reader.rs: new Reader::read_bits_uint_msb0(amt), the read shown above.
src/writer.rs: new Writer::write_bits_uint_msb0(value, amt), the dual.
Whole bytes leave in one write_all rather than one call per byte.
src/impls/primitive.rs: DekuReader::from_reader_with_ctx and DekuWriter::to_writer return early through those helpers when the field is
big-endian and Msb0, plus the integer fit check.
src/lib.rs: two small helpers to view a one-byte Msb0 leftover as (byte, bit count).
Little-endian and Order::Lsb0 are untouched and keep their existing code path.
Numbers
Measured against master at 088018f, so this includes #657, #659, #661 and the
syn v3 upgrade. Per-item cost, from benches that push 128 frames through a
single reader or writer so nothing is overlapped or optimised away:
before
after
CCSDS TM primary header, 11 bit fields, read
723 ns
23.8 ns
30x
Same header, write
534 ns
182 ns
2.9x
1-bit field in a u64, read
69.1 ns
1.66 ns
42x
6 byte-aligned fields, read (control)
12.8 ns
12.8 ns
no change
6 byte-aligned fields, write (control)
20.4 ns
22.7 ns
no change, see below
The two control rows matter as much as the wins, since a change scoped to bit
fields should leave byte-aligned ones alone. Criterion agrees on the read
control, reporting "No change in performance detected".
The byte-aligned write control is too noisy on my machine to say anything
with. Re-running the unmodifiedmaster twice against its own baseline, the
same benchmark reported +2.6% and then +13.3%, so its noise floor is at least
±13% here and the +8% on this branch sits inside it. Nothing in this PR touches
that path: the writer.rs change is purely additive, and the primitive.rs
change is behind a big-endian Msb0 bit-field guard. Worth watching on CI
hardware.
Reproduce with cargo bench --bench bebits --all-features.
Also in here
One commit improves benches/bebits.rs itself: it black_boxes the inputs
(without it the compiler can see the buffer contents and fold part of the read
away, which made the old numbers look better than they were) and adds benches
that read or write a stream of frames rather than one struct per iteration.
Testing
cargo test across the full CI feature matrix (default, --all-features, --no-default-features and each individual feature) plus all examples. No test
changes were needed: the fast path is required to produce bit-identical results
to the path it replaces.
I have a follow-up ready that builds directly on the helpers this PR introduces.
This PR makes one bit field cheap. The follow-up makes a run of adjacent
bit fields cost one call instead of one call per field: the derive detects
neighbouring fields that a single read can serve, emits one read_bits_uint_msb0 for the whole run, and gives each field its bits with a
compile-time shift and mask. Writing is the mirror.
Measured on top of this PR, per header, from benches that push 128 frames
through a single reader or writer:
CCSDS TM primary header, 11 fields
this PR
+ follow-up
read
23.8 ns
5.74 ns (4.1x)
write
182 ns
17.9 ns (10x)
The follow-up touches only deku-derive, plus making the two helpers here pub so generated code can call them. Anything a run cannot serve keeps its
current per-field path, so it stays purely additive.
It is on my fork at perf/derive-batch-bit-runs
if you want to look now. I would rather land this one first and open that as a
clean two-commit PR afterwards, but happy to open it right away if you prefer
to review them together.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Reading a
#[deku(bits = N)]field currently goes through an intermediatebit-slice: deku copies the bits out of the input one slice operation at a time,
assembles them into a
BitSlice, and only then turns that into an integer.Writing does the mirror image. That intermediate is where nearly all the time
goes.
This PR skips it. For big-endian,
Order::Msb0fields only, the value isread straight into an integer and written straight out of one. No bit-slice is
built at all.
This does not replace #666
The two changes optimise the same idea on two different public traits, and
both stay reachable:
DekuRead::read(&BitSlice, ctx), the slice-level trait.That is the entry point for anyone who already holds a
BitSlice, and it iswhat deku's own delegating impls in
src/impls/primitive.rscall internally(the
ByteSizeand signed-integer wrappers all funnel into it).DekuReader::from_reader_with_ctx, the reader-leveltrait. Derived
from_readerand derivedfrom_bytesboth go through it,since
from_bytesbuilds aCursorand callsfrom_reader_with_ctx.Where both apply, this one returns first, so a derived big-endian
Msb0read nolonger reaches #666's line. But
DekuRead::readis public and still reached bydirect callers, hand-written impls, and deku's internal delegating impls, so
#666 keeps doing its job there. Same optimisation, two layers, neither made
dead by the other.
What it looks like on real bytes
Take a two-field header:
Given the input bytes
[0x2A, 0xB5], that is the bit stream00 1010101011 0101, soversion = 0andid = 683.Reading
versionconsumes byte0x2A, keeps its top 2 bits, and leaves theother 6 behind as the reader's "leftover". So both traces below start from:
Reading
idtodaySix steps, three bit-slice copies, and two
BoundedBitVecs built and dropped,to move 10 bits. (Step 6 is the one #666 already fixed: before that PR the
zero-extension was 6 separate
insert(0, false)calls, each shifting the whole16-bit array. #666 made it a single
load_be.)Reading
idwith this PRThe whole read is one
u128accumulator,acc, and a count of how many bitsare in it,
have.One byte fetched, one shift, one mask. Steps 1 to 5 of the old path are gone,
and no
BitSliceorBoundedBitVecis ever constructed.Writing
id = 683, afterversionhas queued 2 bitsSame accumulator, run backwards. Writing
versionemitted nothing, it only left2 pending bits (
00) waiting for the rest of the byte.That trailing
1011is the top half of0xB5, which is exactly where the nextfield continues. Read and write are mirror images of each other.
Step 1 is worth calling out on its own. Today that check scans a bit-slice with
first_oneto locate the highest set bit:Identical test. One walks a bit-slice, the other is a single
leading_zerosinstruction.
Why this matters more than it looks
The cost removed here is per field and flat. It does not depend on how many
bits the field holds, because it is the price of building and tearing down the
bit-slice machinery, not of moving the bits.
The measurements say exactly that. The CCSDS header has 11 bit fields and costs
723 ns, which is 66 ns per field. A single 1-bit field in a
u64costs 69 ns.Same number. A field that carries one bit pays what a field carrying eleven
does.
After this PR those become 23.8 ns for 11 fields (2.2 ns each) and 1.66 ns for
the single field. Still flat, just roughly 30 times smaller.
Concretely, per file
src/reader.rs: newReader::read_bits_uint_msb0(amt), the read shown above.src/writer.rs: newWriter::write_bits_uint_msb0(value, amt), the dual.Whole bytes leave in one
write_allrather than one call per byte.src/impls/primitive.rs:DekuReader::from_reader_with_ctxandDekuWriter::to_writerreturn early through those helpers when the field isbig-endian and
Msb0, plus the integer fit check.src/lib.rs: two small helpers to view a one-byteMsb0leftover as(byte, bit count).Little-endian and
Order::Lsb0are untouched and keep their existing code path.Numbers
Measured against
masterat 088018f, so this includes #657, #659, #661 and thesyn v3 upgrade. Per-item cost, from benches that push 128 frames through a
single reader or writer so nothing is overlapped or optimised away:
u64, readThe two control rows matter as much as the wins, since a change scoped to bit
fields should leave byte-aligned ones alone. Criterion agrees on the read
control, reporting "No change in performance detected".
The byte-aligned write control is too noisy on my machine to say anything
with. Re-running the unmodified
mastertwice against its own baseline, thesame benchmark reported +2.6% and then +13.3%, so its noise floor is at least
±13% here and the +8% on this branch sits inside it. Nothing in this PR touches
that path: the
writer.rschange is purely additive, and theprimitive.rschange is behind a big-endian
Msb0bit-field guard. Worth watching on CIhardware.
Reproduce with
cargo bench --bench bebits --all-features.Also in here
One commit improves
benches/bebits.rsitself: itblack_boxes the inputs(without it the compiler can see the buffer contents and fold part of the read
away, which made the old numbers look better than they were) and adds benches
that read or write a stream of frames rather than one struct per iteration.
Testing
cargo testacross the full CI feature matrix (default,--all-features,--no-default-featuresand each individual feature) plus all examples. No testchanges were needed: the fast path is required to produce bit-identical results
to the path it replaces.