From 154c52008558a6982c230cf9d948393e342c19d1 Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 18:25:44 +0900 Subject: [PATCH 1/5] Benchmark window-state resource usage in CI and compare PRs against main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a resource benchmark that measures CPU-time delta, RSS, and (best-effort via powermetrics) wakeups across three window states: focused, open-but-unfocused, and minimized. Pushes to main publish a baseline artifact; PR runs fetch the latest main baseline and post a comparison table to the job summary and an updated-in-place PR comment. CI can't script window states through AppleScript or synthetic keys (both need TCC grants a headless runner can't approve), so the app gains a benchmark-only remote control: launched with MACTERM_BENCHMARK=1 it listens for Darwin notifications (notifyutil -p), which need no permissions. Benchmark mode holds an NSProcessInfo activity assertion — otherwise App Nap suspends the occluded app and queues the control notifications undelivered — and skips the notification-permission prompt that would steal key focus mid-measurement. The harness launches via open --env rather than exec'ing the binary: launch-time activation counts as user intent, and SwiftUI doesn't create the window (or run the onAppear that wires app state) until activation is granted. App Support resolves via the user record, not $HOME, so benchmark runs point FileStorage at a throwaway directory via MACTERM_BENCHMARK_DATA_DIR to keep real app data untouched. --- .github/workflows/benchmark.yml | 108 ++++++++ AGENTS.md | 7 + Macterm/App/BenchmarkControl.swift | 126 +++++++++ Macterm/App/MactermApp.swift | 11 +- Macterm/Persistence/FileStorage.swift | 16 ++ mise.toml | 12 + scripts/bench.sh | 29 +++ scripts/benchmark.py | 358 ++++++++++++++++++++++++++ 8 files changed, 666 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/benchmark.yml create mode 100644 Macterm/App/BenchmarkControl.swift create mode 100755 scripts/bench.sh create mode 100644 scripts/benchmark.py diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 0000000..4b33879 --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,108 @@ +name: Benchmark + +# Window-state resource benchmark (scripts/benchmark.py). Pushes to main +# publish a `benchmark-results` artifact that becomes the baseline; PR runs +# download the latest main baseline and post a comparison table to the job +# summary and a (single, updated-in-place) PR comment. + +on: + push: + branches: [main] + pull_request: + branches: [main] + +concurrency: + group: benchmark-${{ github.ref }} + cancel-in-progress: true + +jobs: + changes: + name: Detect changes + runs-on: ubuntu-latest + outputs: + swift: ${{ steps.filter.outputs.swift }} + steps: + - uses: actions/checkout@v7 + - uses: dorny/paths-filter@v4 + id: filter + with: + filters: | + swift: + - "**/*.swift" + - "project.yml" + - "mise.toml" + - "scripts/**" + - ".github/workflows/benchmark.yml" + + benchmark: + name: Window states + needs: changes + if: needs.changes.outputs.swift == 'true' + runs-on: macos-26 + timeout-minutes: 30 + permissions: + contents: read + actions: read # download the baseline artifact from a main run + pull-requests: write # upsert the comparison comment + steps: + - name: Checkout + uses: actions/checkout@v7 + + - uses: jdx/mise-action@v4 + + - name: Setup GhosttyKit + run: mise run setup --verbose + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Build and benchmark + run: mise run bench --verbose + + - name: Upload results + uses: actions/upload-artifact@v4 + with: + name: benchmark-results + path: build/benchmark/results.json + + - name: Fetch main baseline + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # Walk recent successful main runs: a run whose benchmark job was + # skipped by the paths filter has no artifact, so try until one + # downloads. + for run_id in $(gh run list --workflow benchmark.yml --branch main --status success \ + --limit 20 --json databaseId --jq '.[].databaseId'); do + if gh run download "$run_id" --name benchmark-results --dir baseline 2>/dev/null; then + echo "baseline: run $run_id" + break + fi + done + [ -f baseline/results.json ] || echo "no main baseline found; reporting absolute values" + + - name: Report + run: | + args=(build/benchmark/results.json) + [ -f baseline/results.json ] && args+=(--baseline baseline/results.json) + python3 scripts/benchmark.py report "${args[@]}" > report.md + cat report.md >> "$GITHUB_STEP_SUMMARY" + + - name: Comment on PR + if: github.event_name == 'pull_request' + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + marker='' + printf '%s\n\n' "$marker" > comment.md + cat report.md >> comment.md + repo='${{ github.repository }}' + pr='${{ github.event.pull_request.number }}' + comment_id=$(gh api "repos/$repo/issues/$pr/comments" --paginate \ + --jq "[.[] | select(.body | startswith(\"$marker\"))][0].id // empty") + if [ -n "$comment_id" ]; then + gh api -X PATCH "repos/$repo/issues/comments/$comment_id" -F body=@comment.md --silent + else + gh api "repos/$repo/issues/$pr/comments" -F body=@comment.md --silent + fi diff --git a/AGENTS.md b/AGENTS.md index 58259f1..b54184f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,7 @@ mise run format # Auto-fix formatting with swiftformat mise run lint # swiftlint mise run test # Run the test suite mise run build # Release build + DMG +mise run bench # Release build + window-state resource benchmark ``` `format`, `lint`, and `test` show a spinner and print output only on failure. **Always pass `--verbose`** (e.g. `mise run test --verbose`) to stream the raw output. @@ -27,6 +28,12 @@ Auto-updates ship via [Sparkle](https://sparkle-project.org/) — daily backgrou Tag-pushed builds release via `.github/workflows/release.yml`, which needs repo secrets: `GH_PAT` (contents:read on `thdxg/ghostty`, downloads GhosttyKit), `SPARKLE_ED_PUBLIC_KEY` (baked into `Info.plist`), and `SPARKLE_ED_PRIVATE_KEY` (signs each DMG — **back it up; losing it means users can't auto-update to any further release**). The workflow appends an `` to `appcast.xml` on `gh-pages` (served at `https://thdxg.github.io/macterm/appcast.xml`, the feed URL in `Info.plist`) along with a per-version notes page rendered from the GitHub Release body (`publish-appcast.sh`). +## Benchmarks + +`.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — the report flags only ±25% swings. + +The harness (`scripts/benchmark.py`, invoked by `mise run bench`) launches the Release app under a throwaway `$HOME` (hermetic: no user ghostty config, fresh App Support) with `MACTERM_BENCHMARK=1`, which arms `BenchmarkControl` — Darwin-notification remote control (`notifyutil -p com.thdxg.macterm.bench.`; commands: `open-project`, `activate`, `minimize`, `restore`). Darwin notifications and `open`-based activation need no TCC grants, which is what lets a headless CI runner script window states at all (AppleScript/synthetic keys would need an Accessibility grant). Benchmark mode also skips the notification-permission prompt, which would steal key focus mid-measurement. + ## Architecture ``` diff --git a/Macterm/App/BenchmarkControl.swift b/Macterm/App/BenchmarkControl.swift new file mode 100644 index 0000000..50b0794 --- /dev/null +++ b/Macterm/App/BenchmarkControl.swift @@ -0,0 +1,126 @@ +import AppKit +import os + +private let logger = Logger(subsystem: appBundleID, category: "BenchmarkControl") + +/// CI-only remote control for the window-state benchmark +/// (`.github/workflows/benchmark.yml` → `scripts/benchmark.py`). +/// +/// When the app is launched with `MACTERM_BENCHMARK=1` it listens for Darwin +/// notifications (posted from a shell with `notifyutil -p `) and drives +/// itself through the benchmarked window states. Darwin notifications need no +/// TCC grant — unlike AppleScript/System Events or synthetic key events — so a +/// headless CI runner can script the app without touching the SIP-protected +/// permission database. +@MainActor +enum BenchmarkControl { + static let isEnabled = ProcessInfo.processInfo.environment["MACTERM_BENCHMARK"] == "1" + + // Strong references are fine here: both objects live for the app's + // lifetime, and benchmark mode never releases them. + private static var appState: AppState? + private static var projectStore: ProjectStore? + + /// An `openProject` command can arrive before `connect`: SwiftUI only + /// creates the window (whose `onAppear` wires the state objects) once + /// macOS grants activation, and the harness polls the command until + /// then. Park it instead of dropping it. + private static var pendingOpenProject = false + + /// Called by `AppDelegate.installResponders` once the state objects exist. + static func connect(appState: AppState, projectStore: ProjectStore) { + self.appState = appState + self.projectStore = projectStore + if pendingOpenProject { + pendingOpenProject = false + openProject() + } + } + + private static let notificationPrefix = "com.thdxg.macterm.bench." + + /// Held for the app's lifetime in benchmark mode. Without it, App Nap + /// suspends the backgrounded/occluded app — queued Darwin notifications + /// go undelivered and, on a busy desktop, the window never even appears. + /// It also makes the numbers measure what we care about: the app's own + /// timer/render/wakeup behavior, not the OS's nap throttling masking a + /// regression. + private static var activity: NSObjectProtocol? + + private enum Command: String, CaseIterable { + case openProject = "open-project" + case activate + case minimize + case restore + } + + static func install() { + guard isEnabled else { return } + activity = ProcessInfo.processInfo.beginActivity( + options: .userInitiatedAllowingIdleSystemSleep, + reason: "window-state benchmark" + ) + let center = CFNotificationCenterGetDarwinNotifyCenter() + for command in Command.allCases { + CFNotificationCenterAddObserver( + center, + nil, + { _, _, name, _, _ in + guard let name else { return } + let raw = name.rawValue as String + DispatchQueue.main.async { + MainActor.assumeIsolated { BenchmarkControl.handle(raw) } + } + }, + (notificationPrefix + command.rawValue) as CFString, + nil, + .deliverImmediately + ) + } + logger.info("benchmark control listening (MACTERM_BENCHMARK=1)") + } + + private static func handle(_ rawName: String) { + guard let command = Command(rawValue: String(rawName.dropFirst(notificationPrefix.count))) else { return } + logger.info("bench command: \(command.rawValue, privacy: .public)") + switch command { + case .openProject: + openProject() + case .activate: + NSApp.activate() + mainWindow?.makeKeyAndOrderFront(nil) + case .minimize: + mainWindow?.miniaturize(nil) + case .restore: + mainWindow?.deminiaturize(nil) + NSApp.activate() + mainWindow?.makeKeyAndOrderFront(nil) + } + } + + /// Select (creating if needed) a project at `MACTERM_BENCHMARK_DIR` (or + /// `$HOME`) so the window hosts a live terminal pane. A fresh CI runner + /// otherwise sits on the WelcomeView, and the benchmark would measure an + /// app with no surface, no shell, and no foreground-process poll. + private static func openProject() { + guard let appState, let projectStore else { + logger.info("bench open-project queued until state objects exist") + pendingOpenProject = true + return + } + let path = ProcessInfo.processInfo.environment["MACTERM_BENCHMARK_DIR"] ?? NSHomeDirectory() + if let existing = projectStore.projects.first(where: { $0.path == path }) { + appState.selectProject(existing) + return + } + let project = Project(name: "Benchmark", path: path, sortOrder: projectStore.projects.count) + projectStore.add(project) + appState.selectProject(project) + } + + /// Same filter as `AppDelegate.reopenIfNeeded`: the single main window is + /// the only non-panel window (panels: quick terminal, settings). + private static var mainWindow: NSWindow? { + NSApp.windows.first { !($0 is NSPanel) } + } +} diff --git a/Macterm/App/MactermApp.swift b/Macterm/App/MactermApp.swift index 2d9d70d..8609321 100644 --- a/Macterm/App/MactermApp.swift +++ b/Macterm/App/MactermApp.swift @@ -190,7 +190,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate { return } UNUserNotificationCenter.current().delegate = NotificationHandler.shared - NotificationHandler.shared.requestAuthorization() + if BenchmarkControl.isEnabled { + // Under the CI benchmark, the notification-permission alert would + // steal key focus mid-measurement and nobody is there to answer it. + BenchmarkControl.install() + } else { + NotificationHandler.shared.requestAuthorization() + } NSApp.setActivationPolicy(.regular) NSApp.activate() _ = GhosttyApp.shared @@ -254,6 +260,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate { func installResponders(appState: AppState, projectStore: ProjectStore) { guard !hasInstalledResponders else { return } hasInstalledResponders = true + if BenchmarkControl.isEnabled { + BenchmarkControl.connect(appState: appState, projectStore: projectStore) + } KeyRouter.shared.register(PaletteResponder(appState: appState)) KeyRouter.shared.register(QuickTerminalResponder()) let mainResponder = MainAppResponder(appState: appState, projectStore: projectStore) diff --git a/Macterm/Persistence/FileStorage.swift b/Macterm/Persistence/FileStorage.swift index 59ad7aa..21bc919 100644 --- a/Macterm/Persistence/FileStorage.swift +++ b/Macterm/Persistence/FileStorage.swift @@ -6,6 +6,22 @@ enum FileStorage { } static func appSupportDirectory() -> URL { + // The benchmark harness (scripts/benchmark.py) points this at a + // throwaway directory so a benchmark run can't read or pollute the + // real app data. An env override is required because + // `.applicationSupportDirectory` resolves via the user record, not + // `$HOME` — a temp `$HOME` doesn't isolate it. + if let benchDir = ProcessInfo.processInfo.environment["MACTERM_BENCHMARK_DATA_DIR"], + !benchDir.isEmpty + { + let dir = URL(fileURLWithPath: benchDir, isDirectory: true) + try? FileManager.default.createDirectory( + at: dir, + withIntermediateDirectories: true, + attributes: [.posixPermissions: 0o700] + ) + return dir + } guard let appSupport = FileManager.default.urls( for: .applicationSupportDirectory, in: .userDomainMask diff --git a/mise.toml b/mise.toml index 9b682d2..6e1fecf 100644 --- a/mise.toml +++ b/mise.toml @@ -124,6 +124,18 @@ run = """ fi """ +[tasks.bench] +description = "Build the release app and run the window-state benchmark" +usage = 'flag "-v --verbose" help="Stream raw output instead of the spinner"' +run = """ + source scripts/_lib.sh + if [[ "$usage_verbose" == "true" ]]; then + ./scripts/bench.sh + else + run_step "Benchmarking..." ./scripts/bench.sh + fi +""" + [tasks.clean] description = "Clean build artifacts" run = "rm -rf build Macterm.xcodeproj" diff --git a/scripts/bench.sh b/scripts/bench.sh new file mode 100755 index 0000000..24f26ff --- /dev/null +++ b/scripts/bench.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +set -euo pipefail + +PROJECT_ROOT="$PWD" +DERIVED_DATA="$PROJECT_ROOT/build/DerivedData" +OUT_DIR="$PROJECT_ROOT/build/benchmark" + +# Ensure GhosttyKit + bundled resources are present before xcodegen resolves +# the folder references. Idempotent; no-op in CI where setup already ran. +"$PROJECT_ROOT/scripts/setup.sh" + +xcodegen generate --spec "$PROJECT_ROOT/project.yml" >/dev/null + +# Release configuration so the numbers reflect what ships. ONLY_ACTIVE_ARCH +# skips the other half of the universal binary — the benchmark only runs on +# the build machine's own arch. +xcodebuild \ + -project Macterm.xcodeproj \ + -scheme Macterm \ + -configuration Release \ + -derivedDataPath "$DERIVED_DATA" \ + ONLY_ACTIVE_ARCH=YES \ + build \ + | (xcbeautify --quiet 2>/dev/null || cat) + +python3 "$PROJECT_ROOT/scripts/benchmark.py" run \ + --app "$DERIVED_DATA/Build/Products/Release/Macterm.app" \ + --out "$OUT_DIR/results.json" \ + --seconds "${BENCH_SECONDS:-30}" diff --git a/scripts/benchmark.py b/scripts/benchmark.py new file mode 100644 index 0000000..31a8fe3 --- /dev/null +++ b/scripts/benchmark.py @@ -0,0 +1,358 @@ +#!/usr/bin/env python3 +"""Window-state resource benchmark. + +Measures Macterm's CPU and memory across three window states — focused, +open-but-unfocused, and minimized — by launching the app with the +MACTERM_BENCHMARK=1 control hook (Macterm/App/BenchmarkControl.swift) and +driving it with Darwin notifications (`notifyutil -p`) plus LaunchServices +activation (`open`). Neither needs a TCC grant, so this runs on a stock CI +runner. + +Per state: settle, then over a sampling window read the process's CPU-time +delta (the primary metric — immune to sampling aliasing) and median RSS. +When passwordless sudo is available (GitHub runners), a concurrent +`powermetrics --samplers tasks` window adds idle-wakeups/s and CPU ms/s; +inside virtualized runners powermetrics is best-effort and the fields are +null when it fails. + +Subcommands: + run launch the app, sample each state, write a results JSON + report render a results JSON (optionally vs. a baseline) as markdown +""" + +import argparse +import json +import os +import re +import shutil +import signal +import statistics +import subprocess +import sys +import tempfile +import time + +STATES = ("focused", "unfocused", "minimized") +NOTIFY_PREFIX = "com.thdxg.macterm.bench." + + +def sh(args, **kwargs): + return subprocess.run(args, capture_output=True, text=True, **kwargs) + + +def notify(command): + sh(["notifyutil", "-p", NOTIFY_PREFIX + command]) + + +def read_info_plist_key(app, key): + result = sh(["defaults", "read", os.path.join(app, "Contents", "Info"), key]) + if result.returncode != 0: + sys.exit(f"error: cannot read {key} from {app}: {result.stderr.strip()}") + return result.stdout.strip() + + +def parse_cputime(value): + """ps cputime: [[dd-]hh:]mm:ss.cc → seconds.""" + days = 0 + if "-" in value: + day_part, value = value.split("-", 1) + days = int(day_part) + parts = [float(p) for p in value.split(":")] + seconds = 0.0 + for part in parts: + seconds = seconds * 60 + part + return days * 86400 + seconds + + +def ps_sample(pid): + """Return (cputime_seconds, rss_kb) or None if the process is gone.""" + result = sh(["ps", "-p", str(pid), "-o", "cputime=,rss="]) + fields = result.stdout.split() + if result.returncode != 0 or len(fields) != 2: + return None + return parse_cputime(fields[0]), int(fields[1]) + + +def start_powermetrics(seconds): + """Kick off one powermetrics tasks-sampler window; None if sudo needs a password.""" + if sh(["sudo", "-n", "true"]).returncode != 0: + return None + return subprocess.Popen( + ["sudo", "-n", "powermetrics", "--samplers", "tasks", + "-i", str(seconds * 1000), "-n", "1"], + stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, text=True, + ) + + +def parse_powermetrics(output, pid): + """Pull (cpu_ms_per_s, wakeups_per_s) for pid from a tasks-sampler table. + + Rows are `Name ID CPU ms/s User% Deadlines (<2 ms, 2-5 ms) Wakeups + (Intr, Pkg idle)` with an optional trailing GPU ms/s column depending on + the macOS release — so index the numeric fields from the front, never the + back. The name can contain spaces; anchor on the pid column instead. + """ + for line in output.splitlines(): + tokens = line.split() + try: + pid_index = tokens.index(str(pid)) + except ValueError: + continue + numbers = [] + for token in tokens[pid_index + 1:]: + match = re.match(r"^\d+(\.\d+)?$", token) + if match: + numbers.append(float(token)) + if len(numbers) >= 6: + # cpu, user%, deadline1, deadline2, wakeups-intr, wakeups-pkg-idle + return numbers[0], numbers[4] + numbers[5] + return None + + +def check_alive(pid): + # The app is launchd's child (launched via `open`), so it vanishes from + # ps on death — but guard against a lingering zombie too, which ps still + # reports (with rss 0 and a reset cputime). + result = sh(["ps", "-p", str(pid), "-o", "state="]) + state = result.stdout.strip() + if result.returncode != 0 or not state or state.startswith("Z"): + sys.exit("error: app process died mid-benchmark") + + +def sample_state(pid, seconds): + check_alive(pid) + start = ps_sample(pid) + if start is None: + sys.exit("error: app process not sampleable") + power = start_powermetrics(seconds) + + rss_samples = [] + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + time.sleep(1) + check_alive(pid) + sample = ps_sample(pid) + if sample is None: + sys.exit("error: app process died mid-sample") + rss_samples.append(sample[1]) + end = ps_sample(pid) + + cpu_ms_per_s = wakeups_per_s = None + if power is not None: + try: + output, _ = power.communicate(timeout=seconds + 30) + parsed = parse_powermetrics(output, pid) + if parsed: + cpu_ms_per_s, wakeups_per_s = parsed + except subprocess.TimeoutExpired: + power.kill() + + return { + "cpu_pct": round((end[0] - start[0]) / seconds * 100, 3), + "rss_mb": round(statistics.median(rss_samples) / 1024, 1), + "cpu_ms_per_s": cpu_ms_per_s, + "wakeups_per_s": wakeups_per_s, + } + + +def git_sha(): + sha = os.environ.get("GITHUB_SHA") + if sha: + return sha + result = sh(["git", "rev-parse", "HEAD"]) + return result.stdout.strip() if result.returncode == 0 else "unknown" + + +def cmd_run(args): + app = os.path.abspath(args.app) + executable = read_info_plist_key(app, "CFBundleExecutable") + binary = os.path.join(app, "Contents", "MacOS", executable) + + # Isolate the run. A throwaway $HOME keeps the spawned shell's rc files + # and $HOME-derived config out of the picture, but App Support and + # preferences resolve via the user record, NOT $HOME — so app data + # isolation needs the explicit MACTERM_BENCHMARK_DATA_DIR override + # (FileStorage.swift). Without it, a local run reads and writes the real + # app's projects/workspaces. + home = tempfile.mkdtemp(prefix="macterm-bench-home-") + bench_env = { + "MACTERM_BENCHMARK": "1", + "MACTERM_BENCHMARK_DATA_DIR": os.path.join(home, "app-data"), + "HOME": home, + } + + # Launch via LaunchServices rather than exec'ing the binary: launch-time + # activation counts as user intent, so the app actually becomes active + # and SwiftUI creates its window. (A directly-exec'd app starts + # backgrounded, and macOS's cooperative activation can deny post-hoc + # activation requests indefinitely on a busy desktop — SwiftUI then + # never creates a window at all.) + print(f"launching {app}", flush=True) + env_args = [f"--env={key}={value}" for key, value in bench_env.items()] + result = sh(["open", "-n", *env_args, app]) + if result.returncode != 0: + sys.exit(f"error: open failed: {result.stderr.strip()}") + + pid = None + for _ in range(20): + pids = sh(["pgrep", "-f", binary]).stdout.split() + if pids: + pid = int(pids[0]) + break + time.sleep(0.5) + if pid is None: + sys.exit("error: app process did not appear after launch") + + try: + time.sleep(args.boot_settle) + check_alive(pid) + + # Ask the app to open a project so a real shell + surface is on + # screen. ProjectStore.add saves projects.json into the isolated + # data dir synchronously, so its existence is the readiness marker; + # retry (idempotently) rather than sleep-and-hope, since the window + # this needs only exists once activation was granted. + project_marker = os.path.join(bench_env["MACTERM_BENCHMARK_DATA_DIR"], "projects.json") + for _ in range(30): + notify("activate") + notify("open-project") + time.sleep(2) + check_alive(pid) + if os.path.exists(project_marker): + break + else: + sys.exit( + "error: app never opened the benchmark project — window creation " + "requires app activation; is someone actively using this desktop?" + ) + # Let the shell spawn and the initial render burst drain. + time.sleep(args.boot_settle) + + results = {} + for state in STATES: + if state == "focused": + # `open` on the running bundle activates it via LaunchServices + # (user-intent level, unlike cooperative NSApp.activate). + sh(["open", app]) + notify("activate") + elif state == "unfocused": + sh(["open", "-a", "Finder"]) + elif state == "minimized": + notify("minimize") + time.sleep(args.settle) + print(f"sampling {state} for {args.seconds}s", flush=True) + results[state] = sample_state(pid, args.seconds) + print(f" {results[state]}", flush=True) + finally: + # SIGKILL: SIGTERM would hang on the quit-confirmation dialog for the + # running shell. + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + shutil.rmtree(home, ignore_errors=True) + + payload = { + "schema": 1, + "commit": git_sha(), + "seconds_per_state": args.seconds, + "states": results, + } + os.makedirs(os.path.dirname(os.path.abspath(args.out)), exist_ok=True) + with open(args.out, "w") as f: + json.dump(payload, f, indent=2) + print(f"wrote {args.out}") + + +METRICS = ( + ("cpu_pct", "CPU %", "{:.2f}"), + ("rss_mb", "Memory (RSS MB)", "{:.1f}"), + ("cpu_ms_per_s", "CPU ms/s (powermetrics)", "{:.1f}"), + ("wakeups_per_s", "Wakeups/s (powermetrics)", "{:.1f}"), +) + + +def fmt(value, pattern): + return pattern.format(value) if value is not None else "—" + + +def delta_cell(base, current): + if base is None or current is None: + return "—" + diff = current - base + if base == 0: + return "—" if diff == 0 else f"+{diff:.2f}" + pct = diff / base * 100 + arrow = "🔺" if pct > 25 else ("🔻" if pct < -25 else "") + return f"{pct:+.0f}% {arrow}".strip() + + +def cmd_report(args): + with open(args.results) as f: + current = json.load(f) + baseline = None + if args.baseline: + with open(args.baseline) as f: + baseline = json.load(f) + + lines = ["## Window-state benchmark", ""] + if baseline: + base_ref = f"main@{baseline.get('commit', 'unknown')[:9]}" + lines += [ + f"| State | Metric | {base_ref} | this branch | Δ |", + "|---|---|---:|---:|---:|", + ] + else: + lines += ["| State | Metric | Value |", "|---|---|---:|"] + + for state in STATES: + cur_state = current["states"].get(state, {}) + base_state = (baseline or {}).get("states", {}).get(state, {}) + for key, label, pattern in METRICS: + cur = cur_state.get(key) + if cur is None and (not baseline or base_state.get(key) is None): + continue + if baseline: + base = base_state.get(key) + lines.append( + f"| {state} | {label} | {fmt(base, pattern)} " + f"| {fmt(cur, pattern)} | {delta_cell(base, cur)} |" + ) + else: + lines.append(f"| {state} | {label} | {fmt(cur, pattern)} |") + + lines += [ + "", + f"_{current['seconds_per_state']}s sampling window per state; CPU % is the " + "process CPU-time delta over the window. Runs land on different shared " + "runners, so treat small deltas as noise — 🔺/🔻 marks ±25%._", + ] + if baseline is None: + lines.append("") + lines.append("_No main-branch baseline found; showing absolute values only._") + print("\n".join(lines)) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + sub = parser.add_subparsers(dest="command", required=True) + + run = sub.add_parser("run", help="launch the app and benchmark each window state") + run.add_argument("--app", required=True, help="path to the built Macterm.app") + run.add_argument("--out", required=True, help="path for the results JSON") + run.add_argument("--seconds", type=int, default=30, help="sampling window per state") + run.add_argument("--settle", type=int, default=5, help="settle time after each state change") + run.add_argument("--boot-settle", type=int, default=10, help="settle time after launch / project open") + run.set_defaults(func=cmd_run) + + report = sub.add_parser("report", help="render results as markdown") + report.add_argument("results", help="results JSON from `run`") + report.add_argument("--baseline", help="baseline results JSON to compare against") + report.set_defaults(func=cmd_report) + + args = parser.parse_args() + args.func(args) + + +if __name__ == "__main__": + main() From 1024da7b8861119256fe9b93c2c73b28dd95b245 Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 18:35:00 +0900 Subject: [PATCH 2/5] Keep Sparkle and activation from stalling the benchmark launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bench-built Release app carries the placeholder EdDSA key, so Sparkle's updater fails to start and raises an app-modal alert that blocks the launch run loop until someone clicks OK — locally that interrupts the run, and on CI nobody is there to click. Don't start the updater in benchmark mode. Also nudge activation once a second until the window exists (a single post-launch request can be denied or race session readiness on a fresh runner), and dump the app's log plus a screenshot as an artifact when the readiness marker never appears. --- .github/workflows/benchmark.yml | 8 ++++++++ AGENTS.md | 2 +- Macterm/App/BenchmarkControl.swift | 20 ++++++++++++++++++++ Macterm/App/Updater.swift | 7 ++++++- scripts/benchmark.py | 14 ++++++++++++++ 5 files changed, 49 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 4b33879..eac2701 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -64,6 +64,14 @@ jobs: name: benchmark-results path: build/benchmark/results.json + - name: Upload failure diagnostics + if: failure() + uses: actions/upload-artifact@v4 + with: + name: benchmark-diagnostics + path: build/benchmark/diagnostics/ + if-no-files-found: ignore + - name: Fetch main baseline if: github.event_name == 'pull_request' env: diff --git a/AGENTS.md b/AGENTS.md index b54184f..e930d92 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ Tag-pushed builds release via `.github/workflows/release.yml`, which needs repo `.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — the report flags only ±25% swings. -The harness (`scripts/benchmark.py`, invoked by `mise run bench`) launches the Release app under a throwaway `$HOME` (hermetic: no user ghostty config, fresh App Support) with `MACTERM_BENCHMARK=1`, which arms `BenchmarkControl` — Darwin-notification remote control (`notifyutil -p com.thdxg.macterm.bench.`; commands: `open-project`, `activate`, `minimize`, `restore`). Darwin notifications and `open`-based activation need no TCC grants, which is what lets a headless CI runner script window states at all (AppleScript/synthetic keys would need an Accessibility grant). Benchmark mode also skips the notification-permission prompt, which would steal key focus mid-measurement. +The harness (`scripts/benchmark.py`, invoked by `mise run bench`) launches the Release app under a throwaway `$HOME` (hermetic: no user ghostty config, fresh App Support) with `MACTERM_BENCHMARK=1`, which arms `BenchmarkControl` — Darwin-notification remote control (`notifyutil -p com.thdxg.macterm.bench.`; commands: `open-project`, `activate`, `minimize`, `restore`). Darwin notifications and `open`-based activation need no TCC grants, which is what lets a headless CI runner script window states at all (AppleScript/synthetic keys would need an Accessibility grant). Benchmark mode also skips the notification-permission prompt (it would steal key focus mid-measurement) and doesn't start the Sparkle updater — a bench-built Release app carries the placeholder EdDSA key, so Sparkle's failed-start alert would otherwise block the launch run loop until someone clicks OK. ## Architecture diff --git a/Macterm/App/BenchmarkControl.swift b/Macterm/App/BenchmarkControl.swift index 50b0794..6fdee43 100644 --- a/Macterm/App/BenchmarkControl.swift +++ b/Macterm/App/BenchmarkControl.swift @@ -60,6 +60,7 @@ enum BenchmarkControl { options: .userInitiatedAllowingIdleSystemSleep, reason: "window-state benchmark" ) + nudgeActivationUntilWindowExists() let center = CFNotificationCenterGetDarwinNotifyCenter() for command in Command.allCases { CFNotificationCenterAddObserver( @@ -80,6 +81,25 @@ enum BenchmarkControl { logger.info("benchmark control listening (MACTERM_BENCHMARK=1)") } + /// SwiftUI doesn't create the WindowGroup window until the app becomes + /// active, and a single post-launch activation request can be denied + /// (cooperative activation) or arrive before the session is ready on a + /// fresh CI runner. Re-request every second until the window exists. + private static func nudgeActivationUntilWindowExists(attempt: Int = 0) { + if mainWindow != nil { + logger.info("bench window exists after \(attempt, privacy: .public) activation nudges") + return + } + guard attempt < 120 else { + logger.error("bench window never appeared; giving up activation nudges") + return + } + NSApp.activate() + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + nudgeActivationUntilWindowExists(attempt: attempt + 1) + } + } + private static func handle(_ rawName: String) { guard let command = Command(rawValue: String(rawName.dropFirst(notificationPrefix.count))) else { return } logger.info("bench command: \(command.rawValue, privacy: .public)") diff --git a/Macterm/App/Updater.swift b/Macterm/App/Updater.swift index 35add34..2924546 100644 --- a/Macterm/App/Updater.swift +++ b/Macterm/App/Updater.swift @@ -33,11 +33,16 @@ final class Updater: ObservableObject { // the production EdDSA key, so it pops an "Unable to Check For // Updates" dialog on every launch. Start the controller without // kicking off update checks; release builds still auto-check. + // + // Benchmark mode is a Release build with the placeholder key + // (scripts/bench.sh builds without SPARKLE_ED_PUBLIC_KEY), so the + // updater fails to start and its app-modal alert blocks the run + // loop at launch — on CI nobody can click OK. let startUpdater: Bool = { #if DEBUG return false #else - return true + return !BenchmarkControl.isEnabled #endif }() let updaterDelegate = updaterDelegate diff --git a/scripts/benchmark.py b/scripts/benchmark.py index 31a8fe3..ea53b89 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -155,6 +155,19 @@ def sample_state(pid, seconds): } +def dump_diagnostics(out_path): + """On failure, surface what the app saw: its log lines and a screenshot.""" + diag_dir = os.path.join(os.path.dirname(os.path.abspath(out_path)), "diagnostics") + os.makedirs(diag_dir, exist_ok=True) + sh(["screencapture", "-x", os.path.join(diag_dir, "screen.png")]) + result = sh([ + "log", "show", "--last", "5m", "--style", "compact", "--level", "debug", + "--predicate", 'subsystem == "com.thdxg.macterm"', + ]) + print("--- app log ---", flush=True) + print(result.stdout or result.stderr, flush=True) + + def git_sha(): sha = os.environ.get("GITHUB_SHA") if sha: @@ -221,6 +234,7 @@ def cmd_run(args): if os.path.exists(project_marker): break else: + dump_diagnostics(args.out) sys.exit( "error: app never opened the benchmark project — window creation " "requires app activation; is someone actively using this desktop?" From f36c0a93dfa1dc1feadb0594d1fd4d04418eefbd Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 18:43:44 +0900 Subject: [PATCH 3/5] Label PRs whose benchmark deltas cross the significance threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A delta is significant only when it clears both ±25% and the metric's absolute noise floor — a pure percentage threshold would flag noise on tiny absolute values (minimized CPU going 0.03% → 0.05% reads as +67%). Significant deltas add a benchmark-regression or benchmark-improvement label (kept in sync on later pushes) and the PR comment gains a section explaining exactly which metrics triggered the label. --- .github/workflows/benchmark.yml | 31 ++++++++++++- AGENTS.md | 2 +- scripts/benchmark.py | 82 ++++++++++++++++++++++++++++----- 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index eac2701..fe214df 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -44,6 +44,7 @@ jobs: contents: read actions: read # download the baseline artifact from a main run pull-requests: write # upsert the comparison comment + issues: write # create/apply the benchmark-regression/-improvement labels steps: - name: Checkout uses: actions/checkout@v7 @@ -91,11 +92,39 @@ jobs: - name: Report run: | - args=(build/benchmark/results.json) + args=(build/benchmark/results.json --verdict build/benchmark/verdict.json) [ -f baseline/results.json ] && args+=(--baseline baseline/results.json) python3 scripts/benchmark.py report "${args[@]}" > report.md cat report.md >> "$GITHUB_STEP_SUMMARY" + - name: Label PR on significant deltas + if: github.event_name == 'pull_request' + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + verdict=build/benchmark/verdict.json + regression=$(jq -r '.regression' "$verdict" 2>/dev/null || echo false) + improvement=$(jq -r '.improvement' "$verdict" 2>/dev/null || echo false) + repo='${{ github.repository }}' + pr='${{ github.event.pull_request.number }}' + gh label create benchmark-regression --repo "$repo" --force \ + --color D93F0B --description "CI benchmark: significant resource regression vs main" + gh label create benchmark-improvement --repo "$repo" --force \ + --color 0E8A16 --description "CI benchmark: significant resource improvement vs main" + # Keep labels in sync with the latest run: apply when the verdict + # says so, remove a stale one when a newer push clears it. + if [ "$regression" = "true" ]; then + gh pr edit "$pr" --repo "$repo" --add-label benchmark-regression + else + gh pr edit "$pr" --repo "$repo" --remove-label benchmark-regression || true + fi + if [ "$improvement" = "true" ]; then + gh pr edit "$pr" --repo "$repo" --add-label benchmark-improvement + else + gh pr edit "$pr" --repo "$repo" --remove-label benchmark-improvement || true + fi + - name: Comment on PR if: github.event_name == 'pull_request' continue-on-error: true diff --git a/AGENTS.md b/AGENTS.md index e930d92..8e81ec9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ Tag-pushed builds release via `.github/workflows/release.yml`, which needs repo ## Benchmarks -`.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — the report flags only ±25% swings. +`.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — a delta is flagged only when it clears both ±25% _and_ the metric's absolute noise floor (`METRICS` in `benchmark.py`). Significant deltas add the `benchmark-regression` / `benchmark-improvement` PR label (removed again when a later push clears them) and the comment explains which metrics triggered it. The harness (`scripts/benchmark.py`, invoked by `mise run bench`) launches the Release app under a throwaway `$HOME` (hermetic: no user ghostty config, fresh App Support) with `MACTERM_BENCHMARK=1`, which arms `BenchmarkControl` — Darwin-notification remote control (`notifyutil -p com.thdxg.macterm.bench.`; commands: `open-project`, `activate`, `minimize`, `restore`). Darwin notifications and `open`-based activation need no TCC grants, which is what lets a headless CI runner script window states at all (AppleScript/synthetic keys would need an Accessibility grant). Benchmark mode also skips the notification-permission prompt (it would steal key focus mid-measurement) and doesn't start the Sparkle updater — a bench-built Release app carries the placeholder EdDSA key, so Sparkle's failed-start alert would otherwise block the launch run loop until someone clicks OK. diff --git a/scripts/benchmark.py b/scripts/benchmark.py index ea53b89..4317070 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -278,11 +278,16 @@ def cmd_run(args): print(f"wrote {args.out}") +# A delta is significant when it clears BOTH bars: the relative threshold +# and the metric's absolute noise floor. The floor keeps tiny absolute +# swings (minimized CPU going 0.03 → 0.05 is "+66%") from flagging noise. +THRESHOLD_PCT = 25 METRICS = ( - ("cpu_pct", "CPU %", "{:.2f}"), - ("rss_mb", "Memory (RSS MB)", "{:.1f}"), - ("cpu_ms_per_s", "CPU ms/s (powermetrics)", "{:.1f}"), - ("wakeups_per_s", "Wakeups/s (powermetrics)", "{:.1f}"), + # key, table label, format, absolute noise floor + ("cpu_pct", "CPU %", "{:.2f}", 0.5), + ("rss_mb", "Memory (RSS MB)", "{:.1f}", 25.0), + ("cpu_ms_per_s", "CPU ms/s (powermetrics)", "{:.1f}", 5.0), + ("wakeups_per_s", "Wakeups/s (powermetrics)", "{:.1f}", 50.0), ) @@ -290,15 +295,26 @@ def fmt(value, pattern): return pattern.format(value) if value is not None else "—" -def delta_cell(base, current): +def significant_pct(base, current, floor): + """Signed % change if significant (positive = regression), else None.""" + if base is None or current is None or base == 0: + return None + diff = current - base + pct = diff / base * 100 + if abs(pct) >= THRESHOLD_PCT and abs(diff) >= floor: + return pct + return None + + +def delta_cell(base, current, floor): if base is None or current is None: return "—" diff = current - base if base == 0: return "—" if diff == 0 else f"+{diff:.2f}" - pct = diff / base * 100 - arrow = "🔺" if pct > 25 else ("🔻" if pct < -25 else "") - return f"{pct:+.0f}% {arrow}".strip() + sig = significant_pct(base, current, floor) + arrow = "" if sig is None else ("🔺" if sig > 0 else "🔻") + return f"{diff / base * 100:+.0f}% {arrow}".strip() def cmd_report(args): @@ -319,10 +335,11 @@ def cmd_report(args): else: lines += ["| State | Metric | Value |", "|---|---|---:|"] + regressions, improvements = [], [] for state in STATES: cur_state = current["states"].get(state, {}) base_state = (baseline or {}).get("states", {}).get(state, {}) - for key, label, pattern in METRICS: + for key, label, pattern, floor in METRICS: cur = cur_state.get(key) if cur is None and (not baseline or base_state.get(key) is None): continue @@ -330,22 +347,64 @@ def cmd_report(args): base = base_state.get(key) lines.append( f"| {state} | {label} | {fmt(base, pattern)} " - f"| {fmt(cur, pattern)} | {delta_cell(base, cur)} |" + f"| {fmt(cur, pattern)} | {delta_cell(base, cur, floor)} |" ) + sig = significant_pct(base, cur, floor) + if sig is not None: + bucket = regressions if sig > 0 else improvements + bucket.append({ + "state": state, + "metric": label, + "base": base, + "current": cur, + "pct": round(sig), + "pattern": pattern, + }) else: lines.append(f"| {state} | {label} | {fmt(cur, pattern)} |") + for entries, label_name, verdict_line in ( + (regressions, "benchmark-regression", "regressed"), + (improvements, "benchmark-improvement", "improved"), + ): + if not entries: + continue + lines += [ + "", + f"### {'⚠️' if verdict_line == 'regressed' else '🎉'} Labeled `{label_name}`", + "", + f"This PR is labeled `{label_name}` because these metrics {verdict_line} " + f"by ≥{THRESHOLD_PCT}% vs {base_ref} (beyond each metric's absolute noise floor):", + "", + ] + lines += [ + f"- **{e['state']} — {e['metric']}**: " + f"{fmt(e['base'], e['pattern'])} → {fmt(e['current'], e['pattern'])} ({e['pct']:+d}%)" + for e in entries + ] + lines += [ "", f"_{current['seconds_per_state']}s sampling window per state; CPU % is the " "process CPU-time delta over the window. Runs land on different shared " - "runners, so treat small deltas as noise — 🔺/🔻 marks ±25%._", + f"runners, so treat small deltas as noise — 🔺/🔻 marks changes ≥{THRESHOLD_PCT}% " + "that also clear the metric's absolute noise floor, and those add the " + "`benchmark-regression` / `benchmark-improvement` label._", ] if baseline is None: lines.append("") lines.append("_No main-branch baseline found; showing absolute values only._") print("\n".join(lines)) + if args.verdict: + with open(args.verdict, "w") as f: + json.dump({ + "regression": bool(regressions), + "improvement": bool(improvements), + "regressions": regressions, + "improvements": improvements, + }, f, indent=2) + def main(): parser = argparse.ArgumentParser(description=__doc__) @@ -362,6 +421,7 @@ def main(): report = sub.add_parser("report", help="render results as markdown") report.add_argument("results", help="results JSON from `run`") report.add_argument("--baseline", help="baseline results JSON to compare against") + report.add_argument("--verdict", help="also write a labeling verdict JSON to this path") report.set_defaults(func=cmd_report) args = parser.parse_args() From a857977ace1cb7f6576f291892494e65a6a4c48f Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 18:45:27 +0900 Subject: [PATCH 4/5] Use benchmark: label prefix to match the area: label convention --- .github/workflows/benchmark.yml | 14 +++++++------- AGENTS.md | 2 +- scripts/benchmark.py | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index fe214df..803549e 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -44,7 +44,7 @@ jobs: contents: read actions: read # download the baseline artifact from a main run pull-requests: write # upsert the comparison comment - issues: write # create/apply the benchmark-regression/-improvement labels + issues: write # create/apply the benchmark:regression / benchmark:improvement labels steps: - name: Checkout uses: actions/checkout@v7 @@ -108,21 +108,21 @@ jobs: improvement=$(jq -r '.improvement' "$verdict" 2>/dev/null || echo false) repo='${{ github.repository }}' pr='${{ github.event.pull_request.number }}' - gh label create benchmark-regression --repo "$repo" --force \ + gh label create benchmark:regression --repo "$repo" --force \ --color D93F0B --description "CI benchmark: significant resource regression vs main" - gh label create benchmark-improvement --repo "$repo" --force \ + gh label create benchmark:improvement --repo "$repo" --force \ --color 0E8A16 --description "CI benchmark: significant resource improvement vs main" # Keep labels in sync with the latest run: apply when the verdict # says so, remove a stale one when a newer push clears it. if [ "$regression" = "true" ]; then - gh pr edit "$pr" --repo "$repo" --add-label benchmark-regression + gh pr edit "$pr" --repo "$repo" --add-label benchmark:regression else - gh pr edit "$pr" --repo "$repo" --remove-label benchmark-regression || true + gh pr edit "$pr" --repo "$repo" --remove-label benchmark:regression || true fi if [ "$improvement" = "true" ]; then - gh pr edit "$pr" --repo "$repo" --add-label benchmark-improvement + gh pr edit "$pr" --repo "$repo" --add-label benchmark:improvement else - gh pr edit "$pr" --repo "$repo" --remove-label benchmark-improvement || true + gh pr edit "$pr" --repo "$repo" --remove-label benchmark:improvement || true fi - name: Comment on PR diff --git a/AGENTS.md b/AGENTS.md index 8e81ec9..e74b2d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ Tag-pushed builds release via `.github/workflows/release.yml`, which needs repo ## Benchmarks -`.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — a delta is flagged only when it clears both ±25% _and_ the metric's absolute noise floor (`METRICS` in `benchmark.py`). Significant deltas add the `benchmark-regression` / `benchmark-improvement` PR label (removed again when a later push clears them) and the comment explains which metrics triggered it. +`.github/workflows/benchmark.yml` measures resource usage (CPU-time delta, RSS, and — best-effort via `powermetrics` — wakeups/s) across three window states: focused, open-but-unfocused, and minimized. Pushes to `main` upload a `benchmark-results` artifact that serves as the baseline; PR runs download the latest main baseline and post a comparison table to the job summary and a single updated-in-place PR comment. Runs land on different shared runners, so cross-run deltas are noisy — a delta is flagged only when it clears both ±25% _and_ the metric's absolute noise floor (`METRICS` in `benchmark.py`). Significant deltas add the `benchmark:regression` / `benchmark:improvement` PR label (removed again when a later push clears them) and the comment explains which metrics triggered it. The harness (`scripts/benchmark.py`, invoked by `mise run bench`) launches the Release app under a throwaway `$HOME` (hermetic: no user ghostty config, fresh App Support) with `MACTERM_BENCHMARK=1`, which arms `BenchmarkControl` — Darwin-notification remote control (`notifyutil -p com.thdxg.macterm.bench.`; commands: `open-project`, `activate`, `minimize`, `restore`). Darwin notifications and `open`-based activation need no TCC grants, which is what lets a headless CI runner script window states at all (AppleScript/synthetic keys would need an Accessibility grant). Benchmark mode also skips the notification-permission prompt (it would steal key focus mid-measurement) and doesn't start the Sparkle updater — a bench-built Release app carries the placeholder EdDSA key, so Sparkle's failed-start alert would otherwise block the launch run loop until someone clicks OK. diff --git a/scripts/benchmark.py b/scripts/benchmark.py index 4317070..a7f34af 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -364,8 +364,8 @@ def cmd_report(args): lines.append(f"| {state} | {label} | {fmt(cur, pattern)} |") for entries, label_name, verdict_line in ( - (regressions, "benchmark-regression", "regressed"), - (improvements, "benchmark-improvement", "improved"), + (regressions, "benchmark:regression", "regressed"), + (improvements, "benchmark:improvement", "improved"), ): if not entries: continue @@ -389,7 +389,7 @@ def cmd_report(args): "process CPU-time delta over the window. Runs land on different shared " f"runners, so treat small deltas as noise — 🔺/🔻 marks changes ≥{THRESHOLD_PCT}% " "that also clear the metric's absolute noise floor, and those add the " - "`benchmark-regression` / `benchmark-improvement` label._", + "`benchmark:regression` / `benchmark:improvement` label._", ] if baseline is None: lines.append("") From 37c7fb8e59a59a4a68575bcd8b1c8d52182b9ba8 Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 18:53:02 +0900 Subject: [PATCH 5/5] Only label the first row of each state group in the benchmark table --- scripts/benchmark.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/benchmark.py b/scripts/benchmark.py index a7f34af..aee1e85 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -339,6 +339,7 @@ def cmd_report(args): for state in STATES: cur_state = current["states"].get(state, {}) base_state = (baseline or {}).get("states", {}).get(state, {}) + state_cell = state # only label the state's first row for key, label, pattern, floor in METRICS: cur = cur_state.get(key) if cur is None and (not baseline or base_state.get(key) is None): @@ -346,7 +347,7 @@ def cmd_report(args): if baseline: base = base_state.get(key) lines.append( - f"| {state} | {label} | {fmt(base, pattern)} " + f"| {state_cell} | {label} | {fmt(base, pattern)} " f"| {fmt(cur, pattern)} | {delta_cell(base, cur, floor)} |" ) sig = significant_pct(base, cur, floor) @@ -361,7 +362,8 @@ def cmd_report(args): "pattern": pattern, }) else: - lines.append(f"| {state} | {label} | {fmt(cur, pattern)} |") + lines.append(f"| {state_cell} | {label} | {fmt(cur, pattern)} |") + state_cell = "" for entries, label_name, verdict_line in ( (regressions, "benchmark:regression", "regressed"),