Skip to content

chore(diagnostics): add canvas and window stall profiling - #412

Closed
DeadWaveWave wants to merge 1 commit into
mainfrom
codex/issue-407
Closed

chore(diagnostics): add canvas and window stall profiling#412
DeadWaveWave wants to merge 1 commit into
mainfrom
codex/issue-407

Conversation

@DeadWaveWave

@DeadWaveWave DeadWaveWave commented Sep 9, 2026

Copy link
Copy Markdown
Owner

💡 Change Scope

  • Small Change: Fast feedback, localized UI/logic, low-risk.
  • Large Change: New feature, cross-boundary logic, runtime-risk (persistence, IPC, lifecycle, recovery).

This adds opt-in diagnostic tooling that coordinates Main/renderer instrumentation
and capture lifecycle. It does not change product runtime behavior.

📝 What Does This PR Do?

Refs #407

Canvas panning and desktop app-switch stalls need evidence spanning renderer
JavaScript, Main scheduling, and Chromium raster/compositor activity. This PR
adds pnpm profile:canvas:window-stall, shared probe/summary helpers, four unit
tests, and a documented diagnosis workflow. It reuses the existing isolated
workspace and synthetic terminal fixture helpers.

The runner supports idle, automated pan, and manual capture; records bounded
frame, long-task, focus, Main timer, and process CPU observations; and exports
Chromium trace and renderer CPU profile artifacts. It preserves foreground gaps
above one second, separates background intervals, verifies a changed pan viewport,
and reports missing observations, sample loss, trace-buffer usage, and errors.

Dated findings: 2026-09-10

Eight existing macOS captures cover two repetitions of idle/pan at 0/8 terminals.
Each was configured for 10 seconds with tracing enabled. All eight reports have
no capture/cleanup errors, no lost samples, no recorded long JavaScript tasks,
and trace buffers below capacity. All four automated pan runs changed viewport.
Frame p95 is 17.4-17.9 ms across the eight runs.

Scenario Terminals Rep 1 frame max (ms) Rep 2 frame max (ms)
idle 0 17.8 18.7
pan 0 67.7 35.2
idle 8 18.6 18.6
pan 8 133.5 18.6

The 67.7 ms gap overlaps a 96.482 ms RendererRasterWorker slice and its
96.242 ms RasterDecoderImpl::DoEndRasterCHROMIUM::Flush slice. The 133.5 ms gap
is spanned by a 178.309 ms worker slice and its 177.370 ms flush slice (the
parent DoEndRasterCHROMIUM duration is 177.399 ms). These inclusive durations
are not additive. The 35.2 ms gap has no overlapping complete slice of those
raster/flush names or DisplayScheduler::DrawAndSwap in the GPU process.

GPU raster/compositor delay is a candidate mechanism for the two largest captured
gaps, not an established root cause. The real user's approximately one-second
cross-application stall was not reproduced; Issue #407 remains unresolved.

The host had concurrent desktop load (capture-session observations: Codex Renderer
approximately 80% CPU, ChatGPT Renderer approximately 54% CPU, and another installed
OpenCove process running). There are only two repetitions per cell, below the
recommended minimum of three, and no TRACE=0 overhead control. The recommended
1- and 10-terminal cells were not captured. The fixture lacks the real user's
layout/workload, manual app switching, installed-release parity, native desktop
profiling, and another application's presentation-latency evidence. These limits
prevent a root-cause, clean-baseline, or fix claim.

The full local report is
artifacts/canvas-window-stall-profile/FINDINGS-2026-09-10.md, with the eight-row
matrix, exact trace timestamps/PID/TID, marker alignment, and all three gap windows.
It is ignored and is not a remotely accessible PR attachment. Raw traces,
profiles, logs, and screenshots remain local; this PR contains reusable assets only.


🏗️ Large Change Spec (Required if "Large Change" is checked)

1. Context & Business Logic

Following the Electron performance/contentTracing and Chromium CPU Profiler
references documented in docs/development/CANVAS_WINDOW_PROFILING.md, correlate
multiple processes before assigning a bottleneck. A renderer CPU profile cannot
measure WindowServer or another application's first presented frame.

Acceptance for this PR is a reusable isolated diagnostic runner, bounded exported
observations with health metadata, tests of evidence handling, and a repeatable
manual/native follow-up method. Automated pan is not app-switch reproduction.
Shared Electron capture is intended for macOS/Windows/Linux; actual captures in
this change are macOS-only, and Windows/Linux runtime validation is outstanding.

2. State Ownership & Invariants

Existing fixture helpers and runtime APIs own disposable workspace/PTYS; the
renderer interaction owner handles synthetic mouse input. Injected probes own
temporary bounded observations, Main coordinates Electron tracing, and Chromium
owns the renderer CPU profiler. Exported files are local observations, not durable
product state. Existing product IPC/security/persistence contracts are unchanged;
there is no executable architecture-rule impact.

  1. Fixture writes target isolated temporary userData; viewport changes use the
    existing input path. No production state migration or tuning is introduced.
  2. Foreground intervals above one second are retained; focus/visibility transitions
    separate suspended intervals. Empty samples remain unavailable, not zero cost.
  3. Errors, sample loss, full buffers, and unchanged pan viewport invalidate evidence.
    Probe listeners/timers/observers and capture resources have cleanup paths; a
    successful process exit alone is not sufficient proof of evidence completeness.

Top risks are instrumentation overhead, host load, cross-clock interpretation,
partial capture/cleanup failure, and synthetic fixture parity. Windows home/cache
redirection is explicitly documented and is not claimed to be hermetic shell
isolation. Raw local artifacts can include paths/content and need review before
sharing. No performance threshold is used as a cross-machine merge gate.

3. Verification Plan & Regression Layer

The lowest meaningful layer is pure summary/config unit tests plus fresh-VM probe
serialization tests. The four tests cover configuration bounds, retained >1-second
frames, background separation, missing/lost observations, closure independence,
and normal timer/listener cleanup. Existing eight fixture captures demonstrate
changed pan viewport, nonempty observations/trace/profile, and clean reported
capture completion on macOS.

Validation already completed: frozen offline dependency install and build;
four targeted tests; targeted Oxlint and Prettier; script syntax and diff checks.
Final staged review confirmed exactly eight reusable asset files; staged line
check and the four targeted tests passed again. The final
OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit completed successfully on the final
eight-file staged set: 4 related unit tests, 8 native recovery tests, and
322 Electron E2E tests passed, with 79 platform/manual skips (22.9 minutes).
Required E2E used zero retries and no crash fallback. No source changes occurred
during that gate, and no additional profiling samples were collected.

CI first attempt on 0ec82519: all jobs passed except
macOS shard 2,
which reported 116 passed, 16 skipped, and one existing terminal drag test failure
at workspace-canvas.selection.terminal-drag-selected-body.spec.ts:157:
the immediate persisted-position read returned x=220, expected >340. The trace
shows selection/membrane checks passing and ordered mouse movement from
(684.2, 356.6) to (944.2, 576.6), followed immediately by the persistence read.
The failure screenshot shows the selected windows visually displaced, so the
assertion alone does not establish that dragging failed; persistence timing is
a candidate explanation, not a confirmed root cause. No product code changes
are included in this PR. The first failure artifacts remain available in the
linked job. Only that failed CI job was rerun once, without source changes.
Attempt 2 macOS shard 2
passed: 117 E2E tests passed, 16 skipped (11.2 minutes); the previously failed
terminal drag test passed in 4.6 seconds. All current PR checks are now green on
0ec8251907fecec8a5df32a3892c7390ab8af59b, including macOS, Ubuntu, Windows,
Web continuity, PR template, and CLA. The first-attempt failure remains recorded
above; the rerun does not establish its cause or resolve Issue #407.

Residual test gaps: failure-path cleanup, the supported Long Tasks observer branch,
and blur/focus round-trips between rAF callbacks are not directly exercised by the
four unit tests. Unit tests do not establish real-user/app-switch performance.


✅ Delivery & Compliance Checklist

  • My code passes the ultimate gatekeeper: pnpm pre-commit is completely green.
  • I have signed the CLA if required (see CLA.md).
  • I have included new tests to lock down the behavior (or explicitly stated why it's untestable).
  • I have strictly adhered to the DEVELOPMENT.md architectural boundaries.
  • I have attached a screenshot or screen recording (if this touches the UI).
  • I have updated the documentation accordingly (if adding a feature or changing a contract).

The full local gate passed on the final staged asset set. CLA signing status has
not been verified.
UI media is not applicable: there is no product UI change. No changelog entry
claims a user-visible performance fix.

📸 Screenshots / Visual Evidence

No screenshots or recordings are committed or uploaded in this diagnostic PR.
Local before/after fixture screenshots are diagnostic artifacts, not maintained
visual regression baselines or proof of app-switch latency. The dated findings
summary above provides the reviewable evidence and its limits.

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