From 88f61fb8246900f6a992ca2776e83472553a91c8 Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Thu, 18 Jun 2026 22:35:33 -0700 Subject: [PATCH 1/2] Limit wavediff filtered value reads --- src/diff.rs | 95 +++++++++++++++++++++++++++++++++++++++++++--- tests/set_tests.rs | 33 ++++++++++++++++ 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 1d6e837..45d056b 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -12,7 +12,7 @@ use std::io::{self, BufRead, Seek, Write}; use std::path::Path; use crossbeam_channel as channel; -use fst_reader::{FstFilter, FstSignalValue}; +use fst_reader::{FstFilter, FstSignalHandle, FstSignalValue}; use crate::{next_vcd_change, NameOptions, NameTree, SignalMap, WaveHierarchy, WaveReader}; @@ -221,6 +221,25 @@ fn flush_batch(tx: &channel::Sender, time: u64, changes: &mut Vec Vec> { + let mut include_sets: Vec> = + (0..offsets.len()).map(|_| HashSet::new()).collect(); + + for &handle in signal_map.keys() { + let reader_idx = match offsets.binary_search(&handle) { + Ok(idx) => idx, + Err(0) => continue, + Err(idx) => idx - 1, + }; + include_sets[reader_idx].insert(handle - offsets[reader_idx]); + } + + include_sets +} + /// Read signals from an FST reader and send same-time batches to a channel. fn read_and_send_signals( mut fst_reader: fst_reader::FstReader, @@ -423,16 +442,18 @@ fn compare_signal_channels( fn send_wave_changes( reader: WaveReader, handle_offset: usize, + include: HashSet, start: u64, end: Option, tx: channel::Sender, ) -> io::Result<()> { match reader { WaveReader::Fst(fst_reader) => { + let include = include.into_iter().map(FstSignalHandle::from_index).collect(); let filter = FstFilter { start, end, - include: None, + include: Some(include), }; read_and_send_signals(*fst_reader, filter, handle_offset, tx)?; } @@ -448,6 +469,9 @@ fn send_wave_changes( break; } } + if !include.contains(&handle) { + continue; + } if !batch.is_empty() && (time != batch_time || batch.len() >= BATCH_SIZE) { flush_batch(&tx, batch_time, &mut batch); } @@ -472,14 +496,19 @@ fn send_wave_changes( fn send_merged_wave_changes( readers: Vec, offsets: &[usize], + include_sets: Vec>, start: u64, end: Option, tx: channel::Sender, ) -> io::Result<()> { + debug_assert_eq!(readers.len(), offsets.len()); + debug_assert_eq!(readers.len(), include_sets.len()); + if readers.len() == 1 { return send_wave_changes( readers.into_iter().next().unwrap(), offsets[0], + include_sets.into_iter().next().unwrap(), start, end, tx, @@ -490,10 +519,10 @@ fn send_merged_wave_changes( let mut inner_rxs = Vec::with_capacity(readers.len()); let mut threads = Vec::new(); - for (reader, &offset) in readers.into_iter().zip(offsets.iter()) { + for ((reader, &offset), include) in readers.into_iter().zip(offsets.iter()).zip(include_sets) { let (inner_tx, inner_rx) = channel::bounded(CHANNEL_BOUND); threads.push(std::thread::spawn(move || { - send_wave_changes(reader, offset, start, end, inner_tx) + send_wave_changes(reader, offset, include, start, end, inner_tx) })); inner_rxs.push(inner_rx); } @@ -644,12 +673,14 @@ pub fn diff_wave_sets( hier2, offsets2, } = sets; + let include_sets1 = reader_include_sets(&hier1.signal_map, &offsets1); + let include_sets2 = reader_include_sets(&hier2.signal_map, &offsets2); let thread1 = std::thread::spawn(move || { - send_merged_wave_changes(readers1, &offsets1, start, end, tx1) + send_merged_wave_changes(readers1, &offsets1, include_sets1, start, end, tx1) }); let thread2 = std::thread::spawn(move || { - send_merged_wave_changes(readers2, &offsets2, start, end, tx2) + send_merged_wave_changes(readers2, &offsets2, include_sets2, start, end, tx2) }); let result = compare_signal_channels( @@ -695,3 +726,55 @@ pub fn open_and_read_wave_sets( offsets2, }) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{SignalInfo, VarEntry, VarMeta, IMPLICIT_DIRECTION}; + + fn signal_map(handles: &[usize]) -> SignalMap { + handles + .iter() + .copied() + .map(|handle| { + ( + handle, + SignalInfo { + vars: vec![VarEntry { + name: 0, + meta: VarMeta { + var_type: "wire", + size: 1, + direction: IMPLICIT_DIRECTION, + }, + attrs: Vec::new(), + }], + }, + ) + }) + .collect() + } + + #[test] + fn reader_include_sets_split_merged_handles_by_reader_offset() { + let map = signal_map(&[0, 1, 4, 6, 9]); + + let include_sets = reader_include_sets(&map, &[0, 4, 9]); + + assert_eq!(include_sets.len(), 3); + assert_eq!(include_sets[0], HashSet::from([0, 1])); + assert_eq!(include_sets[1], HashSet::from([0, 2])); + assert_eq!(include_sets[2], HashSet::from([0])); + } + + #[test] + fn reader_include_sets_preserve_empty_filtered_readers() { + let map = signal_map(&[5]); + + let include_sets = reader_include_sets(&map, &[0, 5]); + + assert_eq!(include_sets.len(), 2); + assert!(include_sets[0].is_empty()); + assert_eq!(include_sets[1], HashSet::from([0])); + } +} diff --git a/tests/set_tests.rs b/tests/set_tests.rs index 3fc97ee..8819fcc 100644 --- a/tests/set_tests.rs +++ b/tests/set_tests.rs @@ -423,6 +423,39 @@ fn test_cli_wavediff_sets_only_value_diff() { assert!(stdout.contains("t.the_sub.cyc_plus_one")); } +#[test] +fn test_cli_wavediff_set_filter_excludes_value_diff() { + // The value difference lives in set_counter_modified.vcd. Filtering to the + // disjoint clk reader should avoid streaming/reporting that difference. + let output = run_wavediff_cli(&[ + "--filter", "*.clk", + "--set1", "tests/data/set_clk.vcd", + "--set1", "tests/data/set_counter_modified.vcd", + "--set2", "tests/data/counter.vcd", + ]); + assert!( + output.status.success(), + "Expected filtered set diff to exit 0, stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); +} + +#[test] +fn test_cli_wavediff_set_filter_includes_value_diff() { + // Filtering to a signal in the second set1 reader exercises non-zero + // handle offsets while preserving the expected value diff. + let output = run_wavediff_cli(&[ + "--filter", "*.cyc_plus_one", + "--set1", "tests/data/set_clk.vcd", + "--set1", "tests/data/set_counter_modified.vcd", + "--set2", "tests/data/counter.vcd", + ]); + assert_eq!(output.status.code(), Some(1), "Expected exit 1"); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!(stdout.contains("t.the_sub.cyc_plus_one")); +} + #[test] fn test_cli_wavediff_set1_only_no_positional_fails() { // Only --set1 without positional args should fail From 75c7e7421e23d743523a52e81c3db98d5a96301f Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Mon, 22 Jun 2026 13:28:12 -0700 Subject: [PATCH 2/2] Skip include sets for unfiltered diffs --- src/diff.rs | 87 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 17 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 45d056b..88c6148 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -221,10 +221,18 @@ fn flush_batch(tx: &channel::Sender, time: u64, changes: &mut Vec Vec> { +/// Build per-reader local handle include sets from a filtered merged signal map. +/// Returns `None` when every source handle is still present so the readers can +/// keep their unfiltered fast path. +fn reader_include_sets( + signal_map: &SignalMap, + offsets: &[usize], + source_signal_count: usize, +) -> Option>> { + if signal_map.len() == source_signal_count { + return None; + } + let mut include_sets: Vec> = (0..offsets.len()).map(|_| HashSet::new()).collect(); @@ -237,7 +245,14 @@ fn reader_include_sets(signal_map: &SignalMap, offsets: &[usize]) -> Vec usize { + match reader { + WaveReader::Fst(reader) => reader.get_header().max_handle as usize, + WaveReader::Vcd(vcd_data) => vcd_data.id_to_idx.len(), + } } /// Read signals from an FST reader and send same-time batches to a channel. @@ -442,18 +457,20 @@ fn compare_signal_channels( fn send_wave_changes( reader: WaveReader, handle_offset: usize, - include: HashSet, + include: Option>, start: u64, end: Option, tx: channel::Sender, ) -> io::Result<()> { match reader { WaveReader::Fst(fst_reader) => { - let include = include.into_iter().map(FstSignalHandle::from_index).collect(); + let include = include.map(|handles| { + handles.into_iter().map(FstSignalHandle::from_index).collect() + }); let filter = FstFilter { start, end, - include: Some(include), + include, }; read_and_send_signals(*fst_reader, filter, handle_offset, tx)?; } @@ -469,7 +486,7 @@ fn send_wave_changes( break; } } - if !include.contains(&handle) { + if include.as_ref().is_some_and(|handles| !handles.contains(&handle)) { continue; } if !batch.is_empty() && (time != batch_time || batch.len() >= BATCH_SIZE) { @@ -496,19 +513,21 @@ fn send_wave_changes( fn send_merged_wave_changes( readers: Vec, offsets: &[usize], - include_sets: Vec>, + include_sets: Option>>, start: u64, end: Option, tx: channel::Sender, ) -> io::Result<()> { debug_assert_eq!(readers.len(), offsets.len()); - debug_assert_eq!(readers.len(), include_sets.len()); + if let Some(sets) = &include_sets { + debug_assert_eq!(readers.len(), sets.len()); + } if readers.len() == 1 { return send_wave_changes( readers.into_iter().next().unwrap(), offsets[0], - include_sets.into_iter().next().unwrap(), + include_sets.and_then(|sets| sets.into_iter().next()), start, end, tx, @@ -519,7 +538,9 @@ fn send_merged_wave_changes( let mut inner_rxs = Vec::with_capacity(readers.len()); let mut threads = Vec::new(); - for ((reader, &offset), include) in readers.into_iter().zip(offsets.iter()).zip(include_sets) { + let mut include_sets = include_sets.map(Vec::into_iter); + for (reader, &offset) in readers.into_iter().zip(offsets.iter()) { + let include = include_sets.as_mut().and_then(|sets| sets.next()); let (inner_tx, inner_rx) = channel::bounded(CHANNEL_BOUND); threads.push(std::thread::spawn(move || { send_wave_changes(reader, offset, include, start, end, inner_tx) @@ -673,8 +694,10 @@ pub fn diff_wave_sets( hier2, offsets2, } = sets; - let include_sets1 = reader_include_sets(&hier1.signal_map, &offsets1); - let include_sets2 = reader_include_sets(&hier2.signal_map, &offsets2); + let source_signal_count1 = readers1.iter().map(reader_signal_count).sum(); + let source_signal_count2 = readers2.iter().map(reader_signal_count).sum(); + let include_sets1 = reader_include_sets(&hier1.signal_map, &offsets1, source_signal_count1); + let include_sets2 = reader_include_sets(&hier2.signal_map, &offsets2, source_signal_count2); let thread1 = std::thread::spawn(move || { send_merged_wave_changes(readers1, &offsets1, include_sets1, start, end, tx1) @@ -759,7 +782,7 @@ mod tests { fn reader_include_sets_split_merged_handles_by_reader_offset() { let map = signal_map(&[0, 1, 4, 6, 9]); - let include_sets = reader_include_sets(&map, &[0, 4, 9]); + let include_sets = reader_include_sets(&map, &[0, 4, 9], 12).unwrap(); assert_eq!(include_sets.len(), 3); assert_eq!(include_sets[0], HashSet::from([0, 1])); @@ -771,10 +794,40 @@ mod tests { fn reader_include_sets_preserve_empty_filtered_readers() { let map = signal_map(&[5]); - let include_sets = reader_include_sets(&map, &[0, 5]); + let include_sets = reader_include_sets(&map, &[0, 5], 10).unwrap(); assert_eq!(include_sets.len(), 2); assert!(include_sets[0].is_empty()); assert_eq!(include_sets[1], HashSet::from([0])); } + + #[test] + fn reader_include_sets_skip_unfiltered_sources() { + let map = signal_map(&[0, 1, 2, 3, 4]); + + assert!(reader_include_sets(&map, &[0, 3], 5).is_none()); + } + + #[test] + fn native_reader_counts_match_unfiltered_hierarchies() { + for path in ["tests/data/counter.fst", "tests/data/counter.vcd"] { + let (reader, hierarchy) = + crate::open_wave_file(Path::new(path), &NameOptions::default()).unwrap(); + let source_signal_count = reader_signal_count(&reader); + + assert_eq!(source_signal_count, hierarchy.signal_map.len(), "{path}"); + assert!(reader_include_sets(&hierarchy.signal_map, &[0], source_signal_count).is_none()); + } + + let paths = [ + Path::new("tests/data/set_clk.vcd"), + Path::new("tests/data/set_counter.vcd"), + ]; + let (readers, hierarchy, offsets) = + crate::open_wave_files(&paths, &NameOptions::default(), None).unwrap(); + let source_signal_count = readers.iter().map(reader_signal_count).sum(); + + assert_eq!(source_signal_count, hierarchy.signal_map.len()); + assert!(reader_include_sets(&hierarchy.signal_map, &offsets, source_signal_count).is_none()); + } }