perf(quadwt): elide bounds checks on per-level qvs indexing - #33
Open
eljeffeg wants to merge 11 commits into
Open
perf(quadwt): elide bounds checks on per-level qvs indexing#33eljeffeg wants to merge 11 commits into
qvs indexing#33eljeffeg wants to merge 11 commits into
Conversation
Foundation for PR rossanoventurini#2 zero-copy I/O: - src/bytes/{mod,error,util}.rs: LayoutError, LE/align/cast helpers, QWTB/HQWB magic constants (container encode/decode comes next) - QVector::from_raw_parts, SuperblockPlain::from_counters, RSSupportPlain::from_parts, RSQVector::from_parts - QWaveletTree::from_parts / HuffQWaveletTree::from_parts (reject WITH_PREFETCH_SUPPORT in v1) - Round-trip tests: disassemble via accessors → reassemble via from_parts → equal get/rank/select (plain + Huffman) No feature flags, no new dependencies. serde path unchanged.
Canonical little-endian container for plain QWaveletTree (magic QWTB): - 32-byte header + 128-byte level directory entries - 64-aligned DataLine / SuperblockPlain payloads - select samples and n_occs_smaller per level - empty-tree convention (n_levels=0, sentinel default qv) QWaveletTree::from_parts now takes explicit n_levels so empty trees round-trip cleanly. Owned from_bytes copies POD via copy_pod_slice so heap Vec<u8> need not be 64-byte aligned (zero-copy views still require alignment via cast_slice). Tests: get/rank/select round-trip, empty tree, bad magic, truncated. Full suite: 100 passed.
Canonical little-endian container for HuffQWaveletTree (magic HQWB): - 32-byte header with encode_len / decode_n_buckets - 136-byte level directory (QWTB 128 B + level_len u64) - 8-aligned encode LUT (content:u32, len:u32) and length-prefixed decode buckets (content:u32 + symbol:u64 zero-extended) - 64-aligned DataLine / SuperblockPlain payloads per level HuffQWaveletTree::from_parts normalizes empty-tree sentinels (n_levels=0 with default qv + lens=[0]). Tests: get/rank/select round-trip, empty tree, bad magic, large-σ uneven freqs. Full suite green.
Add borrowing views over QWTB/HQWB blobs that cast POD level payloads in place (DataLine, SuperblockPlain, select samples) and implement AccessUnsigned/RankUnsigned/SelectUnsigned matching the owned trees. - RSQVectorView: per-level get/rank/rank_all/select over borrowed slices - QwtView: plain QWTB open + wavelet-matrix queries - HqwtView: HQWB open; owns small Huffman LUTs, borrows level payloads - AlignedBytes: helper to 64-align heap blobs for cast_slice (mmap is naturally page-aligned) Requires a 64-byte-aligned base buffer; rejects Misaligned otherwise.
Make DataLine.words / SuperblockPlain.counters and owned-layout accessors (data_lines, superblocks, select_samples, n_occs_smaller, position_bits, qvector, rs_support) pub(crate). External mmap clients should use QwtView/HqwtView and RSQVectorView POD accessors instead.
Remove leaked references to the private PR2 design doc from public module docs. Add checked_region for untrusted header offset/size math on the QWTB/HQWB decode paths (and HqwtView), and make align_up overflow-safe.
Use is_multiple_of, iterator-style fill loops, size_of_val, and add missing # Safety docs so src/bytes is clean under clippy.
Fix remaining pre-existing clippy lints outside bytes/: empty-line-after-doc,
legacy numeric constants, needless_range_loop, len_without_is_empty, and
intentional inverted Range tests via std::ops::Range { start, end }.
Document the QWTB/HQWB owned and zero-copy APIs in README and on the public to_bytes/from_bytes entry points. Add PR2_BODY.md as draft PR text.
rank/get/select (and prefetch helpers) walked levels with self.qvs[level], paying a bounds check on every level. Range walkers already used get_unchecked; make the primary hot paths consistent. Safety: level is always < n_levels and qvs.len() == n_levels for non-empty trees (empty trees short-circuit before these loops).
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.
Note
This PR includes the changes from #31 & #32 (only two files were actually changed in this PR)
Summary
rank/get/select(and the related prefetch helpers) walked wavelet levels withself.qvs[level], paying a bounds check on every level. The range walkers (range_next_value,RangeDistinctIter,OccsRangeIter) already usedqvs.get_unchecked(...). This PR makes the primary hot paths consistent.Applies to both:
QWaveletTree(src/quadwt/mod.rs)HuffQWaveletTree(src/quadwt/huffqwt.rs)Motivation
These paths are on the critical query path for every access/rank/select. With typical alphabet depths of ~8–9 levels, each query paid multiple redundant slice bounds checks even though
levelis always< n_levelsandqvs.len() == n_levelsfor non-empty trees.Change
Replace bounds-checked
self.qvs[level]with: