feat: add mantis-seqlock — lock-free sequence lock#20
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ce bracketing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…re fence Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sum payload Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix torn_read.rs clippy warnings (expect_used, print_stderr). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed read Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Vendor rigtorp/Seqlock.h (MIT) to crates/bench/cpp/ - Add seqlock_bench_contender.cpp with BenchMsg64 C FFI wrapper - Add bench-seqlock-contenders (amanieu seqlock 0.2) and bench-seqlock-contenders-cpp features to mantis-bench - Add seqlock_ffi module to mantis-bench library for C++ FFI symbols (required so native link flags propagate through the rlib to the bench) - Wire contender functions into criterion_group in seqlock.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…28 for all implementations
…fetch optimization - Replace read_volatile with CopyPolicy::copy_out (enables NEON/SSE2 wide loads) - Use compiler_fence on x86_64 (TSO), hardware fence on ARM64 only - Prefetch data cache line before sequence counter read - All tests pass including torn-read detection
📝 WalkthroughWalkthroughAdds a new workspace crate Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as Runner (Linux/macOS)
participant Cargo as cargo +nightly bench --bench seqlock
participant Art as Artifacts (bench JSON)
participant Script as .github/scripts/bench-report.sh
participant Report as report.md
GH->>Runner: start bench job (linux/mac)
Runner->>Cargo: run seqlock bench
Cargo->>Art: emit target/bench-report-seqlock.json
Runner->>GH: upload bench artifact
GH->>Script: invoke bench-report.sh with linux/mac JSONs
Script->>Art: read spsc/fixed/(optional) seqlock JSONs
Script->>Report: merge JSONs and render grouped sections
Script->>GH: output consolidated report.md
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/ci.yml (1)
126-126: Consider adding Miri coverage formantis-seqlock.The Miri job currently only tests
mantis-queue. Sincemantis-seqlockalso contains unsafe code in itsraw/module (sequence lock with atomics and volatile reads), it would benefit from Miri's strict provenance and alignment checks.Suggested addition to Miri job
- - run: cargo +nightly miri test -p mantis-queue --all-features + - run: cargo +nightly miri test -p mantis-queue -p mantis-seqlock --all-features🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 126, Miri job only runs "cargo +nightly miri test -p mantis-queue --all-features" and omits mantis-seqlock; update that command to include mantis-seqlock (e.g., add "-p mantis-seqlock" or run miri across the workspace) so the Miri step also tests the unsafe code in the mantis-seqlock raw/ module.crates/verify/src/lib.rs (1)
9-10: Consider keeping verification modules private unless externally consumed.Making these modules public expands the crate API surface. If they are only internal harnesses, prefer
modto avoid unnecessary public-contract maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/verify/src/lib.rs` around lines 9 - 10, The two modules declared as public, seqlock_proofs and seqlock_props, expand the crate's public API but appear to be internal; change their declarations from `pub mod seqlock_proofs;` and `pub mod seqlock_props;` to private `mod seqlock_proofs;` and `mod seqlock_props;`, and then run a workspace-wide build/test to ensure nothing external depends on them—if other crates or tests need access, keep `pub` only where actually consumed and consider re-exporting a stable API instead of exposing internal harness modules.crates/seqlock/src/raw/seqlock.rs (2)
29-33: Consider using a newtype for the sequence counter.The sequence counter uses raw
AtomicUsizerather than a dedicated newtype likeSeqNum(usize). A newtype would:
- Make the even/odd semantics explicit in the type system
- Prevent accidental misuse of raw sequence values
- Align with the coding guidelines for this repository
This would also apply to the
version()return type at line 88.Example newtype approach
/// Sequence number for SeqLock. Even = consistent, odd = write in progress. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(transparent)] pub struct SeqNum(usize); impl SeqNum { #[inline] pub const fn is_even(self) -> bool { self.0 & 1 == 0 } #[inline] pub const fn as_usize(self) -> usize { self.0 } }Based on learnings: "Use newtypes over primitives (e.g.,
SeqNum(u64)instead of rawu64)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/seqlock/src/raw/seqlock.rs` around lines 29 - 33, Introduce a SeqNum newtype and replace the raw AtomicUsize usage in SeqLock so the sequence counter is strongly typed: create a SeqNum(usize) (repr(transparent)) with helpers like is_even() and as_usize(), change the seq field from CachePadded<AtomicUsize> to CachePadded<Atomic<SeqNum>> (or keep AtomicUsize but wrap loads/stores via SeqNum conversions) and update all uses accordingly; update the SeqLock struct declaration (seq field) and the version() method return type to return SeqNum instead of usize, and ensure all comparisons and bit tests use SeqNum.is_even()/as_usize() helpers to preserve even/odd semantics.
107-111: Enhance SAFETY comment to include failure mode.The SAFETY comment documents the invariant but could be more complete per repository guidelines. Consider adding the explicit failure mode:
- // SAFETY: Single-writer guaranteed by &mut self. No concurrent writes. - // UnsafeCell allows interior mutation. MaybeUninit accepts any bit pattern. + // SAFETY: + // - Invariant: Single-writer access guaranteed by &mut self borrow. + // - Guarantee: UnsafeCell allows interior mutation; MaybeUninit accepts any T. + // - Failure mode: If called concurrently (impossible via borrow checker), + // data races would cause UB.As per coding guidelines: "Every
unsafeblock must include a// SAFETY:comment documenting the invariant, guarantee, and failure mode".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/seqlock/src/raw/seqlock.rs` around lines 107 - 111, Update the existing SAFETY comment for the unsafe block that writes into self.data (the block containing core::ptr::write(self.data.get().cast::<T>(), val)) to explicitly state the invariant, the guarantee, and the failure mode: document that a single-writer is guaranteed by &mut self, UnsafeCell permits interior mutation, MaybeUninit accepts any bit pattern, and if these invariants are violated it can lead to data races, undefined behavior, memory corruption, or UB (e.g., concurrent writers or invalid drop of uninitialized memory); include a brief sentence describing these failure consequences so the comment follows the repository guideline requiring invariant, guarantee, and failure mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/bench-report.sh:
- Around line 285-299: The seqlock block is calling render_fixed_platform which
produces fixed-point-specific fallback text; change these calls to a generic
renderer (or extend render_fixed_platform to accept a suite label) so the
heading and fallback copy are correct for seqlock. Specifically, replace the
calls using linux_seqlock and macos_seqlock (currently render_fixed_platform
"$linux_seqlock" "Linux" and render_fixed_platform "$macos_seqlock" "macOS")
with a generic function like render_suite_platform (or add a new parameter to
render_fixed_platform) and pass the suite name "Seqlock" so the renderer prints
neutral/fitting fallback text under the seqlock heading.
In `@crates/seqlock/src/raw/seqlock.rs`:
- Around line 152-168: The code calls MaybeUninit::assume_init() unconditionally
after CopyPolicy::copy_out, which can produce UB if a concurrent writer produced
a torn/invalid bit pattern for T; change the read path in SeqLock so that you
copy bytes into a MaybeUninit buffer (using C::copy_out) but do not call
assume_init() until after validating seq1 == seq2 (i.e., after
seqlock_read_barrier() and reloading seq2); only then convert the initialized
bytes to T. Alternatively, if you cannot defer initialization, tighten the
generic bounds on SeqLock/T (e.g., require a marker trait like
“AllBitsValid”/Pod/Zeroable) so the implementation is only used for types where
every bit pattern is valid.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 126: Miri job only runs "cargo +nightly miri test -p mantis-queue
--all-features" and omits mantis-seqlock; update that command to include
mantis-seqlock (e.g., add "-p mantis-seqlock" or run miri across the workspace)
so the Miri step also tests the unsafe code in the mantis-seqlock raw/ module.
In `@crates/seqlock/src/raw/seqlock.rs`:
- Around line 29-33: Introduce a SeqNum newtype and replace the raw AtomicUsize
usage in SeqLock so the sequence counter is strongly typed: create a
SeqNum(usize) (repr(transparent)) with helpers like is_even() and as_usize(),
change the seq field from CachePadded<AtomicUsize> to
CachePadded<Atomic<SeqNum>> (or keep AtomicUsize but wrap loads/stores via
SeqNum conversions) and update all uses accordingly; update the SeqLock struct
declaration (seq field) and the version() method return type to return SeqNum
instead of usize, and ensure all comparisons and bit tests use
SeqNum.is_even()/as_usize() helpers to preserve even/odd semantics.
- Around line 107-111: Update the existing SAFETY comment for the unsafe block
that writes into self.data (the block containing
core::ptr::write(self.data.get().cast::<T>(), val)) to explicitly state the
invariant, the guarantee, and the failure mode: document that a single-writer is
guaranteed by &mut self, UnsafeCell permits interior mutation, MaybeUninit
accepts any bit pattern, and if these invariants are violated it can lead to
data races, undefined behavior, memory corruption, or UB (e.g., concurrent
writers or invalid drop of uninitialized memory); include a brief sentence
describing these failure consequences so the comment follows the repository
guideline requiring invariant, guarantee, and failure mode.
In `@crates/verify/src/lib.rs`:
- Around line 9-10: The two modules declared as public, seqlock_proofs and
seqlock_props, expand the crate's public API but appear to be internal; change
their declarations from `pub mod seqlock_proofs;` and `pub mod seqlock_props;`
to private `mod seqlock_proofs;` and `mod seqlock_props;`, and then run a
workspace-wide build/test to ensure nothing external depends on them—if other
crates or tests need access, keep `pub` only where actually consumed and
consider re-exporting a stable API instead of exposing internal harness modules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 731be26b-96b5-41bb-a67f-9a8528bcc03a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.github/scripts/bench-report.sh.github/workflows/bench.yml.github/workflows/ci.ymlCLAUDE.mdCargo.tomlREADME.mdcrates/bench/Cargo.tomlcrates/bench/benches/seqlock.rscrates/bench/build.rscrates/bench/cpp/rigtorp_seqlock.hcrates/bench/cpp/seqlock_bench_contender.cppcrates/bench/src/lib.rscrates/bench/src/seqlock_ffi.rscrates/layout/Cargo.tomlcrates/layout/src/lib.rscrates/seqlock/Cargo.tomlcrates/seqlock/src/lib.rscrates/seqlock/src/raw/mod.rscrates/seqlock/src/raw/seqlock.rscrates/seqlock/tests/torn_read.rscrates/verify/Cargo.tomlcrates/verify/src/lib.rscrates/verify/src/seqlock_proofs.rscrates/verify/src/seqlock_props.rsdocs/PROGRESS.md
…ume_init safety docs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/bench-report.sh (1)
8-13: Prefer named flags to avoid ambiguous optional positional pairs.The new positional contract is brittle: a 4-arg call is ambiguous (seqlock-only vs fixed-only), which can silently route data to the wrong section. Consider switching to explicit flags (
--linux-seqlock,--linux-fixed, etc.) for forward-safe parsing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/bench-report.sh around lines 8 - 13, The script currently relies on brittle positional arguments (linux_json, macos_json, linux_seqlock, macos_seqlock, linux_fixed, macos_fixed) which makes 4-arg calls ambiguous; change argument parsing to accept two required positional args for linux_json and macos_json and parse all optional inputs via named flags (e.g., --linux-seqlock, --macos-seqlock, --linux-fixed, --macos-fixed) using a getopt/getopts or while-shift loop, assign values to the existing variables (linux_seqlock, macos_seqlock, linux_fixed, macos_fixed), validate required args and print the usage string when missing/invalid flags, and preserve current behavior downstream by only setting those variables when their flags are provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/scripts/bench-report.sh:
- Around line 8-13: The script currently relies on brittle positional arguments
(linux_json, macos_json, linux_seqlock, macos_seqlock, linux_fixed, macos_fixed)
which makes 4-arg calls ambiguous; change argument parsing to accept two
required positional args for linux_json and macos_json and parse all optional
inputs via named flags (e.g., --linux-seqlock, --macos-seqlock, --linux-fixed,
--macos-fixed) using a getopt/getopts or while-shift loop, assign values to the
existing variables (linux_seqlock, macos_seqlock, linux_fixed, macos_fixed),
validate required args and print the usage string when missing/invalid flags,
and preserve current behavior downstream by only setting those variables when
their flags are provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11e66f57-fb6f-43d4-8225-f1877cbaced0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/scripts/bench-report.sh.github/workflows/ci.ymlCargo.tomlcrates/seqlock/src/raw/seqlock.rs
✅ Files skipped from review due to trivial changes (2)
- Cargo.toml
- .github/workflows/ci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/seqlock/src/raw/seqlock.rs
Benchmark ReportCommit: LinuxCPU: Latency (ns/op, lower is better)Single Push+Pop
Burst 100
Burst 1000
Batch 100
Batch 1000
Full Drain
Instructions per Op (lower is better)Full results (all fields)
macOSCPU: Latency (ns/op, lower is better)Single Push+Pop
Burst 100
Burst 1000
Batch 100
Batch 1000
Full Drain
Instructions per Op (lower is better)Full results (all fields)
Sequence Lock (mantis-seqlock)LinuxCPU: checked_add
checked_div
checked_mul_trunc
display
mul_round_vs_trunc
parse
rescale
macOSCPU: checked_add
checked_div
checked_mul_trunc
display
mul_round_vs_trunc
parse
rescale
Fixed-Point Arithmetic (mantis-fixed)LinuxCPU: checked_add
checked_div
checked_mul_trunc
display
mul_round_vs_trunc
parse
rescale
macOSCPU: checked_add
checked_div
checked_mul_trunc
display
mul_round_vs_trunc
parse
rescale
|
…cross-contamination
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/criterion-to-json.sh:
- Around line 27-33: The prefix glob in the first for-loop misses the common
two-level layout causing matched_ests to remain empty and trigger the broad
fallback; update the glob list in that loop to include the pattern
"${bench_name}*/*/new/estimates.json" (in addition to the existing
"${bench_name}*/new/estimates.json" and "${bench_name}*/*/*/new/estimates.json")
so the loop that appends to matched_ests correctly captures two-level paths
before the fallback; ensure you modify the for-loop that iterates over est and
references matched_ests and bench_name to include the new pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d25a4151-ff02-4074-b291-cba39659be8f
📒 Files selected for processing (1)
.github/scripts/criterion-to-json.sh
Comment on lines
+27
to
+33
| for est in "$criterion_dir"/${bench_name}*/new/estimates.json "$criterion_dir"/${bench_name}*/*/*/new/estimates.json; do | ||
| matched_ests+=("$est") | ||
| done | ||
| # Fallback: if no prefix-matched results, read everything (backwards compatibility) | ||
| if [ ${#matched_ests[@]} -eq 0 ]; then | ||
| for est in "$criterion_dir"/*/new/estimates.json "$criterion_dir"/*/*/new/estimates.json; do | ||
| matched_ests+=("$est") |
There was a problem hiding this comment.
Prefix glob misses Criterion’s two-level path and can silently fall back to broad matching.
At Line 27, the prefix glob includes .../${bench_name}*/*/*/new/estimates.json but not the common .../${bench_name}*/*/new/estimates.json layout (which your own examples at Lines 42-43 use). That can cause matched_ests to stay empty and trigger the broad fallback at Line 31, reintroducing mixed-result risk.
Suggested fix
shopt -s nullglob
matched_ests=()
-for est in "$criterion_dir"/${bench_name}*/new/estimates.json "$criterion_dir"/${bench_name}*/*/*/new/estimates.json; do
+for est in \
+ "$criterion_dir"/"${bench_name}"*/new/estimates.json \
+ "$criterion_dir"/"${bench_name}"*/*/new/estimates.json \
+ "$criterion_dir"/"${bench_name}"*/*/*/new/estimates.json; do
matched_ests+=("$est")
done
# Fallback: if no prefix-matched results, read everything (backwards compatibility)
if [ ${`#matched_ests`[@]} -eq 0 ]; then
+ echo "No prefix-matched Criterion results for '${bench_name}', falling back to all estimates." >&2
for est in "$criterion_dir"/*/new/estimates.json "$criterion_dir"/*/*/new/estimates.json; do
matched_ests+=("$est")
done
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for est in "$criterion_dir"/${bench_name}*/new/estimates.json "$criterion_dir"/${bench_name}*/*/*/new/estimates.json; do | |
| matched_ests+=("$est") | |
| done | |
| # Fallback: if no prefix-matched results, read everything (backwards compatibility) | |
| if [ ${#matched_ests[@]} -eq 0 ]; then | |
| for est in "$criterion_dir"/*/new/estimates.json "$criterion_dir"/*/*/new/estimates.json; do | |
| matched_ests+=("$est") | |
| for est in \ | |
| "$criterion_dir"/"${bench_name}"*/new/estimates.json \ | |
| "$criterion_dir"/"${bench_name}"*/*/new/estimates.json \ | |
| "$criterion_dir"/"${bench_name}"*/*/*/new/estimates.json; do | |
| matched_ests+=("$est") | |
| done | |
| # Fallback: if no prefix-matched results, read everything (backwards compatibility) | |
| if [ ${`#matched_ests`[@]} -eq 0 ]; then | |
| echo "No prefix-matched Criterion results for '${bench_name}', falling back to all estimates." >&2 | |
| for est in "$criterion_dir"/*/new/estimates.json "$criterion_dir"/*/*/new/estimates.json; do | |
| matched_ests+=("$est") | |
| done | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 27-27: Quote expansions in this for loop glob to prevent wordsplitting, e.g. "$dir"/*.txt .
(SC2231)
[info] 27-27: Quote expansions in this for loop glob to prevent wordsplitting, e.g. "$dir"/*.txt .
(SC2231)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/criterion-to-json.sh around lines 27 - 33, The prefix glob
in the first for-loop misses the common two-level layout causing matched_ests to
remain empty and trigger the broad fallback; update the glob list in that loop
to include the pattern "${bench_name}*/*/new/estimates.json" (in addition to the
existing "${bench_name}*/new/estimates.json" and
"${bench_name}*/*/*/new/estimates.json") so the loop that appends to
matched_ests correctly captures two-level paths before the fallback; ensure you
modify the for-loop that iterates over est and references matched_ests and
bench_name to include the new pattern.
This was referenced Apr 5, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
mantis-seqlockcrate: lock-free single-writer multiple-reader sequence lockmantis-queuepatterns:no_std,#![deny(unsafe_code)]on root, unsafe inraw/submodule,CopyPolicystrategy patternBenchmark Results (Apple M4 Pro, ARM64)
Faster than rigtorp (C++ HFT gold standard) on all reads while being portable-correct on ARM64.
What's included
SeqLock<T, C>withT: Copy,C: CopyPolicy<T>(default + SIMD presets)store(&mut self, val)— single-writer enforced by borrow checkerload(&self) -> T— lock-free reader with retry on contentionversion(&self) -> usize— sequence number for change detectionmantis-layoutmantis-verifyTest plan
cargo +nightly test -p mantis-seqlock --lib— 9 unit testscargo +nightly test -p mantis-seqlock --test torn_read— multi-threaded correctnesscargo +nightly test -p mantis-seqlock --no-default-features— no_stdcargo +nightly clippy -p mantis-seqlock -- -D warnings— cleancargo +nightly bench --bench seqlock— benchmarks run🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Benchmarks
Tests
CI
Documentation