Repository navigation
perf(aead-frame): add Criterion benches for the storage profile, so its cost is measured - #534
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🧬 Mutation testing (cargo-mutants,
|
✅ No CRAP threshold violations1500 function(s) analyzed · threshold 30 |
9c2cbe0 to
039186e
Compare
b2b590d to
d9fc3a6
Compare
039186e to
fb57376
Compare
d9fc3a6 to
c2664d7
Compare
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟢 merge as it is
Nothing must change before merging. This PR adds only a bench, dev-dependencies and README text. Three optional comments can wait: a doc comment, a variable name, and a CI step that runs the bench once.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 1 found, 1 posted |
| claude | claude-opus-5-5 | rust | 2 found, 2 posted |
| codex | gpt-5.6-terra | test-gap | 0 found, 0 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 0 posted finding(s) were raised by two or more models.
Plain language: claude-sonnet-5-5 read every comment as a new reader would. 2 comment(s) had a problem that stopped the reader acting; it rewrote 0.
Stack: position 3 of 3 (#532, *️⃣ #534, #557). *️⃣ marks this pull request.
Context loaded: the description, 0 linked issue(s) and 3 discussion entries.
fb57376 to
b28f087
Compare
…ts cost is measured Writes and reads five values with the storage profile, the transport serde format and serde_json, and records the results in the crate README.
…rsh, so its cost is set against compact binary formats
… bench gap reads correctly
…sentence, and call Borsh's bytes borsh_bytes so they read apart from the crate
…t stops reading a value back fails CI
c2664d7 to
1937be3
Compare
Closes #562. Stacked on #532.
Summary
This adds Criterion benches for the storage profile. They write and read five values with
storage::to_vecandstorage::from_slice. The transport serde format,serde_json, MessagePack (rmp_serde) and Borsh run on the same values for comparison. The results are also in the crate README, under Performance.Before it is timed, each value is checked to read back equal in all five formats. Each write includes dropping its result, so the wipe is measured too. MessagePack is written with field names (
to_vec_named), as the storage profile, the transport format and JSON carry them. Borsh carries only the values in order, so it shows what a format costs that carries nothing else.Neither MessagePack, Borsh nor JSON wipes anything; none of them depends on
zeroize. They write into a plainVec<u8>, so each time the buffer grows the old one is freed with its bytes still in it, and the result is freed unwiped when it is dropped. Wrapping their output inZeroizingwould cover only the drop. So on large values most of the gap below is the wipe, not the format.Run them with:
Evidence
These ran on an Apple M4 (4 performance and 6 efficiency cores, 24 GB), macOS 26.5, rustc 1.98.1, Criterion 0.8.2 at its default settings, on mains power. Each time is the median of 100 samples. The background load was under one of the 10 cores (OrbStack, the window server, the terminal).
Size of the bytes written:
Time to write:
Time to read:
Two results were noisy: the MessagePack read of 1,000 records (95% interval 387 to 567 µs) and the transport write of 1,000 records (724 to 773 µs). Every other interval is within a few percent of its median. The storage, transport and JSON numbers match the first run of this PR, without MessagePack and Borsh, to within a few percent.
To explain the numbers, I ran some throwaway probes. They are not committed.
Vecwithzeroizealone takes 31.5 µs, about 2 GiB/s, because it writes one byte at a time with volatile stores. That is 93% of the 33.7 µs write. The transport writer pays the same cost.u64is written in 50 ns, an array of seven in 307 ns, and a struct of sevenu64fields in 703 ns (621 ns with the transport format): about 37 ns for each element and 90 ns for each field. The drop is only about 140 ns of the record's 986 ns. Reading the writer, I found no allocation for each child. I haven't found the cause: no profiler is installed here apart fromxctrace. This is the clearest place to look for a speedup.Checks:
cargo clippy -p vitaminc-aead-frame --all-targets --all-features -D warnings,cargo +1.88.0 check --all-targets --features serde(the MSRV; criterion 0.8.2 needs 1.86), doctests, andcargo fmt --check.Merge Danger
Door: two-way
This adds only a bench and dev-dependencies. Nothing in the library changes.
Blast Radius: none
The bench is built only by
cargo bench, and by CI's--all-targetsclippy and MSRV jobs. Criterion adds 19 crates toCargo.lock, for dev builds only.borshandrmp-serdewere already in the lock file (rmp-serdeis a dev-dependency ofvitaminc-protectedandvitaminc-traits). The README numbers depend on the machine they ran on, and will drift as the code changes.