Skip to content

Add Ci grungraun; Fix 6 simple byte read regression; Add Protocol Benchmarks - #682

Merged
wcampbell0x2a merged 5 commits into
masterfrom
wc/ci-gungraun-and-bitrun-derive-guard
Oct 18, 2026
Merged

wcampbell0x2a merged 5 commits into
masterfrom
wc/ci-gungraun-and-bitrun-derive-guard

Conversation

@wcampbell0x2a

@wcampbell0x2a wcampbell0x2a commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

See commits. as they will be easier to review as commits since I moved some code around.

422b79c bench: Use gungraun to easily compare instr counts

  • Add criterion for measuring on CI (non-quiet infra). This will hopefully
    lead to good benchmarks to use in merge requests, while keeping
    the regular criterion for wall-time, which is still very usefull since
    callgrind is just estimating the cputime per instruction.
  • Copy the existing tests into shared shapes usable by both.

187a48b Keep whole-byte field runs on the byte path

  • Fix regression fonud in be_six_bytes_aligned_x128.
  • Apply batching logic only if the fields are not whole-width reading.
    For example, on a plain u8 field read with no bit-reads. This uses
    the u64 and shift/mask logic. This was greatly needed to cover
    the regression of be_six_bytes_aligned_x128. This also should work
    over the problem found in perf: read and write a run of adjacent bit fields in one call #677 regarding protocol pipeline issues.
  • I benchmarked this over a multi-protocol benchmark (in which we should
    add) and it shows gains (also shows gains in the micro-benchmark
    of be_six_bytes_aligned_x128.

537552b Move plan_bits_run and friends into common mod

6a73d81 test: Add protocol benchmarking to evade branch prediction

  • Add testing to confirm the comment above read_bits_uint_msb0 where
    having code that the branch predictor can't help with quickly makes
    the code pattern that we currently have required. The code didn't really
    mention the branch predictor, but I bet that's what they were hitting.

f2f29c2 ci: run benchmark suite in Docker
Keep only the callgrind benchmark, which gives stable instruction
counts. The criterion wall-clock job gave noisy results on CI runners.

@wcampbell0x2a

Copy link
Copy Markdown
Collaborator Author

@Simon-Calbert-Aerospacelab in #677 you mentioned that pipeline'ing benchmarks are driving the decision you made in src/reader.rs, so I added some benchmarks to prove it. See b2e5de0, as I make some more assumptions on getting some performance back for some common paths.

@wcampbell0x2a

Copy link
Copy Markdown
Collaborator Author

The benchmark CI I added/changed is failing. But hopefully it's just benches against master which are expected to fail.

@wcampbell0x2a

Copy link
Copy Markdown
Collaborator Author

I need to update for #684

@wcampbell0x2a
wcampbell0x2a force-pushed the wc/ci-gungraun-and-bitrun-derive-guard branch from 03fe9af to 288f941 Compare September 24, 2026 03:58
* Add criterion for measuring on CI (non-quiet infra). This will hopefully
  lead to good benchmarks to use in merge requests, while keeping
  the regular criterion for wall-time, which is still very usefull since
  callgrind is just estimating the cputime per instruction.
* Copy the existing tests into shared shapes usable by both.
* Fix regression fonud in be_six_bytes_aligned_x128.
* Apply batching logic only if the fields are not whole-width reading.
  For example, on a plain u8 field read with no bit-reads. This uses
  the u64 and shift/mask logic. This was greatly needed to cover
  the regression of be_six_bytes_aligned_x128. This also should work
  over the problem found in #677 regarding protocol pipeline issues.
* I benchmarked this over a multi-protocol benchmark (in which we should
  add) and it shows gains (also shows gains in the micro-benchmark
  of be_six_bytes_aligned_x128.
* Add testing to confirm the comment above read_bits_uint_msb0 where
  having code that the branch predictor can't help with quickly makes
  the code pattern that we currently have required. The code didn't really
  mention the branch predictor, but I bet that's what they were hitting.

Code generated using coding agent.
@sharksforarms
sharksforarms force-pushed the wc/ci-gungraun-and-bitrun-derive-guard branch from 288f941 to 6cec88f Compare October 18, 2026 00:44
@sharksforarms
sharksforarms force-pushed the wc/ci-gungraun-and-bitrun-derive-guard branch from 6cec88f to f2f29c2 Compare October 18, 2026 00:45
@wcampbell0x2a
wcampbell0x2a merged commit 17c8db2 into master Oct 18, 2026
9 checks passed
@wcampbell0x2a
wcampbell0x2a deleted the wc/ci-gungraun-and-bitrun-derive-guard branch October 18, 2026 01:33
@github-actions github-actions Bot mentioned this pull request Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants