feat(dispatch): bloom dispatch gates (any-window/bloomfw/exact) + decoupled routing-pad - #57
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds optional ChangesOptional bloom gate
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi/src/sidecar/bloom_file.rs`:
- Around line 125-129: The Bloom file sizing path in `BloomFile::body_bytes` /
mmap setup must reject `num_bits` that cannot fit in `usize` before allocating
or setting the file length. Add the same guard used by `validate_blm_header()`
by converting with `usize::try_from` and using `checked_add` when computing the
total size, then fail closed before `set_len()` and any probe-write loop so the
header and mapped body stay consistent.
- Around line 327-335: Add a local bounds check in the `contains` method of
`BloomFile` right before dereferencing `self.data_ptr` so the unsafe read is
self-auditing. Keep the existing `probe`/`byte` logic, but insert a
`debug_assert!` for `byte` immediately before `unsafe { *self.data_ptr.add(byte)
}`, and ensure the adjacent SAFETY comment still matches the invariant used by
the other unsafe reads in this sidecar module.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d14aa78-4bc3-40f6-abcd-8042b735547a
📒 Files selected for processing (15)
prmi-sys/src/lib.rsprmi/examples/z_gate_misroute.rsprmi/src/cli.rsprmi/src/index/collect.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/bloom_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mask.rsprmi/src/train/mod.rsprmi/tests/shm_loader.rsprmi/tests/z_keep_mask.rs
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
prmi/src/index/collect.rs (1)
355-363: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse checked window bounds before slicing caller reads.
These public gates drift from
present_anchor’s fail-closed pattern:start + Kis used in the loop guard and slice bounds. Mirror thechecked_add(K)loop so malformed/edge caller inputs cannot panic or wrap before slicing.Minimal pattern to apply to each gate loop
- 'windows: while start + K <= read.len() { + 'windows: loop { + let Some(end) = start.checked_add(K) else { + return false; + }; + if end > read.len() { + return false; + } // Reject a window with any N by jumping past the offending base. for j in 0..K { if read[start + j] >= 4 { start += j + 1; continue 'windows; } } - let key = crate::encoding::tokenize_32mer(&read[start..start + K], K); + let key = crate::encoding::tokenize_32mer(&read[start..end], K);As per path instructions, public index primitives that slice caller-supplied buffers must fail closed, and
pivot + lenarithmetic must be checked.Also applies to: 397-410, 442-455, 474-482
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi/src/index/collect.rs` around lines 355 - 363, The gate loops in collect.rs use unchecked start + K arithmetic before slicing caller-supplied reads, which can panic or wrap on malformed inputs. Update each affected gate to follow the same fail-closed pattern as present_anchor by using checked_add for the loop bound and slice end, then only slicing after the bounds are verified. Apply this consistently in the gate logic around the mem_search path and the other referenced gate loops so public index primitives never index with unchecked pivot + len arithmetic.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi-sys/src/lib.rs`:
- Around line 2422-2423: Update the documentation for each exported extern "C"
gate function so the return-code contract is exhaustive instead of saying only
“<0 error”; in the doc comments for the affected entrypoints (including the ones
mirrored from prmi_present and the other ABI gates mentioned), explicitly list
-1 for null/invalid pointer, -2 for invalid length/range/PAC metadata, and -3
for internal panic, while keeping the existing 0/1 meanings unchanged.
- Around line 2438-2479: The C ABI empty-slice handling is too strict in
prmi_present_bloom and present_gate_first_window because they reject pac == NULL
before allowing the canonical zero-length PAC case. Update the null-pointer
validation so NULL is accepted only when pac_num_bases is zero, and build an
empty slice for that case instead of calling slice::from_raw_parts on a null
pointer. Keep the existing length and l_pac consistency checks, and apply the
same pattern in both entrypoints.
In `@prmi/src/cli.rs`:
- Around line 375-380: The bloom false-positive rate validation in the CLI
accepts 0.0 even though the error message says the value must be in (0, 1).
Update the guard in the CLI parsing logic around the bloom options to use an
explicit open-interval check so `bloom_fp_rate` must be strictly greater than
0.0 and strictly less than 1.0, and keep the existing `InvalidInput` error path
consistent with `BloomParams::for_keys`.
In `@prmi/src/index/mod.rs`:
- Around line 91-95: The `.blm` bloom gate is being used as a best-effort
sidecar without being tied to the loaded index, so stale or mismatched bloom
data can cause false negatives in `present_anchor_bloom` and `bloom_first`.
Update `load_bloom_best_effort` and the call sites in `mod.rs` to require a
reference binding to the current index, such as a stored digest or SA identity
matching the `.kmt` flow, and ignore any `.blm` that does not validate against
the loaded index before it is used.
In `@prmi/src/index/shm.rs`:
- Around line 425-427: The `.blm` offset/length parsing in the shared-blob
layout code is narrowing on-disk u64 values with `as usize` before validation,
which can silently truncate invalid inputs. Update the `.blm` field handling in
the `index/shm.rs` parsing path to use `usize::try_from(...)` for both
`blm_offset` and `blm_len`, and if either conversion overflows, return
`SizeMismatch` before any layout checks or slice use.
In `@prmi/src/sidecar/bloom_file.rs`:
- Around line 147-154: The bloom filter probing path is recomputing the per-key
hashes inside probe() for every index, which is unnecessary work. Move the
splitmix64(key) and splitmix64(h1) computation out of the inner probe loop and
compute h1/h2 once per key in the writer and reader flows, then pass or reuse
those values for all subsequent probes. Update the probe-related call sites and
the probe() helper so the hashing work is done once per key rather than once per
i.
In `@prmi/tests/z_keep_mask.rs`:
- Around line 324-328: The gate sweep assertions are comparing new primitives
against other production gates, which only gives self-consistency and can hide
shared bugs. In the affected test blocks in z_keep_mask.rs, replace the
comparisons to present_anchor_any and present_anchor with an independent oracle
based on the mem_search logic already used in
present_anchor_any_recovers_boundary_read. Keep the new sweep checks for
present_anchor_bloom and the exact gate, but assert them against that oracle
instead of another gate implementation.
- Around line 536-539: The negative check in the bloom-first test is too strong
because it asserts a probabilistic Bloom miss; update the test around
present_anchor_bloom_first to verify the exact first-window gate instead, while
keeping the padded bloom-positive assertion. Use the existing
zt.present_anchor_bloom_first and the exact first-window matching logic in this
test to ensure the failure condition is deterministic and not dependent on Bloom
false positives.
---
Outside diff comments:
In `@prmi/src/index/collect.rs`:
- Around line 355-363: The gate loops in collect.rs use unchecked start + K
arithmetic before slicing caller-supplied reads, which can panic or wrap on
malformed inputs. Update each affected gate to follow the same fail-closed
pattern as present_anchor by using checked_add for the loop bound and slice end,
then only slicing after the bounds are verified. Apply this consistently in the
gate logic around the mem_search path and the other referenced gate loops so
public index primitives never index with unchecked pivot + len arithmetic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0cfe291b-6753-4018-a98b-6e95a020fdb6
📒 Files selected for processing (15)
prmi-sys/src/lib.rsprmi/examples/z_gate_misroute.rsprmi/src/cli.rsprmi/src/index/collect.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/bloom_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mask.rsprmi/src/train/mod.rsprmi/tests/shm_loader.rsprmi/tests/z_keep_mask.rs
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi-sys/src/lib.rs`:
- Around line 2437-2440: The Safety docs for the exported FFI gates do not
explicitly state that an empty PAC is allowed when pac == NULL and pac_num_bases
== 0, even though the implementation accepts it. Update the Safety section on
each affected exported gate (including the relevant functions around
prmi_present and the other listed exports) to clearly document this nullability
rule for C callers, alongside the existing read pointer NULL case and the handle
validity requirements.
In `@prmi/src/index/collect.rs`:
- Line 370: In the collect/search gate around mem_search, avoid casting
match_len to usize before comparing against K; keep the comparison in u64 space
like present_anchor so the search-core contract does not rely on narrowing
conversions. Update the gate in the collect logic that uses
self.mem_search(...).match_len and any related check at the other flagged site
to compare the raw match length directly, only converting after the gate if
absolutely necessary.
In `@prmi/src/index/shm.rs`:
- Around line 234-238: The `.blm` offset and padding arithmetic in the shm
layout builder can overflow before alignment, which may produce a malformed
layout. Update the logic around `blm_offset`, `write_padding`, and the
`prev_end`/component end calculations to use checked addition instead of
პირდაპირ `+` arithmetic, and return `SizeMismatch` if any sum would overflow.
Keep the fix localized to the shm packing code that computes component offsets
so the `.blm` placement remains based on validated ends.
In `@prmi/src/sidecar/bloom_file.rs`:
- Around line 57-59: The current `.blm` binding in `BloomFile` only validates
reference content and `sa_num`, so a stale bloom built for a different keep-set
can still load. Update the `bloom_file` format and the load-time validation path
to persist and check a routing/key-set digest (or equivalent keep-set identity)
alongside `ref_digest` and `sa_num`, and reject blooms whose key-set identity
does not match the current routing set. Also update the related
serialization/deserialization and any schema/docs/tests near the referenced
`BloomFile` handling and the other affected locations.
In `@prmi/src/sidecar/magic.rs`:
- Around line 29-31: Add a file-backed plus shm/open round-trip test for the new
.blm sidecar using MemoryMode::Mode1, since only Mode2 is currently covered and
the BLM_MAGIC on-disk format change is not verified across all memory modes.
Update the relevant tests in z_keep_mask and/or shm_loader to exercise building
the bloom sidecar, reopening it from disk, and loading it through shm/open in
Mode1 so the new header/layout is round-tripped end to end.
In `@prmi/src/train/mod.rs`:
- Around line 445-450: The bloom-building path in the training flow currently
materializes all routing keys into a Vec<u64>, which can create a large extra
allocation; update the logic around the SA iteration and Bloom writer setup to
stream or chunk keys directly into the writer instead of collecting them first.
If a full materialization is still needed somewhere, add an early guard before
the allocation to reject oversized routing key sets, and keep the fix localized
to the bloom insert path in prmi::train::mod.rs near the existing
keep_routing_pos/full_key_for_position_2x flow.
In `@prmi/tests/z_keep_mask.rs`:
- Around line 145-192: The two oracle helpers are still mirroring the gate
traversal logic, so they can hide the same off-by-one or N-skip bug; rewrite the
traversal in any_window_present_oracle and first_window_present_oracle to use an
independent window-based scan instead of manually advancing start indices. Keep
the comparison against LearnedIndex::mem_search, but drive it from a distinct
test-only path using windows(K) so the oracles validate present_anchor_any,
present_anchor_bloom, present_anchor, and present_anchor_exact without sharing
their scan logic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 208cc6b2-36bd-497e-82f0-de0bc6cccfbd
📒 Files selected for processing (15)
prmi-sys/src/lib.rsprmi/examples/z_gate_misroute.rsprmi/src/cli.rsprmi/src/index/collect.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/bloom_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mask.rsprmi/src/train/mod.rsprmi/tests/shm_loader.rsprmi/tests/z_keep_mask.rs
…g-pad Add the .blm bloom dispatch sidecar and the Design-Z cheap routing gates. - .blm bloom over the keep-set 32-mers (bloom_file.rs, BLM_MAGIC, SidecarPaths.bloom; `prmi build --with-bloom`/`--bloom-fp-rate`) with the any-window `present_anchor_bloom` gate (FFI prmi_present_bloom). - Lever 2 cheap first-window gates: `present_anchor_bloom_first` (in-memory bloom probe) and `present_anchor_exact` (`kmer_exists`: same verdict as `mem_search().match_len>=32`, skips interval recovery). FFI prmi_present_bloom_first / prmi_present_exact. The .blm is carried in the shm blob (6th component) so the gate survives a shared-index deployment. - Lever 3 build flag: `prmi build --routing-pad N` builds the bloom over a padded keep-set (`pad_and_merge`) while the seeding .sa stays tight. Tests: z_keep_mask (bloom-first no-false-negatives, exact equivalence, routing-pad flank coverage + .sa-unchanged), shm_loader (.blm round-trip + corrupt fallback). clippy clean.
Newer nightly rustfmt (unpinned CI fmt toolchain) reformats these; apply it so the fmt check passes. Formatting only.
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
prmi-sys/src/lib.rs (1)
2621-2622: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake each exported Safety block self-contained.
Replace
See prmi_presentwith the explicithandle/read/pacnullability contract, includingpac == NULLiffpac_num_bases == 0.Doc fix
/// # Safety -/// See [`prmi_present`]. +/// `handle` must come from [`prmi_open`]/[`prmi_open_shm`]. `read` must be valid +/// for `read_len` bytes (or NULL iff `read_len == 0`); `pac` must be valid for +/// `ceil(pac_num_bases / 4)` bytes, or NULL iff `pac_num_bases == 0` (the +/// canonical empty PAC, which maps to an empty slice).As per path instructions, FFI docs must be exhaustive about which pointer args may be null for
prmi-sys/src/**/*.rs.Also applies to: 2659-2660
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi-sys/src/lib.rs` around lines 2621 - 2622, The exported Safety docs for the PRMI FFI wrappers are too indirect and should be self-contained. Update the Safety block on the `prmi_present`-related export to explicitly document the nullability contract for `handle`, `read`, and `pac`, including that `pac == NULL` iff `pac_num_bases == 0`. Mirror this same exhaustive pointer-argument documentation in the other affected exported Safety block so each public FFI doc stands alone without referring to another function.Source: Path instructions
prmi/tests/z_keep_mask.rs (1)
426-446: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the independent first-window oracle here too.
present_anchorshares the production gate traversal, so this still misses shared N-skip/off-by-one bugs in the newbloom_firstchecks.Oracle swap
- let exact = zb.present_anchor(read, &bases, e); + let exact = first_window_present_oracle(&zb, read, &bases, e); let bloom = zb.present_anchor_bloom_first(read, &bases, e); @@ assert_eq!( zn.present_anchor_bloom_first(read, &bases, e), - zn.present_anchor(read, &bases, e), + first_window_present_oracle(&zn, read, &bases, e), "no-.blm bloom_first gate must equal the first-window verdict (read start {start})" );As per path instructions, new search primitives need an independent oracle, not a self-consistency check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi/tests/z_keep_mask.rs` around lines 426 - 446, The no-.blm assertion is still using `zn.present_anchor`, which follows the same production gate traversal as `present_anchor_bloom_first` and therefore is only a self-consistency check. Replace that comparison with the independent first-window oracle already used elsewhere in this test, so the `zn.present_anchor_bloom_first` verdict is validated against a separate reference path rather than `present_anchor`. Keep the existing `zb.present_anchor`/`zb.present_anchor_bloom_first` coverage, but ensure the oracle for the no-.blm case is independent of the production traversal.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@prmi-sys/src/lib.rs`:
- Around line 2621-2622: The exported Safety docs for the PRMI FFI wrappers are
too indirect and should be self-contained. Update the Safety block on the
`prmi_present`-related export to explicitly document the nullability contract
for `handle`, `read`, and `pac`, including that `pac == NULL` iff `pac_num_bases
== 0`. Mirror this same exhaustive pointer-argument documentation in the other
affected exported Safety block so each public FFI doc stands alone without
referring to another function.
In `@prmi/tests/z_keep_mask.rs`:
- Around line 426-446: The no-.blm assertion is still using `zn.present_anchor`,
which follows the same production gate traversal as `present_anchor_bloom_first`
and therefore is only a self-consistency check. Replace that comparison with the
independent first-window oracle already used elsewhere in this test, so the
`zn.present_anchor_bloom_first` verdict is validated against a separate
reference path rather than `present_anchor`. Keep the existing
`zb.present_anchor`/`zb.present_anchor_bloom_first` coverage, but ensure the
oracle for the no-.blm case is independent of the production traversal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 216e63f6-f137-4c5b-9f36-84b40b6ffada
📒 Files selected for processing (17)
prmi-sys/src/lib.rsprmi/examples/z_gate_misroute.rsprmi/src/cli.rsprmi/src/index/collect.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/bloom_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/meta.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mask.rsprmi/src/train/mod.rsprmi/tests/shm_loader.rsprmi/tests/sidecar_meta.rsprmi/tests/z_keep_mask.rs
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge.
nh13
added a commit
that referenced
this pull request
Jun 27, 2026
* perf(build): thin LTO + codegen-units=1 release profile and native bench harness Enables cross-crate inlining of the query hot path. target-cpu stays operator-driven via RUSTFLAGS (documented x86-64-v3 / Graviton floor); shipped artifacts remain portable. Records the post-profile bench baseline that subsequent perf PRs are measured against. * test(sa): gate debug_assert should_panic test behind cfg(debug_assertions) doubled_text_rejects_out_of_range_base asserts via debug_assert!, compiled out at opt-level=3, so it spuriously failed under `cargo test --release` (now run as a gate by the release-profile change). Gate the test on cfg(debug_assertions); the assertion semantics are unchanged in debug builds. Pre-existing latent issue, surfaced by adding the release-test gate. * perf(collect): memoize next-ambiguous-base distances per read fwd_qlen rescanned the read forward on every zigzag/reseed/pass-3 step (O(rlen^2) per read). Precompute next_n once per read in CollectScratch and read it O(1) from all three passes. Byte-identical (proptest: next_n[p] == fwd_qlen(read, p)). * perf(spectrum): hoist keyed-compare mask out of the boundary-probe loop ref_less/shares_prefix/lcp_at and the windowed model-locate closure recomputed keyed_compare_mask(query.len()) on every SA probe; the query slice is invariant for the whole find_boundary search, so compute (nbases, mask) once per search and pass it to compare_query_vs_suffix_2x_keyed_with_mask. Byte-identical (the _with_mask variant is proptest-equivalent to the recompute variant). * perf(batch): zip the backward lockstep arm, drop dead windowed guards, shrink lockstep key buffers A2: backward_spectrum_batch_impl mirrors the forward arm's zip (removes per-task bounds checks on the all_steps/tasks_s/out_ns triple). A3: write_fwd_task_steps and write_bwd_task_steps use copy_from_slice via the existing out_steps_as_smemstep helper instead of a field-by-field loop. A5: forward_boundary_windowed drops the always-true is_none_or guards on each bisection step (bisection monotonicity guarantees the most-recent probe is always the extreme; unconditional assignment is correct). A4: lockstep keyv Vec<Option<u64>> -> Vec<u64> + keys_present flag in all three lockstep loops (backward_spectrum_lockstep, forward_spectrum_lockstep, mem_search_lockstep); reduces per-element footprint from 16 B to 8 B. Byte-identical (lockstep/windowed equivalence oracles green, 253 tests passed). * perf(spectrum): amortize bounds checks in unpack_packed_forward's word loop The 32-base middle loop paid a try_into().unwrap() panic-branch and eight per-step out[] bounds checks (the i+4*j arithmetic defeats LLVM's proof from the i+32<=n guard). Drive it with chunks_exact_mut(32).zip(chunks_exact(8)) so bounds are checked once at the slice split with a known stride. Byte-identical (packed_word_decode + unaligned_word_step oracles green); no new unsafe. Add an inline before/after bench in primitives_bench.rs (unpack_packed_middle) that isolates the middle loop over a 1024-base / 256-corpus corpus. Measured ~35% wall-time reduction on aarch64 (19.5 µs → 12.5 µs, 13.4 → 20.9 Gelem/s); the real validation target is x86 (deferred EC2 batch). * perf(collect): drop dead stage-1 sort in sort_within_read The within-read sort ran two unstable sorts: stage 1 (m ASC, n DESC) then stage 2 (m ASC, n ASC). Stage 2 alone orders SMEMs with distinct (m, n); stage 1 could only reorder SMEMs sharing both m and n. Such a reorder is unobservable. Every emitted SMEM's (k, s) is mem_search(read[m..=n]), a deterministic function of the span content, so a fixed (m, n) determines a unique (k, s); with rid constant per read and l == 0 always, two entries sharing (m, n) are equal in all six fields. The multi-pass collection does emit (m, n) duplicates (a reseed or pass-3 round re-finding a pass-1 span), but each is a byte-identical copy, so permuting a tied group is the identity on the output bytes. Dropping stage 1 is therefore byte-identical to the C++ two-stage composition, including at the consumer box-gate (prmi's output bytes are unchanged). Add two regression guards: unsorted_mn_ties_are_byte_identical (a 4096-case proptest asserting every (m, n) tie in the unsorted set is byte-identical) and unsorted_mn_tie_census (a deterministic repeat-heavy corpus that finds 1440 tie-groups / 1764 duplicate entries across ~88k reads, all byte-identical). * perf(bloom): division-free Lemire reduction in bit_at Replace the per-probe `combined % num_bits` (true 64-bit div) with Lemire multiply-shift ((combined*num_bits)>>64), uniform into [0,num_bits) with no division; num_bits stays a multiple of 64 so sizing/header are unchanged. Writer and reader share bit_at so a self-consistent .blm has no false negatives (bloom_has_no_false_negatives green); FP rate stays near target (1.02%). Bench (1024 probes, num_bits=479296): before_mod: ~1.555 µs (~658 Melem/s) after_lemire: ~944 ns (~1084 Melem/s) ~1.65x throughput FORMAT NOTE: this changes which bits a key sets, so the .blm BODY bytes differ from a prior build (header layout unchanged; shared FORMAT_VERSION intentionally NOT bumped — it gates all sidecars). A .blm must be regenerated with a matching binary; a stale .blm yields benign bloom false negatives that the consumer's present-read fallback re-seeds (accelerator, not a correctness oracle). .blm body now diverges from #57's % reduction — flag for the #57 author at merge. * chore(perf): final-review touch-ups — batch-arm parity assert, non-vacuous census guard, clippy-clean test targets - backward_spectrum_batch_impl: debug_assert_eq!(all_steps.len(), tasks_s.len()) for parity with the forward arm (zip would otherwise truncate silently on a length mismatch). - unsorted_mn_tie_census: assert tie_groups > 0 so the (m,n)-tie byte-identity guard cannot pass vacuously. - #[allow] the deliberately production-mirroring loops in the new proptests/bench so clippy --all-targets -D warnings is clean; production code already has zero warnings.
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.
Design-Z stack 6/6 — base:
feat/z-tiered-isa.Bloom dispatch gates + routing-pad: the
.blmbloom sidecar (prmi build --with-bloom) and the gate variants —present_anchor_bloom(any-window + confirm), the cheap first-windowpresent_anchor_bloom_first(bloomfw) andpresent_anchor_exact(kmer_exists, byte-identical to the mem_search gate, skips interval recovery) — with FFI..blmcarried in the shm blob.--routing-pad Nbuilds the bloom over a padded keep-set while the seeding.sastays tight.Already adversarially reviewed (Levers 2/3) + dual-reviewed (coderabbit --agent + local CR) — no findings. green.
Summary by CodeRabbit
.blmbloom-filter dispatch gates for tiered presence gating, with new runtime strategies (any-window, bloom-first, exact first-window) and C-ABI entrypoints.prmi buildwith--with-bloom,--bloom-fp-rate, and--routing-pad, and enabled carrying.blmthrough shared-memory sidecars.kmer_existsfor exact membership checks and a new mis-route analysis example..blmintegrity/fallback.