test: guard pane-scaled render performance

refs #2550
This commit is contained in:
Ogulcan Celik 2026-08-09 20:36:11 +03:00
parent 4e31084495
commit e2aa86a9e8
10 changed files with 433 additions and 6 deletions

View File

@ -14,7 +14,8 @@ Read `references/pre-release-audit.md` and follow its workflow. Treat it as the
- auditing `docs/next/CHANGELOG.md`
- auditing `docs/next/README.md` and staged website docs
- checking issue reference lines
- deciding when to run `just release-docs-check`
- deciding when to run `just pre-release-check` or its component checks
- running and assessing `just bench-render-scale`
- producing the final release-readiness report
Do not edit files during the audit unless the user explicitly asks to apply fixes. When applying fixes, keep changes scoped to the files named in the reference workflow.

View File

@ -75,10 +75,11 @@ Process:
- `nix/package.nix` imports `Cargo.lock` through `cargoLock.lockFile`; normal version and lockfile updates do not require a separate cargo hash refresh. If git dependencies are introduced, verify the required `cargoLock.outputHashes` entries.
- Run or recommend:
```bash
just release-docs-check
just pre-release-check
```
- This check validates the staged draft, localized heading parity, published preview and stable snapshot provenance, and both production and draft website builds.
- Do not run `just release` unless the working tree is clean and the docs check passes.
- The docs check validates the staged draft, localized heading parity, published preview and stable snapshot provenance, and both production and draft website builds.
- The render benchmark has no automatic timing threshold, but reviewing it is a required release checkpoint. Record the 1, 15, and 50-count median and p95 results for background-workspace resize/layout and active panes, compare their scaling ratios, and treat a material regression as a release blocker until investigated rather than relying on absolute timing across machines.
- Do not run `just release` unless the working tree is clean, the docs check passes, and the render-scale result has been reviewed.
9. Apply changes only when asked.
- Do not edit files during the audit unless the user explicitly asks you to apply fixes.
@ -118,6 +119,9 @@ Root docs finalized: YES | NO
Nix Cargo lock integration: OK | NEEDS ATTENTION | NOT CHECKED
<result of nix flake check or any required cargoLock.outputHashes status>
Render scaling: OK | NEEDS ATTENTION | NOT CHECKED
<1, 15, and 50-count median/p95 results and ratios for background-workspace resize/layout and active panes>
Required before release:
1. <short action>
```

View File

@ -35,6 +35,31 @@ These instructions are layered.
- **Screen detection is evidence-based.** When changing `src/detect/manifests/`, first capture the relevant bottom-buffer state with `herdr agent read <pane> --source detection --format text` and, when styling or alternate screen behavior matters, `--format ansi`. Decide which visible controls are invariant, which are alternatives, and encode them as explicit AND/OR gates. Do not match whole-pane incidental text, and do not use the user-visible viewport for agent status because users can scroll it.
- **UI patterns should be reused.** Herdr is a mouse-first TUI. New dialogs, onboarding, settings, and post-update flows should follow the existing UI/UX language and interaction patterns instead of inventing one-off screens. Prefer reusing existing modal/screen structure, affordances, and close actions so the app feels consistent.
### Multiplicative performance paths
Treat work reachable from view computation, rendering, background-pane resizing,
PTY parsing, detection, and client frame fanout as multiplicative. Before adding
work, identify its frequency and cardinality: per byte, event, or render × panes,
tabs, or workspaces × attached clients.
Inside pane-scaled render and layout loops:
- Use narrow terminal-state accessors. Do not collect aggregate input state,
format terminal snapshots, inspect process trees, perform filesystem I/O, or
allocate when one scalar fact is enough.
- Keep terminal-core lock duration minimal.
- Preserve hidden-source and retained-render early exits. Hidden panes still
parse output, but their output must not trigger presentation work merely to
keep terminal or detection state current.
- When a change adds or widens work in one of these loops, profile fixed geometry
with 1 and at least 15 populated panes and report the scaling delta. Use
`just bench-render-scale` to exercise both background-workspace and active-pane
cardinality when applicable.
Prefer deterministic operation or architecture tests to wall-clock CI limits.
Performance benchmarks are supporting evidence, not substitutes for behavioral
coverage.
### Runtime/client boundary guardrail
Herdr is migrating toward a server-owned runtime protocol with the TUI as one client. New work should not deepen the current server/TUI coupling.
@ -227,7 +252,7 @@ just check
just release 0.x.y
```
Before stable release, run `/pre-release-audit`, finalize `docs/next`, and let `just release-docs-check` validate the staged docs and website build. `just release` prepares the changelog and release commit, tags it, and pushes the tag. GitHub Actions builds binaries, creates the GitHub release, closes released issues, snapshots and promotes the tagged docs, and updates `website/latest.json`.
Before stable release, run `/pre-release-audit`, finalize `docs/next`, and run `just pre-release-check` to validate the staged docs, website build, and render scaling. `just release` prepares the changelog and release commit, tags it, and pushes the tag. GitHub Actions builds binaries, creates the GitHub release, closes released issues, snapshots and promotes the tagged docs, and updates `website/latest.json`.
The release workflows must publish these four assets:

View File

@ -4,6 +4,7 @@
test:
cargo nextest run --locked --status-level fail --final-status-level fail --failure-output final --success-output never
python3 -m unittest scripts.test_agent_detection_manifest_check scripts.test_changelog scripts.test_config_reference_check scripts.test_docs_translation_parity scripts.test_hermes_integration_asset scripts.test_package_windows_conpty scripts.test_preview scripts.test_unix_installer scripts.test_vendor_libghostty_vt scripts.test_vendor_portable_pty
just ui-hot-path-architecture-test
just integration-assets-test
just plugin-marketplace-test
@ -11,6 +12,10 @@ test:
test-one filter:
cargo nextest run --locked "{{filter}}" --status-level fail --final-status-level fail --failure-output final --success-output never
# Enforce deterministic UI hot-path architecture boundaries
ui-hot-path-architecture-test:
python3 -m unittest scripts.test_ui_hot_path_architecture
# Run fast local lint checks
[unix]
lint:
@ -26,6 +31,7 @@ lint:
[unix]
ci filter='all()': lint
cargo nextest run --locked -E "{{filter}}" --status-level fail --final-status-level slow --failure-output final --success-output never
just ui-hot-path-architecture-test
just integration-assets-test
just plugin-marketplace-test
@ -57,6 +63,10 @@ install-hooks:
build:
cargo build --release --locked
# Non-gating full-render scaling profile for background workspaces and active panes
bench-render-scale:
cargo test --release --locked --bin herdr render_scale_profile -- --ignored --nocapture --test-threads=1
# Build the website and documentation
website-build:
cd website && bun install --frozen-lockfile && bun run build
@ -113,6 +123,12 @@ release-docs-check:
just website-build
cd website && bun run build:draft
# Validate release docs and review full-render scaling before release preparation
pre-release-check:
just release-docs-check
just bench-render-scale
@echo "release review required: investigate material render-scaling regressions before publishing."
# Prepare the release commit without tagging or pushing (usage: just release-prepare 0.1.1)
release-prepare version:
@printf '%s\n' '{{version}}' | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+$' || { \
@ -128,7 +144,7 @@ release-prepare version:
echo "error: tag v{{version}} already exists"; \
exit 1; \
fi
just release-docs-check
just pre-release-check
python3 scripts/changelog.py prepare --version {{version}}
cp CHANGELOG.md docs/next/CHANGELOG.md
sed -i.bak 's/^version = ".*"/version = "{{version}}"/' Cargo.toml && rm -f Cargo.toml.bak

View File

@ -0,0 +1,184 @@
from __future__ import annotations
import re
import unittest
from pathlib import Path
PROJECT_ROOT = Path(__file__).resolve().parent.parent
HOT_PATH_SOURCES = (
PROJECT_ROOT / "src" / "ui.rs",
*sorted((PROJECT_ROOT / "src" / "ui").rglob("*.rs")),
PROJECT_ROOT / "src" / "server" / "render_stream.rs",
)
TEST_MODULE = re.compile(r"(?m)^#\[cfg\(test\)\]\s*\nmod\s+\w+\s*\{")
FORBIDDEN_CALLS = (
(
re.compile(r"(?:\.|::)input_state\b"),
"aggregate terminal input state; add a narrow accessor",
),
(
re.compile(r"(?:\.|::)(?:keyboard_state_ansi|kitty_keyboard_state_ansi)\b"),
"formatted keyboard state",
),
(
re.compile(r"(?:\.|::)screen_text_snapshot\b"),
"formatted terminal screen snapshot",
),
(
re.compile(r"\bforeground_job\s*\("),
"process-tree inspection",
),
)
def blank_non_newlines(chars: list[str], start: int, end: int) -> None:
for index in range(start, end):
if chars[index] != "\n":
chars[index] = " "
def mask_comments_and_literals(source: str) -> str:
chars = list(source)
index = 0
while index < len(source):
if source.startswith("//", index):
end = source.find("\n", index + 2)
end = len(source) if end == -1 else end
blank_non_newlines(chars, index, end)
index = end
continue
if source.startswith("/*", index):
depth = 1
end = index + 2
while end < len(source) and depth > 0:
if source.startswith("/*", end):
depth += 1
end += 2
elif source.startswith("*/", end):
depth -= 1
end += 2
else:
end += 1
blank_non_newlines(chars, index, end)
index = end
continue
if source[index] == "r":
quote = index + 1
while quote < len(source) and source[quote] == "#":
quote += 1
if quote < len(source) and source[quote] == '"':
suffix = '"' + "#" * (quote - index - 1)
end = source.find(suffix, quote + 1)
end = len(source) if end == -1 else end + len(suffix)
blank_non_newlines(chars, index, end)
index = end
continue
if source[index] == '"':
end = index + 1
while end < len(source):
if source[end] == "\\":
end += 2
elif source[end] == '"':
end += 1
break
else:
end += 1
blank_non_newlines(chars, index, min(end, len(source)))
index = end
continue
if source[index] == "'":
end = index + 2
if index + 1 < len(source) and source[index + 1] == "\\":
end += 1
if end < len(source) and source[end] == "'":
end += 1
blank_non_newlines(chars, index, end)
index = end
continue
index += 1
return "".join(chars)
def production_code(source: str) -> str:
code = mask_comments_and_literals(source)
chars = list(code)
search_from = 0
while test_module := TEST_MODULE.search(code, search_from):
depth = 0
end = test_module.end() - 1
while end < len(code):
if code[end] == "{":
depth += 1
elif code[end] == "}":
depth -= 1
if depth == 0:
end += 1
break
end += 1
blank_non_newlines(chars, test_module.start(), end)
code = "".join(chars)
search_from = end
return code
class UiHotPathArchitectureTests(unittest.TestCase):
def test_render_hot_paths_avoid_known_expensive_runtime_queries(self) -> None:
violations: list[str] = []
for path in HOT_PATH_SOURCES:
source = path.read_text(encoding="utf-8")
code = production_code(source)
for pattern, description in FORBIDDEN_CALLS:
for match in pattern.finditer(code):
line = code.count("\n", 0, match.start()) + 1
relative_path = path.relative_to(PROJECT_ROOT)
violations.append(f"{relative_path}:{line}: {description}")
self.assertEqual(
violations,
[],
"Render/layout code must not perform pane-scaled expensive reads:\n"
+ "\n".join(violations),
)
def test_scanner_ignores_non_production_references(self) -> None:
source = '''
// runtime.input_state()
const EXAMPLE: &str = "runtime.input_state()";
#[cfg(test)]
mod tests {
fn aggregate_state_test() { runtime.input_state(); }
}
fn production_after_tests() {}
'''
code = production_code(source)
self.assertNotRegex(code, FORBIDDEN_CALLS[0][0])
self.assertIn("fn production_after_tests()", code)
self.assertEqual(code.count("\n"), source.count("\n"))
def test_scanner_checks_production_after_test_modules(self) -> None:
source = '''
#[cfg(test)]
mod tests {
const BRACES: &str = "}}";
}
fn render() { TerminalRuntime::input_state; }
'''
self.assertRegex(production_code(source), FORBIDDEN_CALLS[0][0])
def test_scanner_catches_imported_process_query(self) -> None:
source = "fn render() { foreground_job(pid); }"
self.assertRegex(production_code(source), FORBIDDEN_CALLS[3][0])
if __name__ == "__main__":
unittest.main()

View File

@ -54,6 +54,21 @@ const RELEASE_REACQUIRE_SUPPRESSION: std::time::Duration = std::time::Duration::
const PANE_TERM: &str = "xterm-256color";
const PANE_COLORTERM: &str = "truecolor";
#[cfg(test)]
thread_local! {
static AGGREGATE_INPUT_STATE_READS: Cell<usize> = const { Cell::new(0) };
}
#[cfg(test)]
pub(crate) fn reset_aggregate_input_state_reads() {
AGGREGATE_INPUT_STATE_READS.set(0);
}
#[cfg(test)]
pub(crate) fn aggregate_input_state_reads() -> usize {
AGGREGATE_INPUT_STATE_READS.get()
}
fn apply_pane_terminal_env(cmd: &mut CommandBuilder) {
// Each pane is rendered by herdr's own terminal layer, not the outer terminal
// that launched the app. Advertising the inherited TERM leaks the host terminal
@ -2593,6 +2608,8 @@ impl PaneRuntime {
}
pub fn input_state(&self) -> Option<InputState> {
#[cfg(test)]
AGGREGATE_INPUT_STATE_READS.set(AGGREGATE_INPUT_STATE_READS.get() + 1);
self.terminal.input_state()
}

View File

@ -1675,6 +1675,8 @@ impl GhosttyPaneTerminal {
})
}
// This aggregate snapshot performs multiple terminal queries and may format
// keyboard state. Pane-scaled callers should add a narrow accessor instead.
pub fn input_state(&self) -> Option<InputState> {
let Ok(core) = self.core.lock() else {
return None;

View File

@ -467,3 +467,173 @@ fn focused_terminal_suppresses_host_cursor(
.runtime_for_pane_in_workspace(terminal_runtimes, ws_idx, info.id)
.is_some_and(crate::terminal::TerminalRuntime::synchronized_output_active)
}
#[cfg(test)]
mod render_scale_benchmark {
use std::hint::black_box;
use std::time::Instant;
use ratatui::layout::Direction;
use super::*;
use crate::app::Mode;
use crate::terminal::TerminalRuntime;
use crate::workspace::Workspace;
const AREA: Rect = Rect::new(0, 0, 120, 40);
const SAMPLE_COUNT: usize = 40;
const WARMUP_COUNT: usize = 5;
#[derive(Clone, Copy)]
struct RenderStats {
median_us: u128,
p95_us: u128,
max_us: u128,
}
fn history() -> String {
(0..2_000).map(|line| format!("line-{line}\r\n")).collect()
}
fn runtime(history: &str) -> TerminalRuntime {
TerminalRuntime::test_with_scrollback_bytes(
AREA.width,
AREA.height,
1024 * 1024,
history.as_bytes(),
)
}
fn app_with_workspaces(workspace_count: usize) -> AppState {
let history = history();
let workspaces = (0..workspace_count)
.map(|index| {
let mut workspace = Workspace::test_new(&format!("bench-{}", index + 1));
let root_pane = workspace.tabs[0].root_pane;
workspace.tabs[0]
.runtimes
.insert(root_pane, runtime(&history));
workspace
})
.collect();
app_with(workspaces)
}
fn app_with_active_panes(pane_count: usize) -> AppState {
let history = history();
let mut workspace = Workspace::test_new("bench");
let root_pane = workspace.tabs[0].root_pane;
workspace.tabs[0]
.runtimes
.insert(root_pane, runtime(&history));
let mut pane_ids = vec![root_pane];
for index in 1..pane_count {
let target = pane_ids[(index - 1) / 2];
workspace.tabs[0].layout.focus_pane(target);
let direction = if index % 2 == 0 {
Direction::Vertical
} else {
Direction::Horizontal
};
let pane_id = workspace.test_split(direction);
workspace.tabs[0]
.runtimes
.insert(pane_id, runtime(&history));
pane_ids.push(pane_id);
}
app_with(vec![workspace])
}
fn app_with(workspaces: Vec<Workspace>) -> AppState {
let mut app = AppState::test_new();
app.mode = Mode::Terminal;
app.pane_scrollbars = true;
app.workspaces = workspaces;
app.active = Some(0);
app.selected = 0;
app
}
fn profile(mut app: AppState) -> RenderStats {
for _ in 0..WARMUP_COUNT {
black_box(render_virtual(&mut app, AREA, true));
}
let mut samples = Vec::with_capacity(SAMPLE_COUNT);
for _ in 0..SAMPLE_COUNT {
let started = Instant::now();
black_box(render_virtual(&mut app, AREA, true));
samples.push(started.elapsed().as_micros());
}
samples.sort_unstable();
RenderStats {
median_us: samples[SAMPLE_COUNT / 2],
p95_us: samples[(SAMPLE_COUNT - 1) * 95 / 100],
max_us: samples[SAMPLE_COUNT - 1],
}
}
fn profile_cardinalities(build: fn(usize) -> AppState) -> [(usize, RenderStats); 3] {
[1, 15, 50].map(|count| (count, profile(build(count))))
}
fn print_profiles(label: &str, profiles: [(usize, RenderStats); 3]) {
let baseline_median_us = profiles[0].1.median_us as f64;
let baseline_p95_us = profiles[0].1.p95_us as f64;
println!("{label}");
println!(" count median_us p95_us max_us median_vs_1x p95_vs_1x");
for (count, stats) in profiles {
println!(
"{count:>10} {:>9} {:>6} {:>6} {:>12.2} {:>9.2}",
stats.median_us,
stats.p95_us,
stats.max_us,
stats.median_us as f64 / baseline_median_us,
stats.p95_us as f64 / baseline_p95_us,
);
}
}
fn assert_full_render_avoids_aggregate_input_state(mut app: AppState, scenario: &str) {
crate::pane::reset_aggregate_input_state_reads();
black_box(render_virtual(&mut app, AREA, true));
assert_eq!(
crate::pane::aggregate_input_state_reads(),
0,
"full render collected aggregate input state for {scenario}",
);
}
#[tokio::test(flavor = "current_thread")]
async fn aggregate_input_state_counter_records_reads() {
let runtime = TerminalRuntime::test_with_screen_bytes(80, 24, b"");
crate::pane::reset_aggregate_input_state_reads();
black_box(runtime.input_state());
assert_eq!(crate::pane::aggregate_input_state_reads(), 1);
}
#[tokio::test(flavor = "current_thread")]
async fn full_render_avoids_aggregate_input_state_reads() {
assert_full_render_avoids_aggregate_input_state(
app_with_workspaces(15),
"background workspaces",
);
assert_full_render_avoids_aggregate_input_state(app_with_active_panes(15), "active panes");
}
#[tokio::test(flavor = "current_thread")]
#[ignore = "manual full-render scaling profile"]
async fn render_scale_profile() {
print_profiles(
"background-workspace resize/layout (one pane each)",
profile_cardinalities(app_with_workspaces),
);
print_profiles(
"active panes (one workspace)",
profile_cardinalities(app_with_active_panes),
);
}
}

View File

@ -311,10 +311,16 @@ impl TerminalRuntime {
self.0.word_motion_target(row, col, motion)
}
/// Collects the complete terminal input-mode snapshot.
///
/// This performs multiple terminal queries and may format keyboard state.
/// Keep it out of render/layout and pane-scaled loops; add a narrow accessor
/// when only one terminal fact is needed.
pub fn input_state(&self) -> Option<crate::pane::InputState> {
self.0.input_state()
}
/// Reads only whether the alternate screen is active.
pub fn alternate_screen_active(&self) -> bool {
self.0.alternate_screen_active()
}

View File

@ -31,6 +31,8 @@ fn pane_border_title(label: &str, pane_width: u16, _focused: bool) -> Option<Str
Some(format!(" {} ", truncate_end(label, max_label_width)))
}
// Full view computation reaches this helper for active and background panes.
// Keep terminal queries narrow, allocation-free, and short under the core lock.
fn terminal_inner_rect(rt: &TerminalRuntime, pane_inner: Rect, pane_scrollbars: bool) -> Rect {
if !pane_scrollbars || pane_inner.width <= 4 || rt.alternate_screen_active() {
return pane_inner;