perf(encoding): add row-based match finder backend by polaz · Pull Request #84 · structured-world/structured-zstd

polaz · 2026-04-08T14:52:22Z

Summary

add new MatcherBackend::Row with dedicated RowMatchGenerator (row/tag table probing + lazy lookahead)
wire backend selection through level params with runtime routing (row backend enabled for numeric level 4)
preserve existing quality guardrails by keeping CompressionLevel::Default on dfast and moving dfast-specific tests to CompressionLevel::Level(2)
add backend-specific tests for row selection and row multi-block roundtrip

Validation

cargo check --workspace
cargo clippy --workspace --all-targets -- -D warnings
cargo nextest run --workspace
cargo test --doc --workspace

Closes #67

Summary by CodeRabbit

New Features
- Added a row-based match-finding backend to improve compression at higher levels.
Improvements
- Level 4 now uses the row backend; adaptive row-table sizing reduces memory with source-size hints, improves cross-block backfill, and smooths backend switching and buffer reuse.
- Refined match-selection logic for more consistent lazy-match and backward-extension behavior.
- Short blocks may emit literals-only when insufficient data remains.
Tests
- Expanded coverage for the new backend, window shrinking, boundary cases, lazy-match edges, and reset/switch scenarios.
Chores
- Added a benchmark level for the row backend, made benchmark selection driven by available ratio data, and adjusted CI benchmark timeouts.

coderabbitai · 2026-04-08T14:52:37Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 13074238-c79c-4c59-980a-e01b1416c446

📥 Commits

Reviewing files that changed from the base of the PR and between 10de7b5 and 64d2009.

📒 Files selected for processing (1)

.github/scripts/run-benchmarks.sh

📝 Walkthrough

Walkthrough

Adds a Row-based match finder backend (RowMatchGenerator), wires it into level parameter resolution and MatchGeneratorDriver (selected for level 4), introduces RowConfig and dynamic row-hash sizing, updates matching control flow for Row, and adds tests and benchmark/CI support for the Row backend.

Changes

Cohort / File(s)	Summary
Core backend & driver integration `zstd/src/encoding/match_generator.rs`	Add `MatcherBackend::Row`, `RowConfig`, `row_hash_bits_for_window()`, embed `row` in `LevelParams`, route level 4 to Row, and extend `MatchGeneratorDriver` with `row_match_generator` and Row-aware control paths (reset/start/skip/evict/prime/backfill).
RowMatchGenerator implementation `zstd/src/encoding/match_generator.rs` (Row internals)	Introduce `RowMatchGenerator` replacing prior HC logic: row table arrays (`row_heads`, `row_positions`, `row_tags`), combined row+tag hashing, bounded probe/search_depth, shared helpers for best-candidate selection, lazy/backward-extension logic, insertion/backfill, and table resizing/clearing.
DFast refactor & shared helpers `zstd/src/encoding/match_generator.rs`	Refactor Dfast match-selection flow to use new shared helper functions (best-candidate comparison, backward extension, repcode candidate gen, lazy-match picking) replacing bespoke implementations.
Params, tests & routing `zstd/src/encoding/match_generator.rs`, `zstd/src/encoding/...tests`, `zstd/tests/`	Resolve/populate Row sizing from source-size hints, shrink Row hash-table footprint accordingly, and add/adjust tests covering Row selection, correctness, lazy-match edge cases, cross-boundary tail backfill, dictionary priming, reset/backend-switching, and roundtrip behavior.
Benchmarks & CI `zstd/benches/support/mod.rs`, `.github/scripts/run-benchmarks.sh`, `.github/workflows/ci.yml`	Add `"level4-row"` benchmark level (maps to CompressionLevel::Level(4)), extend supported_levels array, derive regression levels from parsed ratios, and parameterize benchmark job timeouts per matrix entry.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Compression Pipeline
    participant MG as MatchGeneratorDriver
    participant Row as RowMatchGenerator
    participant Tables as Row Hash Tables
    participant Collector as Match Collector

    Client->>MG: start_matching(level=4, pos)
    MG->>MG: select MatcherBackend::Row
    MG->>Row: initialize(RowConfig)
    Client->>MG: request_matches(pos)
    MG->>Row: find_candidates(pos)
    Row->>Tables: row+tag lookup (heads/positions/tags)
    Tables-->>Row: candidate positions
    Row->>Row: filter candidates & backward-extend (lazy)
    Row-->>Collector: emit matches or literals
    Row->>Tables: insert/update current position (backfill/tail)
    Tables-->>Row: ack
    MG-->>Client: return matches

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

feat(encoding): add Better compression level (zstd level 7, lazy2 strategy) #48: Adds an alternative matcher backend and updates match_generator.rs with backend/driver lifecycle changes (closely related).
feat(encoding): numeric compression levels (1-22) API #63: Modifies match generator types and wiring that this PR extends with Row-specific fields and logic.
fix(encoding): implement default compression level #34: Introduces a matcher backend and dispatch changes in MatchGeneratorDriver, overlapping implementation concerns.

Poem

🐰 I hopped through hashed rows at dawn,
Tags in pockets, tails reborn,
I probed and leapt, then stitched back right,
Level four hums soft and light —
A rabbit's hop, a compressor's song.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.91% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'perf(encoding): add row-based match finder backend' clearly summarizes the main change: adding a new row-based match finder backend to the encoding module.
Linked Issues check	✅ Passed	All primary objectives from issue `#67` are met: RowMatch backend implemented with dedicated table layout (RowMatchGenerator), backend selection integrated into level parameters (Level 4 routes to Row), existing backends preserved, benchmarks added for row backend, and validation performed.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with PR objectives: match_generator.rs implements Row backend logic, benchmark scripts support new level4-row configuration, CI workflow adjusted for benchmarking needs, and supporting test updates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch perf/#67-row-match-finder

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-04-08T14:54:10Z

Codecov Report

❌ Patch coverage is 99.19920% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
zstd/src/encoding/match_generator.rs	99.19%	8 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

Adds a new row-based match-finder backend to the encoder and routes numeric compression level 4 to use it, with updated parameter resolution and targeted tests to validate backend selection and basic multi-block correctness.

Changes:

Introduce RowMatchGenerator (row/tag table probing + lazy lookahead) and integrate it into MatchGeneratorDriver.
Extend level parameter resolution to include Row-specific config and enable the Row backend for CompressionLevel::Level(4).
Update/add tests to pin backend selection and validate Row multi-block roundtrip reconstruction.

polaz · 2026-04-08T15:29:03Z

@coderabbitai full review

coderabbitai · 2026-04-08T15:29:10Z

✅ Actions performed

Full review triggered.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

coderabbitai · 2026-04-08T15:38:43Z

✅ Actions performed

Full review triggered.

coderabbitai

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 `@zstd/src/encoding/match_generator.rs`:
- Around line 1750-1754: hash_and_row() currently hashes 8 bytes when available
but falls back to 4 bytes near block tails, causing probes and inserts to use
different keys and leaving the last 7 positions underindexed; to fix, make the
Row table key stable across inserts and probes by choosing a single fixed key
width (e.g. always hash 8 bytes with safe padding) or implement dual short/long
row tables and ensure both are consulted on probes, update insert_positions(),
skip_matching(), and start_matching() to backfill the final <4 starts using the
HC-style boundary backfill so that tails inserted with the shorter key remain
visible to later 8-byte probes, and add a regression test exercising a
cross-boundary match across a 7-byte tail to prevent regressions.

🪄 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: 2888c842-27bf-477a-8110-a6374d9ff224

📥 Commits

Reviewing files that changed from the base of the PR and between c9ab3d3 and cf3c0d0.

📒 Files selected for processing (1)

zstd/src/encoding/match_generator.rs

polaz · 2026-04-08T15:59:42Z

@coderabbitai Fixed: row hash key is now stable (4-byte key), previous-block tail positions are backfilled before matching/skip, and a regression test covers cross-boundary tail reuse.

coderabbitai · 2026-04-08T16:00:32Z

Tip

For best results, initiate chat on the files or code changes.

@polaz: Thanks for the update! The three fixes align well with the established patterns across other backends:

Stable 4-byte row hash key — consistent with the same 4-byte lookahead constraint required by HcMatchGenerator::insert_position (if idx + 4 > concat.len() { return; }), so min_primed_tail = 4 applies uniformly across Dfast, HC, and now Row.
Tail backfill before matching/skip — mirrors the backfill_boundary_positions pattern used for HC to ensure cross-block boundary positions are hashable before the encoder processes the new block.
Regression test for cross-boundary tail reuse — good to have explicit coverage so this edge case doesn't silently regress.

✏️ Learnings added

Learnt from: polaz
URL: https://github.com/structured-world/structured-zstd/pull/84

Timestamp: 2026-04-08T16:00:30.414Z
Learning: In `zstd/src/encoding/match_generator.rs`, the `RowMatchGenerator` (used for `CompressionLevel::Level(4)` / `MatcherBackend::Row`) uses a stable 4-byte hash key, consistent with the 4-byte lookahead constraint shared by `HcMatchGenerator`. Previous-block tail positions are backfilled into the row tables before matching/skip begins (analogous to `backfill_boundary_positions` for the HC backend), and a regression test covers cross-boundary tail reuse. Do not flag missing backfill or key-width issues for the Row backend.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:336-339
Timestamp: 2026-04-02T12:03:03.856Z
Learning: In `zstd/src/encoding/match_generator.rs`, `min_primed_tail = 4` is correct for both `MatcherBackend::Dfast` and `MatcherBackend::HashChain` because `HcMatchGenerator::insert_position` unconditionally requires 4 bytes of hash lookahead (`if idx + 4 > concat.len() { return; }`). `backfill_boundary_positions` only re-inserts positions already fully within extended history; it cannot make a <4-byte dictionary tail fragment hashable. Do not suggest lowering `min_primed_tail` below 4 for the `HashChain` backend.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-01T06:49:34.233Z
Learning: In the `structured-zstd` codebase (`zstd/src/encoding/match_generator.rs`), `CompressionLevel::Default` uses the `DfastMatchGenerator` backend (with `pick_lazy_match` lazy evaluation), while `CompressionLevel::Better` uses the `HcMatchGenerator` (hash chain backend) which inserts positions before lazy evaluation inside its own `start_matching` loop. These two backends are independent; review comments about Dfast internals (e.g. position seeding order in `pick_lazy_match`) do not apply to the Better level's implementation path.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-01T06:49:40.012Z
Learning: In the structured-zstd codebase (`zstd/src/encoding/match_generator.rs`), `CompressionLevel::Default` uses the `DfastMatchGenerator` backend (with `pick_lazy_match` for lazy evaluation), while `CompressionLevel::Better` uses the `HcMatchGenerator` (hash chain backend) which inserts positions before lazy evaluation inside its own `start_matching` loop. These are distinct backends with separate lazy-seeding paths.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 53
File: zstd/src/encoding/frame_compressor.rs:280-287
Timestamp: 2026-04-02T16:31:58.167Z
Learning: In `structured-zstd`, `CompressionLevel::Best` intentionally uses `MatcherBackend::HashChain` (not a binary-tree/DUBT backend) due to memory constraints: a BT matcher for a 16 MiB window requires ~128–256 MB of tables (C zstd level 11 uses chainLog=25 → 256 MB), which is impractical. Best is distinguished from Better by deeper HC search (32 vs 16 candidates), larger hash/chain tables (2M/1M vs 1M/512K), and higher target match length (128 vs 48), keeping memory under 20 MB. It achieves a proportional compression gain (~0.27% better than Better on corpus) comparable to C zstd's L7→L11 delta (~0.14%). A proper BT backend is deferred as a future optimization. Do not flag Best's use of HashChain as missing a BT implementation.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 53
File: zstd/src/tests/roundtrip_integrity.rs:498-509
Timestamp: 2026-04-02T22:26:07.979Z
Learning: In `structured-zstd` (`zstd/src/tests/roundtrip_integrity.rs`), `best_level_does_not_regress_vs_better` uses a `<=` (not strict `<`) assertion because the `repeat_offset_fixture(b"HelloWorld", ...)` input is simple enough that HC saturates at both Better (16 candidates) and Best (32 candidates) search depths, producing identical compressed sizes (~30243 bytes). Strict `<` would be a false positive on this fixture. The strict `Best < Better` quality assertion lives in `cross_validation::best_level_beats_better_on_corpus_proxy` on the decodecorpus sample. Do not re-flag the `<=` as a weakened guard.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:1554-1559
Timestamp: 2026-04-02T12:03:04.619Z
Learning: In `zstd/src/encoding/match_generator.rs`, `HcMatchGenerator::insert_position` has an early-return guard at `abs_pos >= u32::MAX` that disables all further HC insertions for streams >4 GiB, degrading `CompressionLevel::Better` to repcodes-only. This is a known limitation tracked in issue `#51`; the proper fix (table rebasing) is deferred. A TODO comment and a limitation note on `CompressionLevel::Better` doc were added in PR `#48`. Do not re-flag this as a new bug.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@zstd/src/encoding/match_generator.rs`:
- Around line 122-126: Add the planned perf benchmark that exercises the
Level(4) routing (the LevelParams entry with backend: MatcherBackend::Row and
ROW_CONFIG) to measure throughput and match-quality on a representative corpus;
the benchmark should run a small/mid-block workload using the same setup as unit
tests but collecting throughput and match ratio metrics, and assert they stay
within acceptable bounds for ROW_SEARCH_DEPTH and ROW_TARGET_LEN so changes to
the Level(4) routing are guarded by automated performance checks. Ensure the
benchmark references the LevelParams entry for Level(4), uses
ROW_CONFIG/ROW_SEARCH_DEPTH/ROW_TARGET_LEN constants, and is integrated into the
CI perf job (or run via cargo bench) so regressions are flagged automatically.
- Around line 450-458: When switching away from MatcherBackend::Row ensure you
not only call row.reset(...) but also release the row tables (row_heads,
row_positions, row_tags) to free their heap allocation; after obtaining row via
self.row_match_generator.as_mut() and before/after calling row.reset(...), drop
or replace those fields (e.g. mem::take or Vec::clear + shrink_to_fit) the same
way the HC backend does so the row table footprint is returned to the allocator
and doesn't remain resident for the driver's lifetime.

🪄 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: 46b74064-8cdc-4f2b-9187-a5bd813bc510

📥 Commits

Reviewing files that changed from the base of the PR and between cf3c0d0 and 8959a3b.

📒 Files selected for processing (1)

zstd/src/encoding/match_generator.rs

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

sw-release-bot

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'structured-zstd vs C FFI (x86_64-gnu)'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite	Current: `af3da92`	Previous: `c9ab3d3`	Ratio
`compress/default/small-4k-log-lines/matrix/pure_rust`	`7.573` ms	`4.994` ms	`1.52`
`compress/better/small-4k-log-lines/matrix/pure_rust`	`0.254` ms	`0.153` ms	`1.66`
`compress/better/small-4k-log-lines/matrix/c_ffi`	`0.182` ms	`0.077` ms	`2.36`

This comment was automatically generated by workflow using github-action-benchmark.

CC: @polaz

coderabbitai

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 `@zstd/src/encoding/match_generator.rs`:
- Around line 2854-2866: The test never sets saw_triple because the
matcher.start_matching closure only matches Sequence::Literals; update the
closure passed to matcher.start_matching to also match the triple variant (e.g.,
Sequence::Triple or Sequence::Match as defined in the Sequence enum) and set
saw_triple = true when that branch is hit, while keeping the existing logic that
extends reconstructed for Sequence::Literals; this ensures the
assert!(!saw_triple, ...) correctly fails if any triples are emitted.

🪄 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: 7da786ef-bb08-4cec-babd-3626972cfe7d

📥 Commits

Reviewing files that changed from the base of the PR and between af3da92 and c229cf4.

📒 Files selected for processing (1)

zstd/src/encoding/match_generator.rs

polaz · 2026-04-08T16:54:20Z

@coderabbitai full review

polaz · 2026-04-08T17:46:41Z

@coderabbitai re-review

coderabbitai · 2026-04-08T17:46:54Z

@polaz: Triggering a full re-review of PR #84 now.

🧠 Learnings used

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-08T16:00:30.438Z
Learning: In `zstd/src/encoding/match_generator.rs`, the `RowMatchGenerator` (used for `CompressionLevel::Level(4)` / `MatcherBackend::Row`) uses a stable 4-byte hash key, consistent with the 4-byte lookahead constraint shared by `HcMatchGenerator`. Previous-block tail positions are backfilled into the row tables before matching/skip begins (analogous to `backfill_boundary_positions` for the HC backend), and a regression test covers cross-boundary tail reuse. Do not flag missing backfill or key-width issues for the Row backend.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-01T06:49:34.233Z
Learning: In the `structured-zstd` codebase (`zstd/src/encoding/match_generator.rs`), `CompressionLevel::Default` uses the `DfastMatchGenerator` backend (with `pick_lazy_match` lazy evaluation), while `CompressionLevel::Better` uses the `HcMatchGenerator` (hash chain backend) which inserts positions before lazy evaluation inside its own `start_matching` loop. These two backends are independent; review comments about Dfast internals (e.g. position seeding order in `pick_lazy_match`) do not apply to the Better level's implementation path.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:1554-1559
Timestamp: 2026-04-02T12:03:04.619Z
Learning: In `zstd/src/encoding/match_generator.rs`, `HcMatchGenerator::insert_position` has an early-return guard at `abs_pos >= u32::MAX` that disables all further HC insertions for streams >4 GiB, degrading `CompressionLevel::Better` to repcodes-only. This is a known limitation tracked in issue `#51`; the proper fix (table rebasing) is deferred. A TODO comment and a limitation note on `CompressionLevel::Better` doc were added in PR `#48`. Do not re-flag this as a new bug.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:336-339
Timestamp: 2026-04-02T12:03:03.856Z
Learning: In `zstd/src/encoding/match_generator.rs`, `min_primed_tail = 4` is correct for both `MatcherBackend::Dfast` and `MatcherBackend::HashChain` because `HcMatchGenerator::insert_position` unconditionally requires 4 bytes of hash lookahead (`if idx + 4 > concat.len() { return; }`). `backfill_boundary_positions` only re-inserts positions already fully within extended history; it cannot make a <4-byte dictionary tail fragment hashable. Do not suggest lowering `min_primed_tail` below 4 for the `HashChain` backend.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 53
File: zstd/src/tests/roundtrip_integrity.rs:498-509
Timestamp: 2026-04-02T22:26:07.979Z
Learning: In `structured-zstd` (`zstd/src/tests/roundtrip_integrity.rs`), `best_level_does_not_regress_vs_better` uses a `<=` (not strict `<`) assertion because the `repeat_offset_fixture(b"HelloWorld", ...)` input is simple enough that HC saturates at both Better (16 candidates) and Best (32 candidates) search depths, producing identical compressed sizes (~30243 bytes). Strict `<` would be a false positive on this fixture. The strict `Best < Better` quality assertion lives in `cross_validation::best_level_beats_better_on_corpus_proxy` on the decodecorpus sample. Do not re-flag the `<=` as a weakened guard.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 53
File: zstd/src/encoding/frame_compressor.rs:280-287
Timestamp: 2026-04-02T16:31:58.167Z
Learning: In `structured-zstd`, `CompressionLevel::Best` intentionally uses `MatcherBackend::HashChain` (not a binary-tree/DUBT backend) due to memory constraints: a BT matcher for a 16 MiB window requires ~128–256 MB of tables (C zstd level 11 uses chainLog=25 → 256 MB), which is impractical. Best is distinguished from Better by deeper HC search (32 vs 16 candidates), larger hash/chain tables (2M/1M vs 1M/512K), and higher target match length (128 vs 48), keeping memory under 20 MB. It achieves a proportional compression gain (~0.27% better than Better on corpus) comparable to C zstd's L7→L11 delta (~0.14%). A proper BT backend is deferred as a future optimization. Do not flag Best's use of HashChain as missing a BT implementation.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-01T06:49:40.012Z
Learning: In the structured-zstd codebase (`zstd/src/encoding/match_generator.rs`), `CompressionLevel::Default` uses the `DfastMatchGenerator` backend (with `pick_lazy_match` for lazy evaluation), while `CompressionLevel::Better` uses the `HcMatchGenerator` (hash chain backend) which inserts positions before lazy evaluation inside its own `start_matching` loop. These are distinct backends with separate lazy-seeding paths.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:1470-1490
Timestamp: 2026-04-02T07:27:15.870Z
Learning: In `zstd/src/encoding/match_generator.rs`, `HcMatchGenerator::pick_lazy_match` intentionally queries `find_best_match(abs_pos + 1, ...)` and `find_best_match(abs_pos + 2, ...)` *before* those positions are inserted into the hash-chain tables. This matches C zstd lazy evaluation semantics: pre-seeding those positions would allow a position to match against itself, producing invalid self-matches. Do not flag the absent pre-seeding as a bug.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 48
File: zstd/src/encoding/match_generator.rs:1531-1535
Timestamp: 2026-04-01T15:33:17.229Z
Learning: In `zstd/src/encoding/match_generator.rs`, `HcMatchGenerator::insert_position` intentionally casts `abs_pos: usize` to `u32` (wrapping on streams > 4 GiB). This is safe by design: (1) the chain table slot is computed as `(pos_u32 as usize) & chain_mask`, so only the low bits matter and wrapping leaves the addressed slot unchanged; (2) `chain_candidates` filters any recovered `cur as usize` value against `history_abs_start` — a wrapped (small) value is always less than the current sliding-window start and is discarded. Do not flag this cast as a bug.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 53
File: zstd/src/tests/roundtrip_integrity.rs:479-523
Timestamp: 2026-04-02T19:19:44.750Z
Learning: In `structured-zstd` (`zstd/src/tests/roundtrip_integrity.rs`), the per-level roundtrip test suites (Better, Best) share a common `level_roundtrip_suite!` macro that expands the same 7-test matrix (compressible, random, multi-block, streaming, edge cases, repeat offsets, large literals) for each level via module-scoped macro invocations. Do not suggest re-inlining these as separate flat test functions — the macro was added intentionally to eliminate cross-level drift.

Learnt from: polaz
Repo: structured-world/structured-zstd PR: 0
File: :0-0
Timestamp: 2026-04-06T01:40:24.378Z
Learning: In `zstd/benches/compare_ffi.rs` (structured-world/structured-zstd), Rust FastCOVER trains with the post-finalization content budget in both the `REPORT_DICT_TRAIN` emission path (around lines 208-225) and the Criterion benchmark path (around lines 266-280). Both paths were aligned in commit 8622344. Do not flag these ranges as using inconsistent budget values.

✅ Actions performed

Full review triggered.

polaz · 2026-04-08T17:53:00Z

@coderabbitai full review

coderabbitai · 2026-04-08T17:53:06Z

✅ Actions performed

Full review triggered.

polaz · 2026-04-08T17:53:50Z

@coderabbitai full review

coderabbitai · 2026-04-08T17:53:56Z

✅ Actions performed

Full review triggered.

coderabbitai

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 `@zstd/src/encoding/match_generator.rs`:
- Around line 1875-1952: The Row backend duplicates the Dfast
lazy-match/repcode/backward-extension logic across best_match, pick_lazy_match,
repcode_candidate, extend_backwards, and better_candidate; factor that shared
logic into one reusable helper (e.g., a MatchHeuristics or shared functions like
repcode_candidate_shared, pick_lazy_match_shared, extend_backwards_shared,
better_candidate_shared) that takes the backend-specific inputs (abs_pos,
lit_len, offset history, history bounds, lazy_depth, target_len, live_history
accessor) and returns the same MatchCandidate/Option semantics, then replace the
duplicated implementations in this file and the corresponding Dfast locations
(also around the other block at 1996-2037) to call the shared helpers so
behavior and tests remain unchanged.

🪄 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: a5169d8d-559b-4dfb-b9ed-c1b8de68f256

📥 Commits

Reviewing files that changed from the base of the PR and between c9ab3d3 and bb40eea.

📒 Files selected for processing (3)

.github/scripts/run-benchmarks.sh
zstd/benches/support/mod.rs
zstd/src/encoding/match_generator.rs

polaz · 2026-04-08T18:42:58Z

@coderabbitai full review

coderabbitai · 2026-04-08T18:43:10Z

✅ Actions performed

Full review triggered.

polaz · 2026-04-08T18:43:37Z

@coderabbitai full review

coderabbitai · 2026-04-08T18:43:42Z

✅ Actions performed

Full review triggered.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

coderabbitai

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/run-benchmarks.sh:
- Line 108: REGRESSION_LEVELS duplicates the canonical list in
supported_levels(), causing silent mismatches; remove the hardcoded
REGRESSION_LEVELS and instead derive/validate benchmark level names against the
canonical source from the Rust side (supported_levels()) by either calling a
small Rust helper that prints supported levels or by loading a single shared
policy map that both sides use, then use that output to populate or validate the
set used in run-benchmarks.sh so the shell script and supported_levels() cannot
drift.

🪄 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: 05a4cff2-5f82-442e-a543-551855e1c2fe

📥 Commits

Reviewing files that changed from the base of the PR and between c9ab3d3 and 10de7b5.

📒 Files selected for processing (4)

.github/scripts/run-benchmarks.sh
.github/workflows/ci.yml
zstd/benches/support/mod.rs
zstd/src/encoding/match_generator.rs

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

perf(encoding): add row-based match finder backend

5741bb8

Copilot AI review requested due to automatic review settings April 8, 2026 14:52

Copilot started reviewing on behalf of polaz April 8, 2026 14:53 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs Outdated

Comment thread zstd/src/encoding/match_generator.rs Outdated

test(encoding): raise row matcher coverage and decouple min match const

cf3c0d0

polaz requested a review from Copilot April 8, 2026 15:28

Copilot started reviewing on behalf of polaz April 8, 2026 15:29 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

coderabbitai bot reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs

fix(encoding): stabilize row hash key across block boundaries

8959a3b

test(encoding): cover row dictionary retention paths

af3da92

polaz requested a review from Copilot April 8, 2026 16:09

Copilot started reviewing on behalf of polaz April 8, 2026 16:09 View session

coderabbitai bot reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs

Comment thread zstd/src/encoding/match_generator.rs

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs Outdated

sw-release-bot bot reviewed Apr 8, 2026

View reviewed changes

test(encoding): close row matcher patch coverage gaps

c229cf4

coderabbitai bot reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs

polaz added 3 commits April 8, 2026 19:41

fix(encoding): harden row backend reset and hash extraction

d5f117c

perf(bench): add level4 row lane to regression matrix

a7c13d2

docs(encoding): document new row matcher checks

d9c89da

polaz requested a review from Copilot April 8, 2026 16:54

coderabbitai bot reviewed Apr 8, 2026

View reviewed changes

Comment thread zstd/src/encoding/match_generator.rs

polaz added 2 commits April 8, 2026 21:37

refactor(encoding): share row and dfast heuristics

77eba99

ci(bench): raise i686 benchmark timeout

10de7b5

polaz requested a review from Copilot April 8, 2026 18:42

Copilot started reviewing on behalf of polaz April 8, 2026 18:43 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

coderabbitai bot reviewed Apr 8, 2026

View reviewed changes

Comment thread .github/scripts/run-benchmarks.sh Outdated

ci(bench): derive regression levels from bench output

64d2009

polaz requested a review from Copilot April 8, 2026 19:04

Copilot started reviewing on behalf of polaz April 8, 2026 19:05 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

polaz merged commit 9f905ad into main Apr 8, 2026
17 checks passed

polaz deleted the perf/#67-row-match-finder branch April 8, 2026 19:13

sw-release-bot bot mentioned this pull request Apr 8, 2026

chore: release v0.0.10 #83

Merged

polaz mentioned this pull request Apr 8, 2026

Roadmap: structured-zstd feature parity with C zstd #28

Open

Conversation

polaz commented Apr 8, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sw-release-bot bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

polaz commented Apr 8, 2026

Uh oh!

coderabbitai bot commented Apr 8, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

polaz commented Apr 8, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 8, 2026 •

edited

Loading

codecov bot commented Apr 8, 2026 •

edited

Loading