Skip to content

perf(opt): fold BT-walk coordinate decode; drop dead hash-chain optimal path #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
polaz merged 6 commits into from
Jul 1, 2026
Merged

perf(opt): fold BT-walk coordinate decode; drop dead hash-chain optimal path #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bae47ce
refactor(opt): drop the non-upstream hash-chain optimal fallback
polaz Jun 30, 2026
17c5ec4
refactor(opt): drop the now-vestigial USE_BT_MATCHFINDER const generic
polaz Jun 30, 2026
faad79b
perf(opt): fold per-node coordinate decode in the BT walk
polaz Jun 30, 2026
e13b3ed
refactor(opt): enforce binary-tree-only contract on the optimal collect
polaz Jun 30, 2026
5ef5c04
test(opt): cover the optimal-collect dispatcher arms + BT-only contract
polaz Jun 30, 2026
8d3387f
test(opt): assert per-tag dispatch via the hash3 observable
polaz Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions zstd/src/encoding/hc/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,24 @@ macro_rules! bt_insert_and_collect_matches_body {
.wrapping_sub($table.index_shift)
.wrapping_sub(window_low);
let win_range = $abs_pos - window_low;
// Decode biases: fold the per-node coordinate conversions into
// loop-invariant additions. The gate-validated chain entry
// `match_stored` maps to its absolute position, its history-relative
// index and its BT pair slot by a single add each, instead of
// re-reading `position_base` / `index_shift` / `history_abs_start` /
// `bt_mask` from `self` and round-tripping `index_shift` through the
// slot computation on every node. Upstream zstd walks one
// window-relative `matchIndex` directly (`match = base + matchIndex`,
// slot `2*(matchIndex & btMask)`); these biases are the
// single-coordinate equivalent. Wrapping throughout: the window gate
// already proved `match_stored ∈ [window_low, abs_pos)` before decode,
// mirroring the `win_off` form above.
let abs_bias = $table
.position_base
.wrapping_sub(1)
.wrapping_sub($table.index_shift);
let idx_bias = abs_bias.wrapping_sub($table.history_abs_start);
let bt_bias = $table.position_base.wrapping_sub(1);
// Raw `+ 9` is safe here — see `bt_insert_step_no_rebase_body!`
// for the full discussion of the upstream `STREAM_ABS_HEADROOM`
// cap in `MatchTable::add_data`.
Expand Down Expand Up @@ -763,19 +781,21 @@ macro_rules! bt_insert_and_collect_matches_body {
while compares_left > 0 && (match_stored as usize).wrapping_add(win_off) < win_range {
compares_left -= 1;
// The condition proved this candidate is in `[window_low,
// abs_pos)`, so `match_stored >= 1` (HC_EMPTY is out of range) and
// the `- 1` cannot underflow; candidate_abs == base + match_stored.
let candidate_abs = ($table.position_base + (match_stored as usize - 1))
.wrapping_sub($table.index_shift);

let next_pair_idx = $table.bt_pair_index_for_abs(candidate_abs);
// abs_pos)`, so `match_stored >= 1` (HC_EMPTY is out of range).
// Decode via the precomputed biases (single add each), the
// single-coordinate form of upstream's `matchIndex`.
let stored = match_stored as usize;
let candidate_abs = stored.wrapping_add(abs_bias);
// `2*(candidate_abs + index_shift & bt_mask)` with `index_shift`
// folded away: `candidate_abs + index_shift == stored + bt_bias`.
let next_pair_idx = 2 * (stored.wrapping_add(bt_bias) & bt_mask);
// SAFETY: `next_pair_idx (+1)` = `2*(candidate_abs & bt_mask) (+1)`
// ≤ `chain_table.len()-1`; `chain_ptr` is the hoisted live base,
// table not realloc'd during the walk.
let next_smaller = unsafe { *chain_ptr.add(next_pair_idx) };
let next_larger = unsafe { *chain_ptr.add(next_pair_idx + 1) };
let seed_len = common_length_smaller.min(common_length_larger);
let candidate_idx = candidate_abs - $table.history_abs_start;
let candidate_idx = stored.wrapping_add(idx_bias);
// SAFETY: BT walk invariant — `candidate_idx + tail_limit ≤
// concat.len()`.
let match_len = unsafe { $cmf(concat, idx, candidate_idx, tail_limit, seed_len) };
Expand Down
35 changes: 0 additions & 35 deletions zstd/src/encoding/hc/hc_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,41 +21,6 @@ fn table_with_history(buf: &[u8]) -> MatchTable {
t
}

#[test]
fn chain_candidates_returns_sentinels_when_suffix_too_short() {
let hc = HcMatcher::new(2, 4, 32);
// History exactly at min-prefix - 1 → idx + 4 > concat.len() →
// early return with all-sentinel buffer.
let t = table_with_history(b"abc");
let buf = hc.chain_candidates(&t, 0);
assert!(buf.iter().all(|&v| v == usize::MAX));
}

#[test]
fn chain_candidates_terminates_on_self_loop_with_in_range_pick() {
// Construct a self-loop in the chain: hash_table → cur,
// chain_table[cur_rel] = cur (points back to itself). The walker
// must pick the position (in-range) and stop.
let mut hc = HcMatcher::new(2, 4, 32);
hc.search_depth = 4;
let mut t = table_with_history(b"abcdef_abcdef_abcdef");
let abs_pos = 10usize;
// The walker hashes the suffix at `abs_pos`, not the prefix at 0.
let concat = t.live_history();
let hash = t.hash_position(&concat[abs_pos..]);
// Stored = relative + 1 → stored=6 means candidate_rel=5.
t.hash_table[hash] = 6;
let chain_mask = (1 << t.chain_log) - 1;
t.chain_table[5 & chain_mask] = 6; // self-loop

let buf = hc.chain_candidates(&t, abs_pos);
assert_eq!(
buf[0], 5,
"self-loop pick must surface the in-range candidate"
);
assert_eq!(buf[1], usize::MAX, "walker must stop after self-loop");
}

#[test]
fn repcode_candidate_returns_none_when_suffix_too_short() {
let mut t = table_with_history(b"abc");
Expand Down
75 changes: 0 additions & 75 deletions zstd/src/encoding/hc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,81 +123,6 @@ impl HcMatcher {
}
}

/// Walk the hash chain at `abs_pos` and collect up to
/// [`HcMatcher::search_depth`] absolute positions of in-window
/// candidates. Stale chain entries (positions evicted from the
/// window) are skipped rather than terminating the walk; the
/// chain is bounded by `search_depth` total iterations to keep
/// pathological self-loops from spinning.
pub(crate) fn chain_candidates(
&self,
table: &MatchTable,
abs_pos: usize,
) -> [usize; MAX_HC_SEARCH_DEPTH] {
let mut buf = [usize::MAX; MAX_HC_SEARCH_DEPTH];
let idx = abs_pos - table.history_abs_start;
let concat = table.live_history();
if idx + 4 > concat.len() {
return buf;
}
let hash = table.hash_position(&concat[idx..]);
let chain_mask = (1 << table.chain_log) - 1;

let mut cur = table.hash_table[hash];
let mut filled = 0;
let mut steps = 0;
// Cap both the loop bound and the result-fill bound at
// MAX_HC_SEARCH_DEPTH so a misconfigured `search_depth >
// MAX_HC_SEARCH_DEPTH` (BT modes set it from the upstream zstd config,
// which can exceed 64) cannot index past `buf`'s fixed size.
let max_chain_steps = self.search_depth.min(MAX_HC_SEARCH_DEPTH);
while filled < max_chain_steps && steps < max_chain_steps {
if cur == HC_EMPTY {
break;
}
let candidate_rel = cur.wrapping_sub(1) as usize;
// Decode through `stored_abs_position_fast` so a non-zero
// `index_shift` (set by future rebase variants) is honored;
// raw `position_base + candidate_rel` would silently
// misread rebased entries.
let candidate_abs = super::match_table::storage::MatchTable::stored_abs_position_fast(
cur,
table.position_base,
table.index_shift,
);
let next = table.chain_table[candidate_rel & chain_mask];
steps += 1;
if next == cur {
// Self-loop: two positions share chain_idx, stop to
// avoid spinning on the same candidate forever.
if let Some(candidate_abs) = candidate_abs.filter(|&p| {
p >= table
.history_abs_start
.max(abs_pos.saturating_sub(table.max_window_size))
&& p < abs_pos
}) {
buf[filled] = candidate_abs;
}
break;
}
cur = next;
let Some(candidate_abs) = candidate_abs else {
continue;
};
if candidate_abs
< table
.history_abs_start
.max(abs_pos.saturating_sub(table.max_window_size))
|| candidate_abs >= abs_pos
{
continue;
}
buf[filled] = candidate_abs;
filled += 1;
}
buf
}

/// Probe the 3 rep-code offsets (with the upstream zstd `ll0 ↦ rep[0] − 1`
/// fallback) and return the best in-range match. Pure helper —
/// only reads from `MatchTable`, no HcMatcher state needed.
Expand Down
Loading
Loading