fix(diff-overlay): keep the row key pointing at the snapshot that landed

`sync_diff_rows` wrote `rows_key` only when it rebuilt the rows. A probe that
found nothing new lands a fresh `Arc` over an equal snapshot, so `describes`
answered yes the long way — by walking every line of the patch — and then left
the key holding the *old* `Arc`. Every frame after that paid the same walk:
once per wheel event, over the ~1 MB of `String` at the 20k-line budget, which
is the cost the key exists to avoid. Retarget it on a match, so the pointer
comparison settles the next frame.

The preview was doubly exposed: `install_diff_snapshot` clears it on every
landing, so the re-read produces a fresh `Arc<FileDiff>` too. Its comparison
also ran eagerly, ahead of the scalars that could already have answered no;
move it into the `&&` chain.

`stack_corners` lost its last non-test caller when the file cards went, and
`cargo build` had started reporting it. Drop it and the test that held it up.
This commit is contained in:
l0ng-ai
2026-09-07 20:59:46 +08:00
parent ecf3ff3577
commit b5c20932cb
2 changed files with 82 additions and 43 deletions
+82 -9
View File
@@ -844,11 +844,11 @@ impl Tty7App {
oversized: focused.is_none() && snap.stats().oversized,
expanded: &overlay.expanded,
};
if overlay
let stale = overlay
.rows_key
.as_ref()
.is_none_or(|held| !held.describes(&from))
{
.is_none_or(|held| !held.describes(&from));
if stale {
let rows = match from.preview {
Some(file) => crate::ui::diff_list::preview_rows(file, from.mode),
None => crate::ui::diff_list::build_rows(
@@ -863,6 +863,8 @@ impl Tty7App {
resync_list(&overlay.list, &overlay.rows, &rows);
overlay.rows = Rc::new(rows);
overlay.rows_key = Some(key);
} else if let Some(held) = overlay.rows_key.as_mut() {
held.retarget(&from);
}
Some(DiffBody::Rows(snap))
}
@@ -978,19 +980,40 @@ impl RowsKey {
/// probe that found nothing new still lands a fresh `Arc` over an equal
/// snapshot, and rebuilding every row of the patch for that would undo the
/// point of keeping them.
///
/// The scalars go first so that the walk of the patch behind that second
/// comparison is only ever paid to answer a question the cheap fields
/// have not already answered.
fn describes(&self, from: &RowsFrom<'_>) -> bool {
let same_preview = match (&self.preview, from.preview) {
(None, None) => true,
(Some(a), Some(b)) => Arc::ptr_eq(a, b) || a == b,
_ => false,
};
self.mode == from.mode
&& self.focused == from.focused
&& self.oversized == from.oversized
&& self.expanded == *from.expanded
&& same_preview
&& self.same_preview(from)
&& (Arc::ptr_eq(&self.snap, from.snap) || self.snap == *from.snap)
}
/// The preview, by pointer and then by contents — a re-read of an
/// untracked file lands a fresh `Arc` over bytes that did not change.
fn same_preview(&self, from: &RowsFrom<'_>) -> bool {
match (&self.preview, from.preview) {
(None, None) => true,
(Some(a), Some(b)) => Arc::ptr_eq(a, b) || a == b,
_ => false,
}
}
/// Points the key at the `Arc`s this frame was asked about, having just
/// found them equal to the ones held.
///
/// Without this the key goes on holding the snapshot from the last
/// *rebuild*, so every frame after a probe that found nothing new proves
/// the two equal the long way — a walk of every line of the patch, once
/// per wheel event, which is the cost this key exists to avoid.
fn retarget(&mut self, from: &RowsFrom<'_>) {
self.snap = Arc::clone(from.snap);
self.preview = from.preview.cloned();
}
}
/// Tells the list which rows changed, rather than that all of them did.
@@ -2115,6 +2138,56 @@ mod tests {
assert_eq!((replaced.start, replaced.end, with), (0, rows.len(), 0));
}
/// A probe that found nothing new still lands a fresh `Arc` over an equal
/// snapshot. The rows are rightly kept — and the key has to come away
/// holding the `Arc` that landed, or every frame from then on proves the
/// two equal the long way: a walk of every line of the patch, per wheel
/// event.
#[test]
fn an_equal_snapshot_leaves_the_key_pointing_at_the_one_that_landed() {
let held = Arc::new(DiffSnapshot {
files: vec![small_file("a.rs", 2)],
..Default::default()
});
let landed = Arc::new(DiffSnapshot {
files: vec![small_file("a.rs", 2)],
..Default::default()
});
assert!(
!Arc::ptr_eq(&held, &landed),
"two separate Arcs over equal contents"
);
let expanded = HashMap::new();
let mut key = RowsFrom {
snap: &held,
preview: None,
mode: DiffViewMode::Unified,
focused: None,
oversized: false,
expanded: &expanded,
}
.to_key();
let landed_from = RowsFrom {
snap: &landed,
preview: None,
mode: DiffViewMode::Unified,
focused: None,
oversized: false,
expanded: &expanded,
};
assert!(
key.describes(&landed_from),
"nothing about the rows changed"
);
key.retarget(&landed_from);
assert!(
Arc::ptr_eq(&key.snap, &landed),
"so the next frame settles it by pointer rather than by contents"
);
}
#[test]
fn a_focused_untracked_file_asks_for_a_preview_not_the_list() {
let snap = DiffSnapshot {
-34
View File
@@ -40,24 +40,6 @@ pub(crate) fn segment_corners(
}
}
pub(crate) fn stack_corners(
i: usize,
count: usize,
outer: Pixels,
border: Pixels,
) -> Corners<Pixels> {
let r = inner_radius(outer, border);
let zero = px(0.);
let first = i < count && i == 0;
let last = i < count && i + 1 == count;
Corners {
top_left: if first { r } else { zero },
top_right: if first { r } else { zero },
bottom_left: if last { r } else { zero },
bottom_right: if last { r } else { zero },
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -109,20 +91,4 @@ mod tests {
Corners::all(px(0.))
);
}
#[test]
fn a_stack_caps_its_first_and_last_band() {
let r = inner_radius(CARD_RADIUS, HAIRLINE);
let zero = px(0.);
let top = stack_corners(0, 2, CARD_RADIUS, HAIRLINE);
assert_eq!((top.top_left, top.top_right), (r, r));
assert_eq!((top.bottom_left, top.bottom_right), (zero, zero));
let bottom = stack_corners(1, 2, CARD_RADIUS, HAIRLINE);
assert_eq!((bottom.bottom_left, bottom.bottom_right), (r, r));
assert_eq!((bottom.top_left, bottom.top_right), (zero, zero));
assert_eq!(stack_corners(0, 1, CARD_RADIUS, HAIRLINE), Corners::all(r));
}
}