Skip to content

perf(benchmark): bound terminal run/batch registries - #738

Draft
beruro wants to merge 1 commit into
developfrom
junyu/perf-benchmark-registry-bounds
Draft

perf(benchmark): bound terminal run/batch registries#738
beruro wants to merge 1 commit into
developfrom
junyu/perf-benchmark-registry-bounds

Conversation

@beruro

@beruro beruro commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

BENCHMARK_RUNS and BENCHMARK_AGENT_BATCHES in src-tauri/src/benchmark/mod.rs are process-wide static HashMaps that only ever grow. Individual run logs are capped at 1,000 lines, but entries for finished (terminal) runs and batches are never evicted, so repeatedly running benchmarks in a long session grows memory without bound. Additionally, evaluator subprocesses (Python/Docker) still running at app exit were orphaned: the ExitRequested handler kills PTY shells but never terminated benchmark evaluators.

Solution

New src-tauri/src/benchmark/retention.rs implements a pure, unit-tested eviction policy:

  • Active entries are never evicted. Runs in running and batches in queued/running (and any unknown status, conservatively) always stay resident.
  • Terminal entries are bounded by "most recent N" (N=50 per registry, tunable constants). Count-based retention was chosen over TTL because it bounds memory even when many runs finish in a short window, needs no sweep timer, and is deterministic to test. Recency comes from the existing finished_at RFC3339 timestamps (all writers use Utc::now().to_rfc3339(), so lexicographic order is chronological); ties break by key so repeated application is idempotent.
  • The policy is the pure function terminal_keys_to_evict(entries: &[RetentionEntry], budget) -> Vec<String> (input: terminal flag + timestamp + budget; output: keys to delete). prune_terminal_runs / prune_terminal_agent_batches apply it under the registry lock at every site where an entry is inserted in or transitions to a terminal state (finish_run_with_result, run cancel, patch-only insert, batch item updates, batch persist/reload/refresh, batch cancel).
  • Batch eviction is lossless: batches are already persisted to disk on every mutation, and both benchmark_get_agent_batch_status and load_agent_batch_for_update already fall back to load_agent_batch_history and re-insert into the registry, so evicted batches reload on demand.
  • Shutdown handle release: new benchmark::terminate_running_evaluators_sync() is called from the ExitRequested handler (next to the existing PTY cleanup) and best-effort SIGTERMs evaluator PIDs still marked running. Cancel and error paths already release the child handle today (benchmark_cancel_run SIGTERMs the PID and run_swe_bench_process reaps it via child.wait(); error paths mark the run failed and drop the handle) — verified, unchanged.

Unit tests cover: all-active never evicted, oldest terminal evicted beyond budget, active entries not counted toward the budget, idempotency on repeated application, missing-timestamp ordering, key tie-breaks, zero budget, status classification, and map-level pruning for both registries.

Potential risks

  • Text conflict with junyu/fix-benchmark-personal-paths: a parallel branch is cleaning personal-path defaults in the same src-tauri/src/benchmark/ directory (it extracted paths.rs). This PR does not touch path defaults, but import blocks in mod.rs/commands.rs may conflict textually. Resolve at review/merge time.
  • Evicted terminal runs are unrecoverable: BENCHMARK_RUNS has no persistence layer, so benchmark_get_run_status returns "not found" for terminal runs older than the 50 most recent. Scope is limited: the frontend (useBenchmarkRun.ts) polls only while a run is running (never evicted) and stops at terminal; batch evaluation outcomes are mirrored into batch items and persisted with the batch, and refresh_agent_batch_evaluations already tolerates missing runs.
  • A run stuck in running (e.g. its spawned task panics before finishing) is never evicted, per the "active entries are never evicted" requirement. This matches pre-existing behavior; entries are still bounded by the 1,000-line log cap.
  • Batch persist failure + eviction: if persist_agent_batch_status fails (already only logged today) and the batch is later evicted, its latest in-memory state is lost. Same failure mode existed before for app restarts; eviction slightly widens the window.
  • Shutdown termination is best-effort: it uses try_lock and SIGTERM; a hard kill of the app still orphans evaluators (as before).

Verification

  • cargo check -p org2: NOT RUN — full cold compile of the app crate in a fresh worktree exceeded the local time budget on a machine shared with parallel build tasks; CI runs the full check suite for this PR (Draft until green).
  • cargo test -p org2 benchmark::retention: NOT RUN — same reason (test harness requires the same full crate compile). The 11 unit tests in retention.rs are pure-function tests with no I/O and will run in CI.
  • rustfmt on all touched files: run, clean.
  • Manual diff review: changes limited to src-tauri/src/benchmark/ (8 files incl. new retention.rs) + 3-line lib.rs exit-handler hook; no path-default code touched; unrelated pre-existing rustfmt churn in other modules explicitly reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant