Repository navigation
Report pane visibility in prowl list and CPU profiles - #842
Merged
Merged
Conversation
Add pane.visible to `prowl list --json`: true when the surface intersects a visible viewport in an on-screen, unminimized window. Background split panes count; focus is not required. The Canvas viewed-surface check now reuses the same visibility test.
make measure-cpu and make capture-spike now save `prowl list --json` as panes.json and print total, visible, focused, tab, selected-tab, and worktree counts. CPU cost follows the panes on screen, so a profile without that count cannot be compared with another. The steady-state parser also reports AppKit transaction flushes, SwiftUI graph flushes, and Ghostty glyph, row, and hash work as separate symbols. scripts/test-performance-measurement-scripts.sh covers both with fixed command fixtures and runs from make test-scripts.
The socket does not require the app and CLI versions to match, so a newer CLI can talk to an older running app whose panes carry no `visible` field. A non-optional field made text-mode decoding fail, and `prowl list` printed `ok: list` instead of the panes. ListCommandPane.visible is now optional, like `handle` and `agent` beside it; absent reads as unknown rather than not visible. The app still always sends it, and the output schema keeps it required, because the schema describes what the app emits.
After a spike was detected, capture-cpu-spike.sh waited for `prowl
agents` and `prowl list` before starting `sample`. A slow app answer
could let the spike end first, and a stalled request, which had no
timeout, kept sampling from starting at all.
`sample` now starts as soon as the spike is confirmed, and the context
and both CLI queries are recorded while it runs. Each query gives up
after the sample's own duration and records {"ok":false}. The script
test adds a `prowl list` that stalls for 30 seconds and requires the
capture to finish with its sample within 10.
Splitting the flush symbol into stepTransactionFlush and GraphHost.flushTransactions() stopped matching frames named just flushTransactions, so that work disappeared from the report. The parser reports those frames again on their own row. Each row is an independent substring total, so the row excludes GraphHost.flushTransactions() frames, which have a row of their own, and its label says so. The script test adds a bare frame and checks both rows.
ListCommandPane's initializer now takes `visible` without a default, so the one app call site cannot omit it. The mirror command test built a pane without it and no longer compiled.
`prowl agents` and `prowl list` ran one after the other, each with its
own sample-length timeout, so a slow agents answer could push the pane
snapshot past the end of the sample it annotates.
Both queries now start together as soon as the sample does and share
one deadline, the sample's end, with sub-second precision. A query
that misses it records {"ok":false}. The script test stalls `prowl
agents` and requires `prowl list` to answer within the one-second
sample; the previous script answered 1.04 s after the sample started.
The shared deadline was computed after `sample` was launched, so a shell delayed at that point started the countdown late, and a `prowl list` answer that arrived after sampling had ended could still be saved as the pane mix during the spike. The deadline is now taken immediately before the launch, as the launch time plus the sample duration.
The fake `sample` exited at once, and the timing case accepted any `prowl list` answer within a second of its start, so it would pass a snapshot taken after sampling had finished. The fake `sample` now lasts its requested duration in the timing cases and records its start and end, and the saved snapshot must fall between them. A new case answers `prowl list` only once the fake sample has ended and requires the capture to refuse it. The test fails against the sequential queries of bb070d6, and against a deadline computed after the launch when the shell is delayed 0.5 s there.
Computing the CLI deadline before launching `sample` put a Perl process
ahead of the sample, so a busy host could delay sampling past a short
spike. It also ended the deadline before sampling did, because
sample(1) starts sampling only after it attaches, so an answer that
arrived during the sample could be discarded.
Nothing now runs between confirming the spike and launching `sample`.
The CLI queries start after the launch with a generous timeout, and
each marks when its answer arrived. Once the sample has exited, an
answer is kept only if it arrived before sampling ended, taken as the
Date/Time the sample recorded in its own header plus its duration;
otherwise the file records {"ok":false} and the reason. Neither bound
depends on when the shell got to run.
The script test records every stub call with its PID, including
/usr/bin/perl through PERL5OPT, and requires nothing but the spike
detection to run before `sample`. A new case delays the fake sample's
start by 0.5 s and requires an answer 0.6 s into sampling to be kept.
Against 0742c73 the first check reports the deadline's Perl process
running before `sample`, and the second reports the answer discarded.
The window opened at the launch, so while sample(1) was still
attaching, a `prowl list` answer from before sampling began was kept
as the pane mix during the spike.
Both bounds now come from the Date/Time that sample(1) stamps when
sampling begins, which carries milliseconds: an answer is kept only
between that time and that time plus the duration, and is otherwise
recorded as {"ok":false} with "answered before sampling started" or
"answered after sampling ended". The CLI queries wait 0.25 s after the
launch so their answers normally land inside the window; ten real
samples on a loaded host stamped Date/Time 28-36 ms after launch.
Nothing still runs before `sample`.
The script test restores the start bound, read from the same value the
fake writes into its header. With the fake taking 0.5 s to attach, an
answer at the query offset is refused and one 0.6 s into sampling is
kept. Against e21b310 the early answer is kept.
Debug and Release apps share the default CLI socket path, and only one app serves it. A profile of one PID could record the agent and pane mix of another app. Both profilers now verify with lsof that the profiled PID serves the socket the CLI uses, and otherwise record the snapshots as unavailable with the socket and PID.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR continues #834 by @simonheimlicher and keeps its 11 original commits unchanged. Merge it with a merge commit (not rebase) so those commit identities stay in
main.From #834:
prowl list --jsonreportspane.visible: the surface intersects its visible rectangle in a window whoseocclusionStateincludes.visible. The Canvas viewed-surface check uses the same predicate. The CLI decodes a missing field from older apps asniland printsvisible=unknown.make measure-cpuandmake capture-spikesavepanes.jsonbeside each sample and print the pane mix. The spike capture launchessamplefirst and keeps a CLI answer only if it arrived inside the sampled window.measure-agent-detection-cpu.shreportsGraphHost.flushTransactions(),stepTransactionFlush,addGlyph,rebuildRow, andwyhashas separate rows.Added here:
agents.jsonandpanes.jsononly when the profiled PID serves the socket the CLI connects to (PROWL_CLI_SOCKETor the default path, checked withlsof). Debug and Release apps share the default socket path and only one app serves it, so before this change a Debug profile could record the agent and pane mix of the Release app. On a mismatch the files record{"ok":false,"reason":"CLI socket <path> is not served by pid <PID>"}. The spike capture checks after the sample, so nothing runs beforesample. The steady-state profiler skips the queries.origin/mainis merged in.Validation
scripts/test-performance-measurement-scripts.shadds cases where the CLI socket points to another app, the default socket belongs to another PID, and a served override path is kept. The profiler cases failed against the Report pane visibility in prowl list and CPU profiles #834 head scripts and pass now.bash -nandshellcheckon the three scripts: passed.make check,make test-scripts: passed.make build-cli,make test-cli-smoke,make test-cli-unit,make test-cli-integration(106 passed): passed.make build-app: passed, 0 warnings.make test: passed, 3482 app tests with 0 failures.make capture-spikeormake measure-cpuagainst a running Debug app.Continues #834. Thanks to @simonheimlicher for the original work.