Ensure we only calculate opposite positions once

Previously this was computed per-hunk, which was wasteful and slow
when there are many hunks.

CHANGELOG.md

@@ -12,7 +12,12 @@ Improved language detection for files with bash/sh syntax.
 
 ### Integration
 
-Fixed a crash when on Mercurial diffs when a whole file has been removed.
+Fixed a crash when on Mercurial diffs when a whole file has been
+removed.
+
+### Display
+
+Improved display performance when there are a large number of hunks.
 
 ## 0.15 (released 6 January 2022)
 

src/context.rs

@@ -150,7 +150,7 @@ pub fn flip_tuples<Tx: Copy, Ty: Copy>(items: &[(Tx, Ty)]) -> Vec<(Ty, Tx)> {
 /// 122    91 (closest match)
 fn after_with_opposites(
     after_lines: &[LineNumber],
-    opposite_lines: HashMap<LineNumber, HashSet<LineNumber>>,
+    opposite_lines: &HashMap<LineNumber, HashSet<LineNumber>>,
     prev_max_opposite: Option<LineNumber>,
     max_opposite: LineNumber,
 ) -> Vec<(Option<LineNumber>, Option<LineNumber>)> {
@@ -196,17 +196,14 @@ fn after_with_opposites(
 
 pub fn calculate_context(
     lines: &[(Option<LineNumber>, Option<LineNumber>)],
-    lhs_mps: &[MatchedPos],
-    rhs_mps: &[MatchedPos],
+    opposite_to_lhs: &HashMap<LineNumber, HashSet<LineNumber>>,
+    opposite_to_rhs: &HashMap<LineNumber, HashSet<LineNumber>>,
     max_lhs_src_line: LineNumber,
     max_rhs_src_line: LineNumber,
 ) -> (
     Vec<(Option<LineNumber>, Option<LineNumber>)>,
     Vec<(Option<LineNumber>, Option<LineNumber>)>,
 ) {
-    let opposite_to_lhs = opposite_positions(lhs_mps);
-    let opposite_to_rhs = opposite_positions(rhs_mps);
-
     let before_lines: Vec<_> = match lines.first() {
         Some(first_line) => match *first_line {
             (Some(lhs_line), _) => {
@@ -266,13 +263,18 @@ pub fn calculate_context(
 
 pub fn add_context(
     lines: &[(Option<LineNumber>, Option<LineNumber>)],
-    lhs_mps: &[MatchedPos],
-    rhs_mps: &[MatchedPos],
+    opposite_to_lhs: &HashMap<LineNumber, HashSet<LineNumber>>,
+    opposite_to_rhs: &HashMap<LineNumber, HashSet<LineNumber>>,
     max_lhs_src_line: LineNumber,
     max_rhs_src_line: LineNumber,
 ) -> Vec<(Option<LineNumber>, Option<LineNumber>)> {
-    let (before_lines, after_lines) =
-        calculate_context(lines, lhs_mps, rhs_mps, max_lhs_src_line, max_rhs_src_line);
+    let (before_lines, after_lines) = calculate_context(
+        lines,
+        opposite_to_lhs,
+        opposite_to_rhs,
+        max_lhs_src_line,
+        max_rhs_src_line,
+    );
 
     before_lines
         .into_iter()

src/hunks.rs

@@ -11,7 +11,7 @@ use std::{
 };
 
 use crate::{
-    context::{add_context, calculate_context, flip_tuples, opposite_positions},
+    context::{add_context, calculate_context, flip_tuples},
     lines::LineNumber,
     syntax::{zip_pad_shorter, MatchedPos},
 };
@@ -127,8 +127,8 @@ fn extract_lines(hunk: &Hunk) -> Vec<(Option<LineNumber>, Option<LineNumber>)> {
 
 pub fn merge_adjacent(
     hunks: &[Hunk],
-    lhs_mps: &[MatchedPos],
-    rhs_mps: &[MatchedPos],
+    opposite_to_lhs: &HashMap<LineNumber, HashSet<LineNumber>>,
+    opposite_to_rhs: &HashMap<LineNumber, HashSet<LineNumber>>,
     max_lhs_src_line: LineNumber,
     max_rhs_src_line: LineNumber,
 ) -> Vec<Hunk> {
@@ -143,8 +143,13 @@ pub fn merge_adjacent(
         let mut rhs_lines: HashSet<LineNumber> = HashSet::new();
 
         let lines = extract_lines(hunk);
-        let contextual_lines =
-            add_context(&lines, lhs_mps, rhs_mps, max_lhs_src_line, max_rhs_src_line);
+        let contextual_lines = add_context(
+            &lines,
+            opposite_to_lhs,
+            opposite_to_rhs,
+            max_lhs_src_line,
+            max_rhs_src_line,
+        );
         for (lhs_line, rhs_line) in contextual_lines {
             if let Some(lhs_line) = lhs_line {
                 lhs_lines.insert(lhs_line);
@@ -672,8 +677,8 @@ fn fill_matched_lines(
 
 pub fn aligned_lines_from_hunk(
     hunk: &Hunk,
-    lhs_mps: &[MatchedPos],
-    rhs_mps: &[MatchedPos],
+    opposite_to_lhs: &HashMap<LineNumber, HashSet<LineNumber>>,
+    opposite_to_rhs: &HashMap<LineNumber, HashSet<LineNumber>>,
     max_lhs_src_line: LineNumber,
     max_rhs_src_line: LineNumber,
 ) -> Vec<(Option<LineNumber>, Option<LineNumber>)> {
@@ -681,8 +686,8 @@ pub fn aligned_lines_from_hunk(
 
     let (before_context, after_context) = calculate_context(
         &hunk_lines,
-        lhs_mps,
-        rhs_mps,
+        opposite_to_lhs,
+        opposite_to_rhs,
         max_lhs_src_line,
         max_rhs_src_line,
     );
@@ -693,12 +698,9 @@ pub fn aligned_lines_from_hunk(
     let mut res: Vec<(Option<LineNumber>, Option<LineNumber>)> = vec![];
     res.extend(before_context);
 
-    let matched_lhs_lines = opposite_positions(rhs_mps);
-    let matched_rhs_lines = opposite_positions(lhs_mps);
-
     if let (Some(start_pair), Some(end_pair)) = (start_pair, end_pair) {
         // Fill lines between.
-        let aligned_between = fill_aligned(start_pair, end_pair, &matched_rhs_lines);
+        let aligned_between = fill_aligned(start_pair, end_pair, &opposite_to_rhs);
 
         // TODO: align based on blank lines too.
 
@@ -711,13 +713,13 @@ pub fn aligned_lines_from_hunk(
 
         let aligned_between = match first_novel {
             (Some(lhs_start), _) => {
-                fill_matched_lines(lhs_start, max_lhs_src_line, hunk_end, &matched_rhs_lines)
+                fill_matched_lines(lhs_start, max_lhs_src_line, hunk_end, opposite_to_lhs)
             }
             (_, Some(rhs_start)) => flip_tuples(&fill_matched_lines(
                 rhs_start,
                 max_rhs_src_line,
                 hunk_end,
-                &matched_lhs_lines,
+                opposite_to_rhs,
             )),
             (None, None) => unreachable!(),
         };
@@ -735,7 +737,7 @@ mod tests {
     use super::*;
     use crate::{
         positions::SingleLineSpan,
-        syntax::{AtomKind, MatchKind, TokenKind},
+        syntax::{AtomKind, MatchKind, TokenKind}, context::opposite_positions,
     };
     use pretty_assertions::assert_eq;
 
@@ -892,7 +894,18 @@ mod tests {
             },
         ];
 
-        let res = aligned_lines_from_hunk(&hunk, &lhs_mps, &rhs_mps, 1.into(), 2.into());
+        // TODO: Just build a HashMap diretly instead of positions
+        // first for this test.
+        let opposite_to_lhs = opposite_positions(&lhs_mps);
+        let opposite_to_rhs = opposite_positions(&rhs_mps);
+
+        let res = aligned_lines_from_hunk(
+            &hunk,
+            &opposite_to_lhs,
+            &opposite_to_rhs,
+            1.into(),
+            2.into(),
+        );
         assert_eq!(
             res,
             vec![
@@ -1028,7 +1041,18 @@ mod tests {
             },
         ];
 
-        let res = aligned_lines_from_hunk(&hunk, &lhs_mps, &rhs_mps, 1.into(), 3.into());
+        // TODO: Just build a HashMap diretly instead of positions
+        // first for this test.
+        let opposite_to_lhs = opposite_positions(&lhs_mps);
+        let opposite_to_rhs = opposite_positions(&rhs_mps);
+
+        let res = aligned_lines_from_hunk(
+            &hunk,
+            &opposite_to_lhs,
+            &opposite_to_rhs,
+            1.into(),
+            3.into(),
+        );
         assert_eq!(
             res,
             vec![

src/inline.rs

@@ -1,7 +1,7 @@
 //! Inline, or "unified" diff display.
 
 use crate::{
-    context::calculate_context,
+    context::{calculate_context, opposite_positions},
     hunks::Hunk,
     lines::{format_line_num, MaxLine},
     style::apply_colors,
@@ -24,12 +24,15 @@ pub fn display(
 
     let mut res = String::new();
 
+    let opposite_to_lhs = opposite_positions(lhs_positions);
+    let opposite_to_rhs = opposite_positions(rhs_positions);
+
     for hunk in hunks {
         let hunk_lines = hunk.lines.clone();
         let (before_lines, after_lines) = calculate_context(
             &hunk_lines,
-            lhs_positions,
-            rhs_positions,
+            &opposite_to_lhs,
+            &opposite_to_rhs,
             // TODO: repeatedly calculating the maximum is wasteful.
             lhs_src.max_line(),
             rhs_src.max_line(),

src/main.rs

@@ -24,6 +24,7 @@ mod tree_sitter_parser;
 extern crate log;
 
 use crate::hunks::{matched_pos_to_hunks, merge_adjacent};
+use context::opposite_positions;
 use files::read_files_or_die;
 use guess_language::guess;
 use log::info;
@@ -338,11 +339,15 @@ fn print_diff_result(summary: &DiffResult) {
         return;
     }
 
+    let opposite_to_lhs = opposite_positions(&summary.lhs_positions);
+    let opposite_to_rhs = opposite_positions(&summary.rhs_positions);
+
+
     let hunks = matched_pos_to_hunks(&summary.lhs_positions, &summary.rhs_positions);
     let hunks = merge_adjacent(
         &hunks,
-        &summary.lhs_positions,
-        &summary.rhs_positions,
+        &opposite_to_lhs,
+        &opposite_to_rhs,
         summary.lhs_src.max_line(),
         summary.rhs_src.max_line(),
     );

src/side_by_side.rs

@@ -14,7 +14,7 @@ use crate::{
     lines::{codepoint_len, format_line_num, LineNumber, MaxLine},
     positions::SingleLineSpan,
     style::{self, apply_colors, color_positions, split_and_apply, Style},
-    syntax::{zip_pad_shorter, MatchedPos},
+    syntax::{zip_pad_shorter, MatchedPos}, context::opposite_positions,
 };
 
 const SPACER: &str = " ";
@@ -250,6 +250,9 @@ pub fn display_hunks(
     lhs_mps: &[MatchedPos],
     rhs_mps: &[MatchedPos],
 ) -> String {
+    let opposite_to_lhs = opposite_positions(lhs_mps);
+    let opposite_to_rhs = opposite_positions(rhs_mps);
+
     let lhs_colored_src = apply_colors(lhs_src, true, lhs_mps);
     let rhs_colored_src = apply_colors(rhs_src, false, rhs_mps);
 
@@ -284,8 +287,8 @@ pub fn display_hunks(
 
         let aligned_lines = aligned_lines_from_hunk(
             hunk,
-            lhs_mps,
-            rhs_mps,
+            &opposite_to_lhs,
+            &opposite_to_rhs,
             lhs_src.max_line(),
             rhs_src.max_line(),
         );