The OSD stage line stays a partition — an async decode figure is not one of its terms #211
Merged
enricobuehler
merged 1 commits from 2026-08-13 21:56:47 +00:00
worktree-decode-stat-overlap into main
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
81022bcc80 |
fix(client/stats): keep the stage line a partition — an async decode figure is not one of its terms
ci / web (pull_request) Successful in 2m11s
ci / bun-nix (pull_request) Successful in 3m25s
ci / rust-arm64 (pull_request) Successful in 4m39s
android / android (pull_request) Successful in 6m20s
ci / docs-site (pull_request) Successful in 5m37s
ci / rust (pull_request) Successful in 6m40s
windows-client / client (arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (pull_request) Successful in 2m49s
windows-client / client (x64, , x86_64-pc-windows-msvc, C:\t) (pull_request) Successful in 6m34s
A 2026-08-13 field report read the OSD's stage line as a breakdown of e2e and asked why the parts did not add up: `host 5.4 · net 0.3 · decode 6.6 · display 1.4` against `e2e 8.1/9.1`. Fair question, and the numbers are all individually true. They add up without `decode`: 5.4 + 0.3 + 1.4 ≈ 8.1. The stages ARE a per-frame partition of e2e — pts →(host+net)→ received →(decode)→ decoded →(display)→ displayed — and that holds for as long as the `decoded` stamp is a COMPLETION stamp. On the synchronous rungs it is. On the native-Vulkan rung `receive_frame` returns at SUBMISSION (~0.1 ms) and the stamp shipped to the presenter is taken there, so `display` is measured from submit and the GPU decode happens INSIDE it. `host+net` and `display` already tile e2e between them; the `decode` figure, measured received → fence-complete, re-counts the GPU work `display` contains. Two figures, one overlap, printed side by side as though they tiled. So on that rung `decode` leaves the stage line and gets its own, carrying the two caveats a reader needs before the number means anything: it is ONE sample per window there, not the p50 every other figure on that line is, and it is already inside `display` so adding it double-counts. The synchronous rungs are untouched — `decode` is a real term there and stays inline. Deliberately NOT changed: the one-sample-per-window design. `pf_client_core:: session` argues it at length — a per-frame fence wait serialises the decode pipeline (an APU's 19 ms decode capping a 5120×1440 stream at ~51 fps), and M4 already re-examined and rejected polling, which quantises every sample up by a frame interval (8.3 ms at 120 Hz against decodes of ~0.1-2 ms). That reasoning still holds; the reporting around it was the defect. Making `decode` a genuine per-frame term would need a completion stamp off the hot path — a waiter thread on the timeline, which that comment already names as the remaining option — and is a bigger change than this one. Also not answered here: why the sampled frame read 6.6 ms when the sampling comment expects 0.1-2 ms. It is a tail frame by construction (a frame that took 6.6 ms to decode also took ≥ 6.6 ms to display, against a 1.4 ms display p50), but whether the first frame of a window is SYSTEMATICALLY a tail frame needs instrumenting rather than guessing. Verified in the linux/amd64 container: pf-presenter 47/47 (incl. the new case, which pins both shapes and the timed-out-window zero), pf-client-core 188/188, `clippy --all-targets -D warnings` clean on both, fmt clean. The pf-client-core leg was proven non-vacuous with a planted compile_error! first. |