hl-valdemar/vamp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

perf: avoid eager diff loading and full-file typing work

Browse Source
This commit is contained in:
Valdemar Høgsvig Lorenzen
2026-10-11 15:06:05 +02:00
parent 7b4bc80538
commit 2e05725a38
11 changed files with 1881 additions and 386 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/diff.rs
+580 -31
View File
@@ -44,6 +44,7 @@ pub enum RevisionTarget {
Working,
Commit(String),
Remote { remote: String, reference: String },
ListRemote(String),
FetchRemote(String),
}
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -78,6 +79,8 @@ pub struct ChangedFile {
pub path: PathBuf,
pub old_path: Option<PathBuf>,
pub status: FileStatus,
/// False means text, EOF flags, hunks and notices have not been prepared.
pub loaded: bool,
pub before: Vec<String>,
pub after: Vec<String>,
pub before_eof: bool,
@@ -91,22 +94,33 @@ pub struct Comparison {
pub after: String,
pub files: Vec<ChangedFile>,
pub root: PathBuf,
/// Regular working-After candidates; loaded binary details are excluded.
pub editable_paths: BTreeSet<PathBuf>,
}
#[derive(Debug)]
pub enum Operation {
Discover,
ListRemote(String),
FetchRemote(String),
Compare {
before: Revision,
after: Revision,
buffers: Box<[(PathBuf, String)]>,
},
LoadFile {
comparison_id: u64,
file_index: usize,
},
}
#[derive(Debug)]
pub enum Outcome {
Catalog(Catalog),
Comparison(Comparison),
File {
comparison_id: u64,
file_index: usize,
file: ChangedFile,
},
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Phase {
@@ -158,6 +172,10 @@ struct SelectorPosition {
enum RequestKind {
Catalog,
Comparison,
File {
comparison_id: u64,
file_index: usize,
},
}
#[derive(Debug)]
@@ -196,6 +214,7 @@ pub struct DiffState {
before_position: Option<SelectorPosition>,
max_before: usize,
max_after: usize,
after_widths: Vec<usize>,
}
impl DiffState {
@@ -235,6 +254,7 @@ impl DiffState {
before_position: None,
max_before: 0,
max_after: 0,
after_widths: Vec::new(),
}
}
@@ -251,6 +271,13 @@ impl DiffState {
fn queue(&mut self, operation: Operation) {
self.request_kind = Some(match operation {
Operation::Compare { .. } => RequestKind::Comparison,
Operation::LoadFile {
comparison_id,
file_index,
} => RequestKind::File {
comparison_id,
file_index,
},
_ => RequestKind::Catalog,
});
self.pending = Some(operation);
@@ -259,6 +286,24 @@ impl DiffState {
}
pub(crate) fn apply_result(&mut self, id: u64, result: Result<Outcome, String>) {
if self.request_kind.is_none() {
return;
}
if let Ok(Outcome::File {
comparison_id,
file_index,
..
}) = &result
&& (self.comparison_id != Some(*comparison_id)
|| self.file != *file_index
|| self.request_kind
!= Some(RequestKind::File {
comparison_id: *comparison_id,
file_index: *file_index,
}))
{
return;
}
let expected = self.request_kind.take();
self.loading = false;
self.pending = None;
@@ -313,8 +358,37 @@ impl DiffState {
} else {
self.first_text_focus()
};
self.align_file();
self.error = None;
self.prepare_selected_file();
}
Ok(Outcome::File {
comparison_id,
file_index,
file,
}) if expected
== Some(RequestKind::File {
comparison_id,
file_index,
}) =>
{
if let Some(comparison) = &mut self.comparison
&& let Some(selected) = comparison.files.get_mut(file_index)
&& selected.path == file.path
&& file.loaded
{
if file
.notice
.as_deref()
.is_some_and(|notice| notice.contains("Binary or non-UTF-8"))
{
comparison.editable_paths.remove(&file.path);
}
*selected = file;
self.align_file();
self.normalize_focus();
self.error = None;
} else {
self.error = Some("Unexpected changed-file response".into());
}
}
Ok(_) => self.error = Some("Unexpected repository response".into()),
Err(error) => self.error = Some(error),
@@ -481,6 +555,8 @@ impl DiffState {
};
if let RevisionTarget::FetchRemote(remote) = &revision.target {
self.queue(Operation::FetchRemote(remote.clone()));
} else if let RevisionTarget::ListRemote(remote) = &revision.target {
self.queue(Operation::ListRemote(remote.clone()));
} else if self.phase == Phase::Before {
self.before_position = Some(SelectorPosition {
category: self.category,
@@ -597,6 +673,9 @@ impl DiffState {
else {
return PaneLayout::Empty;
};
if !file.loaded {
return PaneLayout::Empty;
}
match file.status {
FileStatus::Added => PaneLayout::After,
FileStatus::Removed => PaneLayout::Before,
@@ -697,16 +776,20 @@ impl DiffState {
pub fn after_editable(&self) -> bool {
self.phase == Phase::View
&& self.selected_file_loaded()
&& matches!(self.pane_layout(), PaneLayout::Split | PaneLayout::After)
&& self
.after
.as_ref()
.is_some_and(|revision| revision.target == RevisionTarget::Working)
&& self.comparison.as_ref().is_some_and(|comparison| {
comparison
.files
.get(self.file)
.is_some_and(|file| comparison.editable_paths.contains(&file.path))
comparison.files.get(self.file).is_some_and(|file| {
comparison.editable_paths.contains(&file.path)
&& file
.notice
.as_deref()
.is_none_or(|notice| !notice.contains("Binary or non-UTF-8"))
})
})
}
@@ -725,10 +808,40 @@ impl DiffState {
self.live_buffer = None;
self.live_revision = None;
self.follow_caret = false;
self.align_file();
self.prepare_selected_file();
true
}
pub fn selected_file_loaded(&self) -> bool {
self.comparison
.as_ref()
.and_then(|comparison| comparison.files.get(self.file))
.is_some_and(|file| file.loaded)
}
fn prepare_selected_file(&mut self) {
self.pending = None;
self.request_kind = None;
self.loading = false;
self.error = None;
if self.selected_file_loaded() {
self.align_file();
} else {
self.align_file();
if let Some(comparison_id) = self.comparison_id
&& self
.comparison
.as_ref()
.is_some_and(|comparison| !comparison.files.is_empty())
{
self.queue(Operation::LoadFile {
comparison_id,
file_index: self.file,
});
}
}
}
pub(crate) fn source_display_row(&self, focus: Focus, source_row: usize) -> Option<usize> {
self.display_rows.iter().position(|display| {
matches!(*display, DisplayRow::Line(index) if self.source_row(focus, index) == Some(source_row))
@@ -829,6 +942,24 @@ impl DiffState {
}
let file = &self.comparison.as_ref().unwrap().files[self.file];
let was_live = self.live_files.contains(&self.file);
let changed_lines = changed_line_range(&file.after, lines);
if was_live && changed_lines.0 == changed_lines.1 && changed_lines.1 == changed_lines.2 {
self.live_revision = Some(revision);
return;
}
if was_live && stable_after_edit(file, lines, changed_lines) {
let file = &mut self.comparison.as_mut().unwrap().files[self.file];
replace_after_lines(
&mut file.after,
&mut self.after_widths,
&mut self.max_after,
lines,
changed_lines,
);
self.left_after = self.left_after.min(self.max_after);
self.live_revision = Some(revision);
return;
}
let mut old = file.after.iter().map(String::as_str).collect::<Vec<_>>();
if !was_live && (file.after_eof || old.is_empty()) {
old.push("");
@@ -870,10 +1001,13 @@ impl DiffState {
.map(map_after);
let was_changed = file_is_changed(file);
let file = &mut self.comparison.as_mut().unwrap().files[self.file];
file.after.resize_with(lines.len(), String::new);
for (target, line) in file.after.iter_mut().zip(lines) {
target.clone_from(line);
}
replace_after_lines(
&mut file.after,
&mut self.after_widths,
&mut self.max_after,
lines,
changed_lines,
);
file.after_eof = lines.len() > 1 && lines.last().is_some_and(String::is_empty);
file.hunks = line_hunks(
&file.before,
@@ -946,35 +1080,31 @@ impl DiffState {
self.left_before = 0;
self.left_after = 0;
self.max_before = 0;
self.max_after = 0;
self.after_widths.clear();
}
self.max_after = 0;
let Some(file) = self
.comparison
.as_ref()
.and_then(|comparison| comparison.files.get(self.file))
.filter(|file| file.loaded)
else {
self.after_widths.clear();
self.max_before = 0;
self.max_after = 0;
return;
};
let width = |lines: &[String]| {
lines
.iter()
.map(|line| {
line.graphemes(true).fold(0, |column, grapheme| {
column
+ if grapheme == "\t" {
4 - column % 4
} else {
UnicodeWidthStr::width(grapheme)
}
})
})
.max()
.unwrap_or(0)
};
if reset {
self.max_before = width(&file.before);
self.max_before = file
.before
.iter()
.map(|line| line_width(line))
.max()
.unwrap_or(0);
self.after_widths
.extend(file.after.iter().map(|line| line_width(line)));
self.max_after = self.after_widths.iter().copied().max().unwrap_or(0);
}
self.max_after = width(&file.after);
let after_len = if self.live_files.contains(&self.file) {
live_content(&file.after, file.after_eof).len()
} else {
@@ -1182,6 +1312,89 @@ fn append_rows(
}
}
fn line_width(line: &str) -> usize {
#[cfg(test)]
tests::WIDTH_SCANS.with(|scans| scans.set(scans.get() + 1));
line.graphemes(true).fold(0, |column, grapheme| {
column
+ if grapheme == "\t" {
4 - column % 4
} else {
UnicodeWidthStr::width(grapheme)
}
})
}
fn changed_line_range(old: &[String], new: &[String]) -> (usize, usize, usize) {
let start = old
.iter()
.zip(new)
.take_while(|(old, new)| old == new)
.count();
let suffix = old[start..]
.iter()
.rev()
.zip(new[start..].iter().rev())
.take_while(|(old, new)| old == new)
.count();
(start, old.len() - suffix, new.len() - suffix)
}
fn stable_after_edit(
file: &ChangedFile,
lines: &[String],
(start, old_end, new_end): (usize, usize, usize),
) -> bool {
if old_end != start + 1
|| new_end != start + 1
|| file.after_eof != (lines.len() > 1 && lines.last().is_some_and(String::is_empty))
|| live_content(&file.after, file.after_eof).len()
!= live_content(lines, file.after_eof).len()
{
return false;
}
let old = &file.after[start];
let new = &lines[start];
// With neither token matching another line, the equality graph seen by Myers
// is unchanged. Hunks, alignment, excerpts and scroll anchors remain exact.
!file.before.iter().any(|line| line == old || line == new)
&& !file
.after
.iter()
.enumerate()
.any(|(index, line)| index != start && (line == old || line == new))
}
fn replace_after_lines(
after: &mut Vec<String>,
widths: &mut Vec<usize>,
max_width: &mut usize,
lines: &[String],
(start, old_end, new_end): (usize, usize, usize),
) {
let removed_max = widths[start..old_end].iter().copied().max().unwrap_or(0);
if old_end == new_end {
for index in start..new_end {
if after[index] != lines[index] {
after[index].clone_from(&lines[index]);
widths[index] = line_width(&lines[index]);
}
}
} else {
after.splice(start..old_end, lines[start..new_end].iter().cloned());
widths.splice(
start..old_end,
lines[start..new_end].iter().map(|line| line_width(line)),
);
}
let added_max = widths[start..new_end].iter().copied().max().unwrap_or(0);
*max_width = if removed_max == *max_width && added_max < removed_max {
widths.iter().copied().max().unwrap_or(0)
} else {
(*max_width).max(added_max)
};
}
fn live_content(lines: &[String], eof: bool) -> &[String] {
if eof || (lines.len() == 1 && lines[0].is_empty()) {
&lines[..lines.len().saturating_sub(1)]
@@ -1220,7 +1433,8 @@ pub(crate) fn line_hunks(
}
fn file_is_changed(file: &ChangedFile) -> bool {
file.status != FileStatus::Modified
!file.loaded
|| file.status != FileStatus::Modified
|| !file.hunks.is_empty()
|| file.notice.as_ref().is_some_and(|notice| {
notice
@@ -1269,6 +1483,9 @@ fn update_eof_notice(file: &mut ChangedFile) {
mod tests {
use super::*;
use crate::editor::{Editor, Effect, Mode};
std::thread_local! {
pub(super) static WIDTH_SCANS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
fn excerpt_state(len: usize, changed: &[usize]) -> DiffState {
let mut state = DiffState::new();
@@ -1583,6 +1800,7 @@ mod tests {
path: "renamed.txt".into(),
old_path: Some("old.txt".into()),
status: FileStatus::Renamed,
loaded: true,
before: vec![
"same".into(),
"old".into(),
@@ -1619,6 +1837,7 @@ mod tests {
path: "added.txt".into(),
old_path: None,
status: FileStatus::Added,
loaded: true,
before: vec![],
after: vec!["a".into(), "b".into()],
before_eof: false,
@@ -1635,6 +1854,7 @@ mod tests {
path: "removed.bin".into(),
old_path: None,
status: FileStatus::Removed,
loaded: true,
before: vec![],
after: vec![],
before_eof: false,
@@ -2134,6 +2354,95 @@ mod tests {
assert_eq!(editor.diff().unwrap().comparison_id, Some(second));
}
#[test]
fn closing_diff_rejects_pending_file_details_even_after_reopening() {
let mut editor = editor();
open(&mut editor);
let comparison_id = choose_pair(&mut editor);
let mut catalog = comparison();
let detail = catalog.files.remove(0);
catalog.files.clear();
catalog.files.push(ChangedFile {
path: detail.path.clone(),
old_path: detail.old_path.clone(),
status: detail.status,
loaded: false,
before: vec![],
after: vec![],
before_eof: false,
after_eof: false,
hunks: vec![],
notice: None,
});
editor.apply_diff_result(comparison_id, Ok(Outcome::Comparison(catalog)));
let (file_request, _) = editor.take_diff_request().unwrap();
editor.execute("diff.close");
assert!(!editor.apply_diff_result(
file_request,
Ok(Outcome::File {
comparison_id,
file_index: 0,
file: detail,
})
));
assert!(editor.diff().is_none());
open(&mut editor);
assert!(editor.diff().unwrap().comparison.is_none());
}
#[test]
fn explicit_remote_ref_loader_preserves_selector_position_without_choosing_revision() {
let mut state = DiffState::new();
let mut initial = catalog();
let loader = initial
.entries
.iter_mut()
.find(|entry| matches!(entry.target, RevisionTarget::FetchRemote(_)))
.unwrap();
loader.target = RevisionTarget::ListRemote("origin".into());
loader.label = "Load refs from origin".into();
state.apply_result(1, Ok(Outcome::Catalog(initial)));
state.phase = Phase::After;
state.query = "origin".into();
state.refilter();
state.selected = 1;
state.choose();
assert!(
matches!(state.take_request(), Some(Operation::ListRemote(remote)) if remote == "origin")
);
let mut loaded = catalog();
let loader = loaded
.entries
.iter_mut()
.find(|entry| matches!(entry.target, RevisionTarget::FetchRemote(_)))
.unwrap();
loader.target = RevisionTarget::ListRemote("origin".into());
loader.label = "Load refs from origin".into();
loaded.entries.push(revision(
RevisionTarget::Remote {
remote: "origin".into(),
reference: "refs/heads/new".into(),
},
"new@origin",
"",
Category::Branches,
));
state.apply_result(2, Ok(Outcome::Catalog(loaded)));
assert_eq!(state.phase, Phase::After);
assert_eq!(state.query, "origin");
assert_eq!(state.category, 0);
assert_eq!(state.selected, 1);
assert!(state.after.is_none());
assert!(!state.loading);
state.category = 1;
state.refilter();
assert!(
state
.entries()
.any(|entry| matches!(entry.target, RevisionTarget::Remote { .. }))
);
}
#[test]
fn discovery_and_fetch_errors_are_retryable_and_stale_responses_cannot_reopen() {
let mut editor = editor();
@@ -2309,6 +2618,7 @@ mod tests {
before_eof: eof,
after_eof: eof,
notice: None,
loaded: true,
}],
});
state.files = Some(FilePicker::from_paths(
@@ -2319,6 +2629,147 @@ mod tests {
state
}
#[test]
fn reverting_loaded_file_keeps_unloaded_changes_selectable() {
let mut state = live_state(vec!["base".into()], vec!["changed".into()], false);
state.comparison_id = Some(17);
state.pending = None;
state.comparison.as_mut().unwrap().files.push(ChangedFile {
path: "unloaded.txt".into(),
old_path: None,
status: FileStatus::Modified,
loaded: false,
before: vec![],
after: vec![],
before_eof: false,
after_eof: false,
hunks: vec![],
notice: None,
});
state.refresh_after(&["base".into()], 1);
assert_eq!(
state
.files
.as_ref()
.unwrap()
.entries()
.map(|entry| entry.path)
.collect::<Vec<_>>(),
[Path::new("unloaded.txt")],
);
assert!(state.select_file(Path::new("unloaded.txt")));
assert!(matches!(
state.take_request(),
Some(Operation::LoadFile {
comparison_id: 17,
file_index: 1
})
));
}
#[test]
fn live_width_cache_scans_only_changed_lines_and_shrinks_after_deletion() {
let base = vec!["original".into(), "medium".into(), "short".into()];
let mut lines = vec!["界\tz".into(), "medium".into(), "short".into()];
let mut state = live_state(base, lines.clone(), false);
state.refresh_after(&lines, 1);
assert_eq!(state.after_widths, [5, 6, 5]);
lines[0] = "界界\t🙂".into();
WIDTH_SCANS.set(0);
state.refresh_after(&lines, 2);
assert_eq!(WIDTH_SCANS.get(), 1);
assert_eq!(state.max_after, 10);
state.left_after = 9;
lines[0] = "界".into();
WIDTH_SCANS.set(0);
state.refresh_after(&lines, 3);
assert_eq!(WIDTH_SCANS.get(), 1);
assert_eq!(state.max_after, 6);
assert_eq!(state.left_after, 6);
lines.remove(1);
WIDTH_SCANS.set(0);
state.refresh_after(&lines, 4);
assert_eq!(WIDTH_SCANS.get(), 0);
assert_eq!(state.after_widths, [2, 5]);
assert_eq!(state.max_after, 5);
lines.insert(1, "巨大\t🙂".into());
WIDTH_SCANS.set(0);
state.refresh_after(&lines, 5);
assert_eq!(WIDTH_SCANS.get(), 1);
assert_eq!(state.after_widths, [2, 10, 5]);
lines = vec!["".into()];
state.refresh_after(&lines, 6);
assert_eq!(state.after_widths, [0]);
assert_eq!(state.max_after, 0);
assert_eq!(state.left_after, 0);
}
#[test]
fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() {
let base = (0..100)
.map(|row| format!("line {row}"))
.collect::<Vec<_>>();
let mut lines = base.clone();
lines[30] = "first change".into();
let mut state = live_state(base.clone(), lines.clone(), true);
state.set_context_options(2, 5);
lines.push(String::new());
state.refresh_after(&lines, 1);
state.top = state.after_display_row(30).unwrap();
state.handle(Input::Char('['));
let display = state.display_rows.clone();
let excerpts = state.excerpt_ranges.clone();
let top = state.top;
lines[30].push('!');
let file = &state.comparison.as_ref().unwrap().files[0];
assert!(stable_after_edit(
file,
&lines,
changed_line_range(&file.after, &lines)
));
WIDTH_SCANS.set(0);
state.refresh_after(&lines, 2);
assert_eq!(WIDTH_SCANS.get(), 1);
assert_eq!(state.display_rows, display);
assert_eq!(state.excerpt_ranges, excerpts);
assert_eq!(state.top, top);
let file = &state.comparison.as_ref().unwrap().files[0];
let hunk_ranges = |hunks: &[Hunk]| {
hunks
.iter()
.map(|hunk| {
(
hunk.before_start,
hunk.before_len,
hunk.after_start,
hunk.after_len,
)
})
.collect::<Vec<_>>()
};
assert_eq!(
hunk_ranges(&file.hunks),
hunk_ranges(&line_hunks(&base, true, live_content(&lines, true), true))
);
lines[30].clone_from(&base[30]);
assert!(!stable_after_edit(
file,
&lines,
changed_line_range(&file.after, &lines)
));
state.refresh_after(&lines, 3);
assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty());
assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal));
assert!(state.after_display_row(23).is_some());
assert_eq!(state.top, state.after_display_row(30).unwrap());
assert_eq!(state.files.as_ref().unwrap().entries().len(), 0);
lines[50] = "changed again".into();
state.refresh_after(&lines, 4);
assert_eq!(state.files.as_ref().unwrap().entries().len(), 1);
assert!(state.after_display_row(23).is_some());
assert!(state.after_display_row(50).is_some());
}
#[test]
fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() {
let base = (0..100)
@@ -2398,4 +2849,102 @@ mod tests {
empty.handle(Input::Escape);
assert_eq!(empty.phase, Phase::View);
}
#[test]
fn lazy_selection_waits_for_details_rejects_stale_files_and_reuses_loaded_cache() {
let mut state = DiffState::new();
state.pending = None;
state.request_kind = Some(RequestKind::Comparison);
let mut catalog = comparison();
catalog.files.truncate(2);
let details: Vec<_> = catalog.files.drain(..).collect();
for detail in &details {
catalog.files.push(ChangedFile {
path: detail.path.clone(),
old_path: detail.old_path.clone(),
status: detail.status,
loaded: false,
before: vec![],
after: vec![],
before_eof: false,
after_eof: false,
hunks: vec![],
notice: None,
});
}
state.apply_result(41, Ok(Outcome::Comparison(catalog)));
assert!(!state.selected_file_loaded());
assert!(state.rows.is_empty() && state.display_rows.is_empty());
assert_eq!(state.pane_layout(), PaneLayout::Empty);
assert!(!state.after_editable());
assert!(matches!(
state.take_request(),
Some(Operation::LoadFile {
comparison_id: 41,
file_index: 0,
})
));
assert!(state.select_file(Path::new("added.txt")));
assert!(matches!(
state.take_request(),
Some(Operation::LoadFile {
comparison_id: 41,
file_index: 1,
})
));
let mut details = details.into_iter();
let first = details.next().unwrap();
let second = details.next().unwrap();
state.apply_result(
42,
Ok(Outcome::File {
comparison_id: 40,
file_index: 1,
file: first,
}),
);
assert!(state.loading && !state.selected_file_loaded());
state.apply_result(
43,
Ok(Outcome::File {
comparison_id: 41,
file_index: 1,
file: second,
}),
);
assert!(state.selected_file_loaded());
assert_eq!(state.pane_layout(), PaneLayout::After);
assert!(!state.rows.is_empty());
assert!(state.select_file(Path::new("renamed.txt")));
assert!(state.loading);
assert!(state.select_file(Path::new("added.txt")));
assert!(!state.loading && state.selected_file_loaded());
assert!(state.take_request().is_none());
state.apply_result(44, Err("cancelled old load".into()));
assert!(state.error.is_none() && state.selected_file_loaded());
}
#[test]
fn failed_lazy_file_load_can_be_retried_without_reopening_comparison() {
let mut state = DiffState::new();
state.pending = None;
state.request_kind = Some(RequestKind::Comparison);
let mut catalog = comparison();
catalog.files.truncate(1);
catalog.files[0].loaded = false;
state.apply_result(7, Ok(Outcome::Comparison(catalog)));
state.take_request();
state.apply_result(8, Err("cannot read blob".into()));
assert_eq!(state.error.as_deref(), Some("cannot read blob"));
assert!(!state.selected_file_loaded());
assert!(state.select_file(Path::new("renamed.txt")));
assert!(state.loading && state.error.is_none());
assert!(matches!(
state.take_request(),
Some(Operation::LoadFile {
comparison_id: 7,
file_index: 0,
})
));
}
}