feat: add devtools trace capture to web profiling - #180
Conversation
`just web profile --trace` records a Chromium DevTools Performance trace (chromium-trace.json) through Playwright's browser tracing in place of the renderer CPU profile. The trace carries style, layout and paint events, React's performance tracks and denser CPU samples; skipping the separate renderer Profiler avoids stacking a second sampler on the timings. Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking findings at 8ab0b52d51e0ebb8fe96c32a4df6e34836ee9743 against a47ddaf452e605a59056476d2091146e1f3902e8. One optional documentation clarification inline. This is a review comment, not formal approval.
Validation: hosted CI is green, including the trace/cancellation fixtures. An independent synthetic capture using pinned Playwright 1.63.0 and installed Chrome 153 produced JSON with CPU samples, layout and paint events. Two focused failure-injection checks confirmed start/stop failures retain broker/network artifacts, close resources, and report failure. No broad local rerun.
The actual-producer check was headless and used only synthetic local data. I did not repeat the author’s attended full-app/React-track capture or DevTools UI import.
| fragments, and WebSocket frame data are omitted. Use `just web profile --trace` | ||
| to record a Chromium DevTools Performance trace (`chromium-trace.json`, with | ||
| style/layout/paint events, React's performance tracks, and denser CPU samples) | ||
| in place of `chromium-renderer.cpuprofile`; traces are large, so keep traced |
There was a problem hiding this comment.
Non-blocking: distinguish raw trace data from sanitized network output
Consider adding one sentence that chromium-trace.json is a raw performance trace and should be reviewed before sharing. The preceding redaction promise correctly applies to network.json, but the adjacent description makes the distinction easy to miss. With the actual pinned Playwright producer, a synthetic request query marker remained in the trace; user-timing labels also remain. No trace sanitization or recorder redesign is requested, just a short sharing warning.
…o-player-polish * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com>
…-content-compat * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com> # Conflicts: # docs/channels.md
Summary
Adds
just web profile --trace, which records a Chromium DevTools Performance trace tochromium-trace.jsonvia Playwrightbrowser.startTracing(page)/stopTracing()(default timeline categories).--tracereplaces the renderer CDPProfilercapture: the trace already includes V8 CPU samples (~128 µs interval vs 1 ms), and running both stacks two samplers. The broker.cpuprofileand--networkcapture are unchanged.finallybeforebrowser.close(); a failed stop is reported with the other capture failures.coveragereportschromium-traceinstead ofchromium-renderer.Validation
node --test tests/integration/profile-dev.test.mjs: 35/35. The 3 new tests (traced artifacts/manifest; cancel during a pendingstartTracingwith late resolve and late reject) fail onmain.just web profile --network --tracerun (~100 s): trace loads in DevTools with layout/paint events,ProfileChunksamples, and React performance tracks.Traces are large (~47 MB for 10 s, ~286 MB for 100 s), so the docs recommend short sessions.