Skip to content

fix: index_stream tail loop ignored QUERY_REMAP - #39

Merged
RagnarGrootKoerkamp merged 2 commits into
RagnarGrootKoerkamp:masterfrom
ilgrad:fix-query-remap-tail
Aug 28, 2026
Merged

RagnarGrootKoerkamp merged 2 commits into
RagnarGrootKoerkamp:masterfrom
ilgrad:fix-query-remap-tail

Conversation

@ilgrad

@ilgrad ilgrad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

The prefetch-ring flush at the end of index_stream_maybe_remap tests REMAP instead of
QUERY_REMAP, so on a minimal PHF index_stream_maybe_remap::<B, false, _> remaps the last B
keys and disagrees with index_no_remap there. index_stream::<B, _> is unaffected — it passes
QUERY_REMAP = REMAP.

Repro on 2.1.1 and on master: 64 mismatches over 200 builds, all at key index >= n - B. Zero with
this patch.

use ptr_hash::{PtrHash, PtrHashParams};

type Mph = PtrHash<u64, ptr_hash::bucket_fn::CubicEps, Vec<u32>,
                   ptr_hash::hash::StrongerIntHash, Vec<u8>, false, true>;

fn main() {
    let mut bad = 0;
    for seed in 0..200u64 {
        let keys: Vec<u64> = (0..1000u64)
            .map(|i| (i + seed * 1000).wrapping_mul(0x9e37_79b9_7f4a_7c15) ^ 0x1234_5678)
            .collect();
        let f = Mph::new(&keys, PtrHashParams::default_compact());
        let mut stream = Vec::with_capacity(keys.len());
        f.index_stream_maybe_remap::<32, false, _>(keys.iter()).for_each(|s| stream.push(s));
        bad += (0..keys.len()).filter(|&i| stream[i] != f.index_no_remap(&keys[i])).count();
    }
    println!("mismatches = {bad}");
}

Second commit drops a leftover eprintln! in max_index_no_remap().

cargo test --lib --release passes.

ilgrad added 2 commits August 28, 2026 10:55
The prefetch-ring flush tested REMAP instead of QUERY_REMAP, so
index_stream_maybe_remap::<B, false, _> remapped the last B keys on a
minimal PHF and disagreed with index_no_remap there.
@RagnarGrootKoerkamp

Copy link
Copy Markdown
Owner

thanks for the fix!

@RagnarGrootKoerkamp
RagnarGrootKoerkamp merged commit 2ecbab8 into RagnarGrootKoerkamp:master Aug 28, 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