Skip to content

Limit wavediff filters to selected value streams#21

Merged
toddstrader merged 2 commits into
hudson-trading:mainfrom
jeet-dekivadia:limit-wavediff-filter-reads
Jun 22, 2026
Merged

Limit wavediff filters to selected value streams#21
toddstrader merged 2 commits into
hudson-trading:mainfrom
jeet-dekivadia:limit-wavediff-filter-reads

Conversation

@jeet-dekivadia

Copy link
Copy Markdown
Contributor

Fixes #20.

Summary

wavediff --filter already narrows the signal hierarchy before name/meta/value comparison, but the value-reader side still streamed filtered-out handles and dropped them later. This makes narrow filters correct in output, but less useful on large traces because the reader/channel/buffer path still sees data the user excluded.

This change derives per-reader local include sets from the filtered merged SignalMap and uses them while reading values:

  • FST readers now pass the selected local handles through FstFilter.include
  • VCD readers skip non-selected local handles before batching them into the diff channel
  • multi-file --set1/--set2 offsets are preserved by translating merged handles back to each reader's local handle space

Fixture metrics

Using the existing fixtures as a deterministic scope check:

  • tests/data/counter.fst: --filter '*.clk' narrows 4 signal names to 1
  • {set_clk.vcd, set_counter.vcd}: --filter '*.clk' narrows 4 signal names to 1
  • {set_clk.vcd, set_counter.vcd}: --filter '*.cyc_plus_one' narrows 4 signal names to 1, and the selected handle lives in the second reader, covering non-zero set offsets

After this patch, those narrowed scopes are reflected in the value stream instead of only in the final comparison/output layer.

Validation

cargo build --release
cargo test
cargo clippy -- -D warnings

I also checked the fixture scope counts with:

target/release/wavecat --names tests/data/counter.fst
target/release/wavecat --names --filter '*.clk' tests/data/counter.fst
target/release/wavecat --names tests/data/set_clk.vcd tests/data/set_counter.vcd
target/release/wavecat --names --filter '*.clk' tests/data/set_clk.vcd tests/data/set_counter.vcd
target/release/wavecat --names --filter '*.cyc_plus_one' tests/data/set_clk.vcd tests/data/set_counter.vcd

@toddstrader

Copy link
Copy Markdown
Collaborator

Thanks. Confirmed that this is way faster for small filter patterns. However, I'm a bit worried about reader_include_sets() in the default case. Does that assemble all the offsets when no -f is given? If so, can we short circuit it when there is no filter?

@jeet-dekivadia

Copy link
Copy Markdown
Contributor Author

Good catch — the previous version did build those sets even without -f. I pushed 75c7e74 to short-circuit that path.

reader_include_sets() now compares the retained merged-handle count with the readers' native signal count and returns None when the hierarchy is unfiltered. That propagates as FstFilter.include = None; the VCD path also skips the per-change HashSet lookup entirely. Filtered and match-nothing cases still use explicit per-reader sets.

I added regressions covering the short circuit for single-file FST, single-file VCD, and multi-file VCD inputs. cargo test and cargo clippy --all-targets --all-features -- -D warnings pass locally, and both upstream CI jobs are green on the new commit.

@toddstrader

Copy link
Copy Markdown
Collaborator

Great, this seems marginally faster in the default scenario now. -f top.* is slower than no filter, but that would be self-inflicted.

@toddstrader toddstrader merged commit 21fc1dd into hudson-trading:main Jun 22, 2026
2 checks passed
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.

wavediff --filter still streams filtered-out signal changes

2 participants