Skip to content

refactor(perf): canonicalize the benchmark / wt-perf system#3403

Merged
max-sixty merged 9 commits into
mainfrom
simplify-wt-perf
Jul 10, 2026
Merged

refactor(perf): canonicalize the benchmark / wt-perf system#3403
max-sixty merged 9 commits into
mainfrom
simplify-wt-perf

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Implements the design proposal from this branch's first commit (b22aced, design/simplify-bench-perf.md — reviewable there; dropped from the merge candidate per the design/ convention). The perf tooling's emission side was already canonical; this consolidates the consumption side around one split: capture/harness work in wt-perf, analysis in worktrunk::trace with wt config state logs profile as its one CLI surface.

The seven design items:

  1. wt-perf cache-check deleted — its output was the cache field of logs profile --format=json. The tend statusline recipe now uses logs profile (its old pipe also fed human-format stderr into a JSON parser, so it's now correct as well as canonical).
  2. Timeline renderer moved into worktrunk::trace (src/trace/timeline.rs), sharing command_label/render_table/fmt_dur with the profile so the two views can't drift (they already had); drops wt-perf's tabwriter and insta deps. Durations now render in the profile's fixed-point format.
  3. Profile.by_context (BY CONTEXT table + JSON array): per-worktree subprocess totals, the one analysis that previously required trace_processor SQL. The SQL sections in benches/CLAUDE.md and the trace module docs are replaced by a logs profile pointer; Perfetto stays for visual critical-path work.
  4. One warm/cold bench runner (wt_perf::bench_wt) owns the BatchSize::PerIteration rationale once, replacing five inline copies. All runs now assert child exit status (benches/list.rs previously ignored it).
  5. Bench matrix pruned: real_repo keeps only the 8-worktree variants (scaling shape stays on synthetic worktree_scaling), cow_copy deleted (benchmarked a hand-written serial copy against production rayon — a settled choice), remove_e2e/first_output deleted (duplicate of first_output/remove). Ends three gist series with this PR as the single discontinuity point; saves roughly 15–20 min of the ~80-min daily bench run.
  6. wt-perf CLI tests moved in-package (native CARGO_BIN_EXE_wt-perf): the dummy builds.rs is gone and the nextest setup script builds only mock-stub. Note cargo test --test integration no longer runs these four tests; full-workspace runs still do.
  7. parse_config returns a SetupConfig enum covering mixed-W-B; the two fixture builders share init_bench_repo.

Merge with main (#3401): the new prune benches and fixtures landed mid-flight in exactly these files; the resolution routes them through the consolidated API — prune-M-U is a SetupConfig::Prune variant, the pair parsing is one shared parse_pair, and prune-real keeps its own cache-managed path (no --path, self-repairing). benches/prune.rs keeps its custom iter_batched arms deliberately: every variant asserts the candidate count from stdout, which bench_wt's shape doesn't express — the exact carve-out the cache-handling docs describe.

Two review passes (correctness + subtraction) then folded in: CacheReport trimmed to its same-context fields (the dropped six duplicated Profile fields or counted cross-context noise the text report never showed), render_table pads by display width (replacing an ASCII-only invariant the new BY CONTEXT table violated), the timeline's instant-event row gained test coverage, and Chrome args dropped the redundant duration_ms.

Decisions surfaced, not taken (from the review passes): (a) wt-perf setup could become --persist-only, dropping the interactive Enter-to-cleanup path — a UX preference; (b) whether the daily benchmark cron's gist time series justifies its remaining ~60 min/day is a cost/value call. Also deferred: measuring whether skeleton/cold adds signal over skeleton/warm before cutting it.

Gate green throughout (4374 tests); clippy --features shell-integration-tests clean; setup/timeline (warm, cold, chrome)/logs profile (text + JSON)/trace all driven end-to-end on a scratch mixed-4-8 repo.

This was written by Claude Code on behalf of max

max-sixty and others added 6 commits July 9, 2026 20:14
Design proposal (review-only, per design/ convention). Inventories the five
perf-tooling layers and proposes seven consolidations: delete the cache-check
subcommand (subsumed by logs profile --format=json), move the timeline
renderer into worktrunk::trace, add by_context to Profile and retire the
trace_processor SQL section, share one warm/cold bench runner, prune the
bench matrix (~15-20 min/day of CI), move the wt-perf CLI tests in-package,
and unify the setup config parser. Emission, chrome.rs, the separate wt-perf
package, and fixture shapes stay as is.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements design/simplify-bench-perf.md (items 1-7):

1. Delete wt-perf cache-check — its output is the .cache field of
   wt config state logs profile --format=json.
2. Move the timeline renderer into worktrunk::trace, sharing
   command_label/render_table/fmt_dur with the profile so the two views
   can't drift (drops wt-perf's tabwriter and insta deps; timeline
   durations now render in the profile's fixed-point format).
3. Add by_context to Profile (BY CONTEXT table + JSON array): per-worktree
   subprocess totals, the one analysis that previously required
   trace_processor SQL. benches/CLAUDE.md's SQL section becomes a pointer
   to logs profile + Perfetto; the running-tend statusline recipe now uses
   logs profile too.
4. One warm/cold bench runner (wt_perf::bench_wt) owns the
   BatchSize::PerIteration rationale once; five inline copies deleted.
   All runs now assert child exit status (list.rs previously ignored it).
5. Prune the matrix: real_repo keeps only the 8-worktree variants,
   cow_copy is deleted (shadow serial impl validating a settled choice),
   and the remove_e2e/first_output arm (duplicate of first_output/remove)
   is deleted. Ends three gist series; saves ~15-20 min/day of bench CI.
6. Move the wt-perf CLI tests in-package (native CARGO_BIN_EXE_wt-perf),
   deleting the dummy builds.rs and dropping wt-perf from the nextest
   setup script.
7. parse_config returns a SetupConfig enum covering mixed-W-B; shared
   init_bench_repo plumbing between the two fixture builders.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
From two review passes over the branch:

- CacheReport shrinks to its same-context fields. The six dropped fields
  (total_commands, total_time, contexts, duplicated_commands, extra_calls,
  unique_commands) duplicated Profile fields or tallied cross-context
  repeats — expected fan-out noise the text report never showed. Their
  only external reader was wt-perf cache-check, deleted earlier on this
  branch.
- src/trace/mod.rs docstring cut over to the module map + logs profile
  pointer; the trace_processor SQL examples it carried were retired from
  benches/CLAUDE.md earlier on this branch for the same reason.
- Chrome args drop duration_ms (redundant with the native dur field).
- render_table pads by display width (unicode-width, already a direct
  dep), replacing the ASCII-only invariant that the new BY CONTEXT table
  violated; CJK alignment test added.
- Timeline instant-event row gains coverage (the one render path no test
  exercised); stale /main suffix fixed in benches/CLAUDE.md's invalidate
  example.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main's diagnostic-section addition calls the per-render cache analysis 'a
separate tool', pointing at the wt-perf cache-check recipe — which this
branch already cut over to wt config state logs profile. Reword: it's the
same report against a statusline capture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per the design/ convention, proposals guide implementation PRs but don't
land on main. The doc remains in this branch's history (b22aced) and
its content is implemented by the commits above.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@max-sixty max-sixty changed the title docs(design): canonicalize the benchmark / wt-perf system refactor(perf): canonicalize the benchmark / wt-perf system Jul 10, 2026
# Conflicts:
#	tests/helpers/wt-perf/src/lib.rs
#	tests/helpers/wt-perf/src/main.rs

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The refactor is clean and internally consistent — the by_context aggregation, the moved traced/wall timeline computation, the SetupConfig enum, and the bench_wt warm/cold strategy all preserve the prior behavior, and I found no correctness issues. Two documentation-accuracy nits:

  • benches/CLAUDE.md (expected-numbers table, the remove_e2e/first_output row) — this row (~86 ms, "single-target validation up to first output") documents the bench variant this PR deletes from benches/remove.rs. It's outside this diff's hunks so I can't attach an inline suggestion; either drop the row or repoint it at the surviving first_output/remove variant in time_to_first_output.rs.

Inline suggestion below for the other one.

Comment thread src/trace/profile.rs Outdated
max-sixty and others added 2 commits July 9, 2026 23:41
The profile module spec referenced a nonexistent `wt diagnose` command
(the consumer is the -vv diagnostic report), and the expected-numbers
table still documented the deleted remove_e2e/first_output variant —
repointed at the surviving first_output/remove.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@max-sixty max-sixty marked this pull request as ready for review July 10, 2026 09:54
@max-sixty max-sixty merged commit 18d2bef into main Jul 10, 2026
44 checks passed
@max-sixty max-sixty deleted the simplify-wt-perf branch July 10, 2026 10:44
@max-sixty max-sixty mentioned this pull request Jul 10, 2026
max-sixty added a commit that referenced this pull request Jul 10, 2026
Release v0.67.0: version bump and changelog.

Highlights since v0.66.0: experimental `wt remove --reap` (#3396), `wt
switch -x` opening the picker (#3394), the termimad panic containment in
the picker's PR-comments preview (#3408), the `wt config show` SIGTTOU
fix (#3327), the `GIT_*` discovery-var scrub for `wt step for-each` and
the `--execute` fallback (#3400), and the `wt list` prompt-reserve fix
(#3409).

Seven commits landed on `main` after the initial cut; `origin/main` is
merged back in and the two user-facing ones (#3394, #3411) plus the BY
CONTEXT profile table (#3403) are folded into the changelog.

Pre-release validation: local pre-merge gate green on the merged tree
(4389 tests); nightly cross-platform suite green on the initial cut
([run
29086956095](https://github.com/max-sixty/worktrunk/actions/runs/29086956095)),
with the seven post-cut commits each validated by their own PR CI;
`cargo semver-checks` reports no breaking changes; data-loss surface
review of the cumulative diff (including the drift commits) found one
new destructive capability (`--reap`, explicit opt-in, adjudicated
acceptable).

> _This was written by Claude Code on behalf of max_
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.

2 participants