From ed168635c96ef4cbf7c6d456c85a8ce73cf83634 Mon Sep 17 00:00:00 2001 From: Wilfred Hughes Date: Mon, 13 Jun 2022 21:42:52 +0100 Subject: [PATCH] Use 32-bit integers for line numbers This saves 16% time, 18% memory and 1% instruction counts from the sample files in #293. These are large files, treated as textual diffs, so reducing the storage per-line is significant. Files with structural diffing, such as sample_files/slow_before.rs, are unchanged. Fixes #293 Closes #297 --- src/diff/dijkstra.rs | 4 ++-- src/diff/sliders.rs | 2 +- src/display/context.rs | 10 +++++----- src/display/hunks.rs | 2 +- src/display/inline.rs | 16 ++++++++++++---- src/display/side_by_side.rs | 24 +++++++++++++++--------- src/display/style.rs | 4 +++- src/lines.rs | 19 ++++++++++++------- 8 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/diff/dijkstra.rs b/src/diff/dijkstra.rs index 9627f63564..85313ab4f1 100644 --- a/src/diff/dijkstra.rs +++ b/src/diff/dijkstra.rs @@ -223,7 +223,7 @@ mod tests { use itertools::Itertools; use typed_arena::Arena; - fn pos_helper(line: usize) -> Vec { + fn pos_helper(line: u32) -> Vec { vec![SingleLineSpan { line: line.into(), start_col: 0, @@ -231,7 +231,7 @@ mod tests { }] } - fn col_helper(line: usize, col: u32) -> Vec { + fn col_helper(line: u32, col: u32) -> Vec { vec![SingleLineSpan { line: line.into(), start_col: col, diff --git a/src/diff/sliders.rs b/src/diff/sliders.rs index 42cc20c44d..e81123200b 100644 --- a/src/diff/sliders.rs +++ b/src/diff/sliders.rs @@ -538,7 +538,7 @@ fn slide_to_next_node<'a>( /// Return the distance between two syntax nodes, as a tuple of number /// of lines and number of columns. -fn distance_between(prev: &Syntax, next: &Syntax) -> (usize, u32) { +fn distance_between(prev: &Syntax, next: &Syntax) -> (u32, u32) { let prev_pos = prev.last_line_span(); let next_pos = next.first_line_span(); diff --git a/src/display/context.rs b/src/display/context.rs index c7d08065ff..91d82b10cb 100644 --- a/src/display/context.rs +++ b/src/display/context.rs @@ -98,16 +98,16 @@ fn add_ends( let mut lhs_line: LineNumber = (lhs_max.0 + 1).into(); let mut rhs_line: LineNumber = (rhs_max.0 + 1).into(); - while lhs_line.0 < lhs_lines.len() && rhs_line.0 < rhs_lines.len() { + while (lhs_line.0 as usize) < lhs_lines.len() && (rhs_line.0 as usize) < rhs_lines.len() { res.push((Some(lhs_line), Some(rhs_line))); lhs_line = (lhs_line.0 + 1).into(); rhs_line = (rhs_line.0 + 1).into(); } - while lhs_line.0 < lhs_lines.len() { + while (lhs_line.0 as usize) < lhs_lines.len() { res.push((Some(lhs_line), None)); lhs_line = (lhs_line.0 + 1).into(); } - while rhs_line.0 < rhs_lines.len() { + while (rhs_line.0 as usize) < rhs_lines.len() { res.push((None, Some(rhs_line))); rhs_line = (rhs_line.0 + 1).into(); } @@ -229,7 +229,7 @@ fn match_blanks_between( let mut res = vec![]; while current_lhs > prev.0 && current_rhs > prev.1 { - if lhs_lines[current_lhs.0] == "" && rhs_lines[current_rhs.0] == "" { + if lhs_lines[current_lhs.as_usize()] == "" && rhs_lines[current_rhs.as_usize()] == "" { res.push((Some(current_lhs), Some(current_rhs))); current_lhs = (current_lhs.0 - 1).into(); @@ -258,7 +258,7 @@ fn match_blanks_before( let mut res = vec![]; loop { - if lhs_lines[current_lhs.0] == "" && rhs_lines[current_rhs.0] == "" { + if lhs_lines[current_lhs.as_usize()] == "" && rhs_lines[current_rhs.as_usize()] == "" { res.push((Some(current_lhs), Some(current_rhs))); if current_lhs.0 == 0 || current_rhs.0 == 0 { diff --git a/src/display/hunks.rs b/src/display/hunks.rs index 2a8f101abf..3b952fd6fc 100644 --- a/src/display/hunks.rs +++ b/src/display/hunks.rs @@ -3,7 +3,7 @@ /// The maximum number of lines that may occur between changed lines in a hunk. /// /// If we exceed this, the lines are stored in separate hunks. -const MAX_DISTANCE: usize = 4; +const MAX_DISTANCE: u32 = 4; use std::collections::HashSet; diff --git a/src/display/inline.rs b/src/display/inline.rs index 63178c7e6c..e0fb34f699 100644 --- a/src/display/inline.rs +++ b/src/display/inline.rs @@ -76,7 +76,11 @@ pub fn print( for (lhs_line, _) in before_lines { if let Some(lhs_line) = lhs_line { - println!("{} {}", format_line_num(lhs_line), lhs_lines[lhs_line.0]); + println!( + "{} {}", + format_line_num(lhs_line), + lhs_lines[lhs_line.as_usize()] + ); } } @@ -85,7 +89,7 @@ pub fn print( println!( "{} {}", format_line_num(*lhs_line).red().bold(), - lhs_lines[lhs_line.0] + lhs_lines[lhs_line.as_usize()] ); } } @@ -94,14 +98,18 @@ pub fn print( println!( " {}{}", format_line_num(*rhs_line).green().bold(), - rhs_lines[rhs_line.0] + rhs_lines[rhs_line.as_usize()] ); } } for (_, rhs_line) in &after_lines { if let Some(rhs_line) = rhs_line { - println!(" {}{}", format_line_num(*rhs_line), rhs_lines[rhs_line.0]); + println!( + " {}{}", + format_line_num(*rhs_line), + rhs_lines[rhs_line.as_usize()] + ); } } println!(); diff --git a/src/display/side_by_side.rs b/src/display/side_by_side.rs index 4df078ad57..72f5ce73fd 100644 --- a/src/display/side_by_side.rs +++ b/src/display/side_by_side.rs @@ -85,7 +85,7 @@ fn display_single_column( is_lhs: bool, display_options: &DisplayOptions, ) -> String { - let column_width = format_line_num(src.lines().count().into()).len(); + let column_width = format_line_num((src.lines().count() as u32).into()).len(); let mut result = String::with_capacity(src.len()); result.push_str(&style::header( @@ -105,7 +105,7 @@ fn display_single_column( for (i, line) in src.lines().enumerate() { result.push_str( - &format_line_num_padded(i.into(), column_width) + &format_line_num_padded((i as u32).into(), column_width) .style(style) .to_string(), ); @@ -197,11 +197,17 @@ impl SourceDimensions { for (lhs_line_num, rhs_line_num) in line_nums { if let Some(lhs_line_num) = lhs_line_num { lhs_max_line = max(lhs_max_line, *lhs_line_num); - lhs_max_content = max(lhs_max_content, codepoint_len(lhs_lines[lhs_line_num.0])); + lhs_max_content = max( + lhs_max_content, + codepoint_len(lhs_lines[lhs_line_num.as_usize()]), + ); } if let Some(rhs_line_num) = rhs_line_num { rhs_max_line = max(rhs_max_line, *rhs_line_num); - rhs_max_content = max(rhs_max_content, codepoint_len(rhs_lines[rhs_line_num.0])); + rhs_max_content = max( + rhs_max_content, + codepoint_len(rhs_lines[rhs_line_num.as_usize()]), + ); } } @@ -283,7 +289,7 @@ fn highlight_as_novel( return true; } - let line_content = lines.get(line_num.0).map(|s| str::trim(s)); + let line_content = lines.get(line_num.as_usize()).map(|s| str::trim(s)); // If this is a blank line without a corresponding line on the // other side, highlight it too. This helps highlight novel // blank lines. @@ -437,7 +443,7 @@ pub fn print( if no_lhs_changes && !show_both { match rhs_line_num { Some(rhs_line_num) => { - let rhs_line = &rhs_colored_lines[rhs_line_num.0]; + let rhs_line = &rhs_colored_lines[rhs_line_num.as_usize()]; if same_lines { println!("{}{}", display_rhs_line_num, rhs_line); } else { @@ -457,7 +463,7 @@ pub fn print( } else if no_rhs_changes && !show_both { match lhs_line_num { Some(lhs_line_num) => { - let lhs_line = &lhs_colored_lines[lhs_line_num.0]; + let lhs_line = &lhs_colored_lines[lhs_line_num.as_usize()]; if same_lines { println!("{}{}", display_lhs_line_num, lhs_line); } else { @@ -474,7 +480,7 @@ pub fn print( } else { let lhs_line = match lhs_line_num { Some(lhs_line_num) => split_and_apply( - lhs_lines[lhs_line_num.0], + lhs_lines[lhs_line_num.as_usize()], source_dims.lhs_content_width, display_options.use_color, lhs_highlights.get(&lhs_line_num).unwrap_or(&vec![]), @@ -484,7 +490,7 @@ pub fn print( }; let rhs_line = match rhs_line_num { Some(rhs_line_num) => split_and_apply( - rhs_lines[rhs_line_num.0], + rhs_lines[rhs_line_num.as_usize()], source_dims.rhs_content_width, display_options.use_color, rhs_highlights.get(&rhs_line_num).unwrap_or(&vec![]), diff --git a/src/display/style.rs b/src/display/style.rs index ca1855d55e..7cc0ec4eca 100644 --- a/src/display/style.rs +++ b/src/display/style.rs @@ -221,7 +221,9 @@ fn apply(s: &str, styles: &[(SingleLineSpan, Style)]) -> String { let mut res = String::with_capacity(s.len()); for (i, line) in s.lines().enumerate() { - let ranges = ranges_by_line.remove(&i.into()).unwrap_or_default(); + let ranges = ranges_by_line + .remove(&(i as u32).into()) + .unwrap_or_default(); res.push_str(&apply_line(line, &ranges)); res.push('\n'); } diff --git a/src/lines.rs b/src/lines.rs index 38ab5dcbee..5e2fe47b1d 100644 --- a/src/lines.rs +++ b/src/lines.rs @@ -9,12 +9,16 @@ use std::{cmp::Ordering, fmt}; /// /// Zero-indexed internally. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct LineNumber(pub usize); +pub struct LineNumber(pub u32); impl LineNumber { - pub fn one_indexed(self) -> usize { + pub fn one_indexed(self) -> u32 { self.0 + 1 } + + pub fn as_usize(self) -> usize { + self.0 as usize + } } impl fmt::Debug for LineNumber { @@ -27,8 +31,8 @@ impl fmt::Debug for LineNumber { } } -impl From for LineNumber { - fn from(number: usize) -> Self { +impl From for LineNumber { + fn from(number: u32) -> Self { Self(number) } } @@ -97,7 +101,7 @@ impl NewlinePositions { for idx in first_idx..=last_idx { let (line_start, line_end) = self.positions[idx]; res.push(SingleLineSpan { - line: idx.into(), + line: (idx as u32).into(), start_col: if line_start > region_start { 0 } else { @@ -163,10 +167,11 @@ pub trait MaxLine { impl> MaxLine for S { fn max_line(&self) -> LineNumber { - self.as_ref() + (self + .as_ref() .trim_end() // Remove extra trailing whitespaces. .split('\n') // Split by `\n` to calculate lines. - .count() + .count() as u32) .sub(1) // Sub 1 to make zero-indexed LineNumber .into() }