Skip to content

Commit 4736a61

Browse files
committed
gix-blame: optimize process_changes to avoid redundant work
Change `process_changes` to take `&[Change]` instead of `Vec<Change>`, eliminating the `changes.clone()` heap allocation at every call site. Replace the O(H×C) restart-from-beginning approach with a cursor that advances through the changes list across hunks. Non-suspect hunks are now skipped immediately. When the rare case of overlapping suspect ranges is detected (from merge blame convergence), the cursor safely resets to maintain correctness.
1 parent 743b433 commit 4736a61

File tree

3 files changed

+45
-28
lines changed

3 files changed

+45
-28
lines changed

gix-blame/src/file/function.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ pub fn incremental(
334334
options.diff_algorithm,
335335
&mut stats,
336336
)?;
337-
hunks_to_blame = process_changes(hunks_to_blame, changes.clone(), suspect, *parent_id);
337+
hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, *parent_id);
338338
if let Some(ref mut blame_path) = blame_path {
339339
let has_blame_been_passed = hunks_to_blame.iter().any(|hunk| hunk.has_suspect(parent_id));
340340

@@ -366,7 +366,7 @@ pub fn incremental(
366366
options.diff_algorithm,
367367
&mut stats,
368368
)?;
369-
hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, *parent_id);
369+
hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, *parent_id);
370370

371371
let mut has_blame_been_passed = false;
372372

gix-blame/src/file/mod.rs

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -322,24 +322,36 @@ fn process_change(
322322
/// Consume `hunks_to_blame` and `changes` to pair up matches ranges (also overlapping) with each other.
323323
/// Once a match is found, it's pushed onto `out`.
324324
///
325-
/// `process_changes` assumes that ranges coming from the same *Source File* can and do
326-
/// occasionally overlap. If it were a desirable property of the blame algorithm as a whole to
327-
/// never have two different lines from a *Blamed File* mapped to the same line in a *Source File*,
328-
/// this property would need to be enforced at a higher level than `process_changes`.
329-
/// Then the nested loops could potentially be flattened into one.
325+
/// Uses a cursor to avoid restarting the changes iteration from the beginning for each hunk.
326+
/// When suspect ranges overlap (rare, from merge blame convergence), the cursor resets to
327+
/// maintain correctness.
330328
fn process_changes(
331329
hunks_to_blame: Vec<UnblamedHunk>,
332-
changes: Vec<Change>,
330+
changes: &[Change],
333331
suspect: ObjectId,
334332
parent: ObjectId,
335333
) -> Vec<UnblamedHunk> {
336334
let mut new_hunks_to_blame = Vec::new();
335+
let mut offset_in_destination = Offset::Added(0);
336+
let mut changes_pos = 0;
337+
let mut last_suspect_end: Option<u32> = None;
338+
339+
for hunk in hunks_to_blame {
340+
if !hunk.has_suspect(&suspect) {
341+
new_hunks_to_blame.push(hunk);
342+
continue;
343+
}
337344

338-
for mut hunk in hunks_to_blame.into_iter().map(Some) {
339-
let mut offset_in_destination = Offset::Added(0);
345+
let suspect_range = hunk.get_range(&suspect).expect("has_suspect was true");
346+
if last_suspect_end.is_some_and(|end| suspect_range.start < end) {
347+
changes_pos = 0;
348+
offset_in_destination = Offset::Added(0);
349+
}
350+
last_suspect_end = Some(suspect_range.end);
340351

341-
let mut changes_iter = changes.iter().cloned();
342-
let mut change = changes_iter.next();
352+
let mut hunk = Some(hunk);
353+
let mut pos = changes_pos;
354+
let mut change: Option<Change> = changes.get(pos).cloned();
343355

344356
loop {
345357
(hunk, change) = process_change(
@@ -351,12 +363,17 @@ fn process_changes(
351363
change,
352364
);
353365

354-
change = change.or_else(|| changes_iter.next());
366+
if change.is_none() {
367+
pos += 1;
368+
change = changes.get(pos).cloned();
369+
}
355370

356371
if hunk.is_none() {
357372
break;
358373
}
359374
}
375+
376+
changes_pos = pos;
360377
}
361378
new_hunks_to_blame
362379
}

gix-blame/src/file/tests.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ mod process_changes {
742742
fn nothing() {
743743
let suspect = zero_sha();
744744
let parent = one_sha();
745-
let new_hunks_to_blame = process_changes(vec![], vec![], suspect, parent);
745+
let new_hunks_to_blame = process_changes(vec![], &[], suspect, parent);
746746

747747
assert_eq!(new_hunks_to_blame, []);
748748
}
@@ -753,7 +753,7 @@ mod process_changes {
753753
let parent = one_sha();
754754
let hunks_to_blame = vec![(0..4, suspect).into()];
755755
let changes = vec![Change::AddedOrReplaced(0..4, 0)];
756-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
756+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
757757

758758
assert_eq!(new_hunks_to_blame, [(0..4, suspect).into()]);
759759
}
@@ -764,7 +764,7 @@ mod process_changes {
764764
let parent = one_sha();
765765
let hunks_to_blame = vec![(0..6, suspect).into()];
766766
let changes = vec![Change::AddedOrReplaced(0..4, 0), Change::Unchanged(4..6)];
767-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
767+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
768768

769769
assert_eq!(
770770
new_hunks_to_blame,
@@ -782,7 +782,7 @@ mod process_changes {
782782
Change::AddedOrReplaced(2..4, 0),
783783
Change::Unchanged(4..6),
784784
];
785-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
785+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
786786

787787
assert_eq!(
788788
new_hunks_to_blame,
@@ -804,7 +804,7 @@ mod process_changes {
804804
Change::AddedOrReplaced(1..4, 0),
805805
Change::Unchanged(4..6),
806806
];
807-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
807+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
808808

809809
assert_eq!(
810810
new_hunks_to_blame,
@@ -822,7 +822,7 @@ mod process_changes {
822822
let parent = one_sha();
823823
let hunks_to_blame = vec![(0..6, suspect).into()];
824824
let changes = vec![Change::AddedOrReplaced(0..1, 0)];
825-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
825+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
826826

827827
assert_eq!(
828828
new_hunks_to_blame,
@@ -836,7 +836,7 @@ mod process_changes {
836836
let parent = one_sha();
837837
let hunks_to_blame = vec![(2..6, suspect, 0..4).into()];
838838
let changes = vec![Change::AddedOrReplaced(0..1, 0)];
839-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
839+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
840840

841841
assert_eq!(
842842
new_hunks_to_blame,
@@ -850,7 +850,7 @@ mod process_changes {
850850
let parent = one_sha();
851851
let hunks_to_blame = vec![(0..6, suspect).into()];
852852
let changes = vec![Change::AddedOrReplaced(0..4, 3), Change::Unchanged(4..6)];
853-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
853+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
854854

855855
assert_eq!(
856856
new_hunks_to_blame,
@@ -864,7 +864,7 @@ mod process_changes {
864864
let parent = one_sha();
865865
let hunks_to_blame = vec![(4..6, suspect, 3..5).into()];
866866
let changes = vec![Change::AddedOrReplaced(0..3, 0), Change::Unchanged(3..5)];
867-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
867+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
868868

869869
assert_eq!(new_hunks_to_blame, [(4..6, parent, 0..2).into()]);
870870
}
@@ -875,7 +875,7 @@ mod process_changes {
875875
let parent = one_sha();
876876
let hunks_to_blame = vec![(1..3, suspect, 0..2).into()];
877877
let changes = vec![Change::AddedOrReplaced(0..1, 2)];
878-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
878+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
879879

880880
assert_eq!(
881881
new_hunks_to_blame,
@@ -893,7 +893,7 @@ mod process_changes {
893893
Change::Unchanged(2..3),
894894
Change::AddedOrReplaced(3..4, 0),
895895
];
896-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
896+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
897897

898898
assert_eq!(
899899
new_hunks_to_blame,
@@ -915,7 +915,7 @@ mod process_changes {
915915
Change::AddedOrReplaced(16..17, 0),
916916
Change::Unchanged(17..37),
917917
];
918-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
918+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
919919

920920
assert_eq!(
921921
new_hunks_to_blame,
@@ -938,7 +938,7 @@ mod process_changes {
938938
Change::AddedOrReplaced(6..9, 0),
939939
Change::Unchanged(9..11),
940940
];
941-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
941+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
942942

943943
assert_eq!(
944944
new_hunks_to_blame,
@@ -958,7 +958,7 @@ mod process_changes {
958958
let parent = one_sha();
959959
let hunks_to_blame = vec![(0..4, suspect).into(), (4..7, suspect).into()];
960960
let changes = vec![Change::Deleted(0, 3), Change::AddedOrReplaced(0..4, 0)];
961-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
961+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
962962

963963
assert_eq!(
964964
new_hunks_to_blame,
@@ -972,7 +972,7 @@ mod process_changes {
972972
let parent = one_sha();
973973
let hunks_to_blame = vec![(13..16, suspect).into(), (10..17, suspect).into()];
974974
let changes = vec![Change::AddedOrReplaced(10..14, 0)];
975-
let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent);
975+
let new_hunks_to_blame = process_changes(hunks_to_blame, &changes, suspect, parent);
976976

977977
assert_eq!(
978978
new_hunks_to_blame,

0 commit comments

Comments
 (0)