Skip to content

Commit 30ebcfc

Browse files
committed
fix is_odd condition to ensure optimal number of iterations are used
closes #32 should slightly reduce number of iterations during diffing and therefore slightly speedup the diff. May change the results slightly in edgecase (but overall the old results where still correct, this would only affect edgecases with multiple correct results, very unlikely to be noticed in practice)
1 parent 8de65af commit 30ebcfc

File tree

5 files changed

+82
-1
lines changed

5 files changed

+82
-1
lines changed

Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ unified_diff = []
2626

2727
[dev-dependencies]
2828
# criterion = "0.4.0"
29+
cov-mark = "2.1.0"
2930
expect-test = "1.4.0"
3031
# git-repository = "0.25.0"
3132
# similar = { version = "2.2.0", features = ["bytes"] }

src/myers.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl Myers {
9999
unsafe { MiddleSnakeSearch::<false>::new(self.kforward, file1, file2) };
100100
let mut backwards_search =
101101
unsafe { MiddleSnakeSearch::<true>::new(self.kbackward, file1, file2) };
102-
let is_odd = (file2.len() - file2.len()) & 1 != 0;
102+
let is_odd = file2.len().wrapping_sub(file1.len()) & 1 != 0;
103103

104104
let mut ec = 0;
105105

@@ -111,6 +111,8 @@ impl Myers {
111111
backwards_search.contains(k)
112112
&& backwards_search.x_pos_at_diagonal(k) <= token_idx1
113113
}) {
114+
#[cfg(test)]
115+
cov_mark::hit!(ODD_SPLIT);
114116
match res {
115117
SearchResult::Snake => found_snake = true,
116118
SearchResult::Found {
@@ -135,6 +137,8 @@ impl Myers {
135137
if let Some(res) = backwards_search.run(file1, file2, |k, token_idx1| {
136138
forward_search.contains(k) && token_idx1 <= forward_search.x_pos_at_diagonal(k)
137139
}) {
140+
#[cfg(test)]
141+
cov_mark::hit!(EVEN_SPLIT);
138142
match res {
139143
SearchResult::Snake => found_snake = true,
140144
SearchResult::Found {

src/myers/middle_snake.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::util::{common_postfix, common_prefix};
66
const SNAKE_CNT: u32 = 20;
77
const K_HEUR: u32 = 4;
88

9+
#[derive(Debug)]
910
pub struct MiddleSnakeSearch<const BACK: bool> {
1011
kvec: NonNull<i32>,
1112
kmin: i32,
@@ -98,6 +99,8 @@ impl<const BACK: bool> MiddleSnakeSearch<BACK> {
9899
let mut res = None;
99100
let mut k = self.kmax;
100101
while k >= self.kmin {
102+
#[cfg(test)]
103+
cov_mark::hit!(SPLIT_SEARCH_ITER);
101104
let mut token_idx1 = if BACK {
102105
if self.x_pos_at_diagonal(k - 1) < self.x_pos_at_diagonal(k + 1) {
103106
self.x_pos_at_diagonal(k - 1)
@@ -249,6 +252,7 @@ impl<const BACK: bool> MiddleSnakeSearch<BACK> {
249252
}
250253
}
251254

255+
#[derive(Debug)]
252256
pub enum SearchResult {
253257
Snake,
254258
Found { token_idx1: i32, token_idx2: i32 },

src/tests.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,71 @@ fn foo() -> Bar{
118118
}
119119
}
120120

121+
#[test]
122+
fn myers_is_odd() {
123+
let before = "a\nb\nx\ny\nx\n";
124+
let after = "b\na\nx\ny\n";
125+
126+
cov_mark::check!(ODD_SPLIT);
127+
// if the check for odd doesn't work then
128+
// we still find the correct result but the number of search
129+
// iterations increases
130+
cov_mark::check_count!(SPLIT_SEARCH_ITER, 9);
131+
let input = InternedInput::new(before, after);
132+
let diff = Diff::compute(Algorithm::Myers, &input);
133+
expect![[r#"
134+
@@ -1,5 +1,4 @@
135+
-a
136+
b
137+
+a
138+
x
139+
y
140+
-x
141+
"#]]
142+
.assert_eq(
143+
&diff
144+
.unified_diff(
145+
&BasicLineDiffPrinter(&input.interner),
146+
UnifiedDiffConfig::default(),
147+
&input,
148+
)
149+
.to_string(),
150+
);
151+
}
152+
#[test]
153+
fn myers_is_even() {
154+
let before = "a\nb\nx\nx\ny\n";
155+
let after = "b\na\nx\ny\nx\n";
156+
157+
cov_mark::check!(EVEN_SPLIT);
158+
// if the check for is_odd incorrectly always true then we take a fastpath
159+
// when we shouldn't which always leads to inifite iterations/recursion
160+
// still we check the number of iterations here in case the search
161+
// is buggy in more subtle ways
162+
cov_mark::check_count!(SPLIT_SEARCH_ITER, 15);
163+
let input = InternedInput::new(before, after);
164+
let diff = Diff::compute(Algorithm::Myers, &input);
165+
expect![[r#"
166+
@@ -1,5 +1,5 @@
167+
-a
168+
b
169+
-x
170+
+a
171+
x
172+
y
173+
+x
174+
"#]]
175+
.assert_eq(
176+
&diff
177+
.unified_diff(
178+
&BasicLineDiffPrinter(&input.interner),
179+
UnifiedDiffConfig::default(),
180+
&input,
181+
)
182+
.to_string(),
183+
);
184+
}
185+
121186
#[test]
122187
fn identical_files() {
123188
let file = r#"fn foo() -> Bar{

0 commit comments

Comments
 (0)