From d67b99a3dd611f0c239d9bce13552417acaba622 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Sun, 2 Aug 2026 01:42:44 -0400 Subject: [PATCH] refactor(terminal): delete the slice-sizing function nothing calls `slice_bytes(columns, lines, scrollback)` stopped being the scheduler's function when slices became remaining-aware, and the fixtures went on asserting against it. The two disagreed exactly where the old floor bound -- reporting 4 bytes where `drain` used 1 -- so the preconditions guarding the decrease arm were describing behaviour that no longer existed. Not wrong at today's geometries, and no test would have noticed when it became wrong. It had zero callers outside the tests it misled. Deleted, with every assertion moved to `slice_bytes_remaining`, which is what the engine calls. One function, one answer. `MIN_SLICE` goes with it: the floor is 1 and lives in the function, because on a grid whose worst atom exceeds the whole budget no wider slice can promise to stop after the callback that crosses. A named constant that only appeared in a doc comment is a second authority waiting to disagree with the first. 69 tests, debug and release. Test-only in effect; the deleted item was unreachable from the engine. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../crates/buzz-terminal/src/fences.rs | 60 ++++++------------- .../crates/buzz-terminal/tests/slicing.rs | 42 +++++++------ 2 files changed, 42 insertions(+), 60 deletions(-) diff --git a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs index 7ee9f30e2..ab5d3234d 100644 --- a/desktop/src-tauri/crates/buzz-terminal/src/fences.rs +++ b/desktop/src-tauri/crates/buzz-terminal/src/fences.rs @@ -34,53 +34,29 @@ pub const OSC_BUDGET: usize = 256 << 10; /// spent to the last unit. pub const WORK_BUDGET: u64 = 250_000; -/// Bounds on how many bytes are handed to the parser at once. +/// Widest slice handed to the parser at once. /// -/// The budget is checked *between* slices, so the slice is what actually -/// bounds one lock hold, and a fixed byte count cannot do it: `ESC#8` is -/// three bytes and costs a full grid, so 256 bytes of it is 1.6 ms at 200x50 -/// and 14 ms at 1600x50. [`slice_bytes`] therefore derives the size from the -/// grid, and these two clamp it -- `MAX` at the throughput plateau (measured: -/// plain-char parsing saturates by 64 bytes and is flat to 64 KiB), `MIN` -/// where a slice stops being able to hold a whole escape sequence: 4 bytes -/// covers `ESC#8` and `ESC c` intact, so the floor never splits the densest -/// atoms across slices for no benefit. Measured throughput at the floor is -/// ~74% of the plateau on plain text and ~77% on SGR, which is the price of -/// bounding a grid whose worst atom exceeds the budget outright. -pub const MIN_SLICE: usize = 4; +/// The floor is 1 byte and lives in [`slice_bytes_remaining`] rather than +/// here: on a grid whose worst atom exceeds the whole budget -- RIS at any +/// real scrollback depth -- no wider slice can promise to stop after the +/// callback that crosses. This cap is the other end, set at the throughput +/// plateau: plain-char parsing saturates by 64 bytes and is flat to 64 KiB +/// measured, so nothing above it buys anything and a larger value only +/// coarsens the cut. pub const MAX_SLICE: usize = 256; -/// Bytes to hand the parser at once on a `columns x lines` grid. +/// Bytes to hand the parser next. /// -/// Sized against the **densest atom the grid admits**, so the bound holds on -/// the first byte of a cold feeder for any payload: two bytes of `ESC c` buy -/// [`max_atom_work`], which is the most work per byte upstream offers, and -/// `budget / (atom / 2)` is the widest slice that cannot exceed one budget. +/// The **only** slice-sizing function, deliberately: an earlier version of +/// this module also exported a `slice_bytes(columns, lines, scrollback)` that +/// the scheduler stopped calling when slices became remaining-aware, and the +/// fixtures went on asserting against it. The two disagreed exactly where the +/// floor bound -- reporting 4 where the engine used 1 -- so the preconditions +/// were describing a function no longer in the path. One function, one +/// answer, and every test asserts on what `drain` actually calls. /// -/// Deliberately *derived rather than learned*. An earlier version sized -/// slices from the density of preceding slices, which is strictly worse where -/// it matters: a fresh feeder has observed nothing, so its first slice is -/// wide, and a first wide slice of RIS spends many budgets before anything -/// looks. A bound that has to be taught is not a bound on the lesson. -/// -/// [`MIN_SLICE`] floors it, and on any grid with real scrollback the floor is -/// what binds -- RIS at the default 10k depth is worth more than the entire -/// budget on its own, so no slice size can keep a drain inside the budget and -/// the floor stops the arithmetic from asking for fractions of a byte. That -/// residual is not hidden: it is exactly [`max_drain_work`], and it is the -/// honest cost of an indivisible callback that upstream can be asked to make -/// smaller only by not calling it. -pub fn slice_bytes(columns: usize, lines: usize, scrollback: usize) -> usize { - let densest = (max_atom_work(columns, lines, scrollback) / 2).max(1); - ((WORK_BUDGET / densest) as usize).clamp(MIN_SLICE, MAX_SLICE) -} - -/// Bytes to hand the parser when `spent` of the budget is already gone. -/// -/// The scheduling rule in one place so the fixtures can assert on it rather -/// than on a copy of the arithmetic: a slice of `N` bytes holds at most -/// `N / atom_bytes` atoms, so `remaining / densest` bytes cannot carry a -/// drain past the budget. +/// The rule: a slice of `N` bytes holds at most `N / atom_bytes` atoms, so +/// `remaining / densest` bytes cannot carry a drain past the budget. /// /// `next_escape` is how far the next `ESC` is from the front of the tail. /// This is the difference between a correct bound and an unusable one. Only diff --git a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs index 32da2ecdc..f4b87a0d8 100644 --- a/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs +++ b/desktop/src-tauri/crates/buzz-terminal/tests/slicing.rs @@ -10,8 +10,8 @@ //! `> 0` is satisfied by a seam that executed exactly one unit. use buzz_terminal::fences::{ - max_atom_work, max_drain_work, slice_bytes, slice_bytes_remaining, Fences, MAX_SLICE, - MIN_SLICE, SYNC_CAP, TAIL_CAP, WORK_BUDGET, + max_atom_work, max_drain_work, slice_bytes_remaining, Fences, MAX_SLICE, SYNC_CAP, TAIL_CAP, + WORK_BUDGET, }; use buzz_terminal::{Size, Terminal}; @@ -339,26 +339,26 @@ fn a_resize_mid_tail_reprices_the_remainder() { /// serves both. #[test] fn slice_size_shrinks_as_the_worst_atom_grows() { - let small = slice_bytes(80, 24, 0); - let large = slice_bytes(1600, 50, 0); + let small = slice_bytes_remaining(80, 24, 0, 0, 0); + let large = slice_bytes_remaining(1600, 50, 0, 0, 0); assert!( small > large, "a bigger grid makes each byte more expensive, so slices must shrink: \ 80x24 -> {small}, 1600x50 -> {large}", ); assert!( - slice_bytes(200, 50, 10_000) <= slice_bytes(200, 50, 0), + slice_bytes_remaining(200, 50, 10_000, 0, 0) <= slice_bytes_remaining(200, 50, 0, 0, 0), "scrollback makes RIS more expensive, so it may only shrink slices", ); for (columns, lines, scrollback) in [(80, 24, 0), (200, 50, 0), (400, 100, 0), (1600, 50, 0)] { - assert!((MIN_SLICE..=MAX_SLICE).contains(&slice_bytes(columns, lines, scrollback))); + assert!((1..=MAX_SLICE).contains(&slice_bytes_remaining(columns, lines, scrollback, 0, 0))); // One slice holds at most N/2 of the densest atom. Either that fits a // budget, or the floor binds -- and then the overshoot is stated by // `max_drain_work` rather than being an accident. - let worst = (slice_bytes(columns, lines, scrollback) as u64 / 2) - * max_atom_work(columns, lines, scrollback); + let width = slice_bytes_remaining(columns, lines, scrollback, 0, 0); + let worst = (width as u64 / 2) * max_atom_work(columns, lines, scrollback); assert!( - worst <= WORK_BUDGET || slice_bytes(columns, lines, scrollback) == MIN_SLICE, + worst <= WORK_BUDGET || width == 1, "{columns}x{lines}: a slice buys {worst} work against a \ {WORK_BUDGET} budget without the MIN clamp to excuse it", ); @@ -923,12 +923,18 @@ fn a_scrollback_change_reprices_the_densest_atom_and_the_slicing() { // depth, so a conforming repair would show work identical to the // control and the assertions here would invert into false failures. assert!( - slice_bytes(shallow.columns, shallow.screen_lines, shallow.scrollback) > MIN_SLICE, + slice_bytes_remaining( + shallow.columns, + shallow.screen_lines, + shallow.scrollback, + 0, + 0 + ) > 1, "geometry cannot discriminate: the shallow arm is already floored", ); assert_eq!( - slice_bytes(deep.columns, deep.screen_lines, deep.scrollback), - MIN_SLICE, + slice_bytes_remaining(deep.columns, deep.screen_lines, deep.scrollback, 0, 0), + 1, ); // How a terminal at `size` retires 200 RIS: work, and how many @@ -1151,8 +1157,8 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { // most expensive. Wrapping inverts the fence. So: the widest possible // atom must give the narrowest possible slice. assert_eq!( - slice_bytes(huge, huge, huge), - MIN_SLICE, + slice_bytes_remaining(huge, huge, huge, 0, 0), + 1, "an overflowing grid must clamp to the smallest slice; a wrapped \ `max_atom_work` would hand back a generous one", ); @@ -1173,10 +1179,10 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { for (axis, at) in [ ( "scrollback", - (|n| slice_bytes(200, 50, n)) as fn(usize) -> usize, + (|n| slice_bytes_remaining(200, 50, n, 0, 0)) as fn(usize) -> usize, ), - ("columns", |n| slice_bytes(n.max(1), 50, 0)), - ("lines", |n| slice_bytes(200, n.max(1), 0)), + ("columns", |n| slice_bytes_remaining(n.max(1), 50, 0, 0, 0)), + ("lines", |n| slice_bytes_remaining(200, n.max(1), 0, 0, 0)), ] { let mut previous = usize::MAX; for exponent in 0..60 { @@ -1186,7 +1192,7 @@ fn extreme_dimensions_saturate_instead_of_wrapping() { "slice widened from {previous} to {width} at {axis} \ 2^{exponent}: more expensive grid, more generous slice", ); - assert!(width >= MIN_SLICE); + assert!(width >= 1); previous = width; } }