Repository navigation
fleet/web mock: skip whole frames when behind, the same ones for every stream - #1346
Conversation
…y stream main's Fleet Web job failed "switching streams" on 47ec5fc: after the switch back to direct, ui.shownMode stayed "aggregated" for 20 s; the same commit's previous run, and #1332's runs, show the same intermittent failure. Measured on the CI runner with the switch logged every 0.5 s (run 40365237969): during the switch the mock serves the 3 aggregates and the 15 direct tiles, 18 streams and about 1.75 times the pixels of direct alone. It fell behind (about 60 renders queued) and every stream got about 6 of its 10 frames a second, but each stream a different 6: each lane skipped a frame when its own previous draw was still in flight. The switch needs one t that all 15 direct tiles hold, and with each missing 4 frames in 10 independently, no such t came in 22 s. The two-viewers timeout fixed by #1339 (11 of 15 tiles synced) is the same mechanism: a tile without the common t is drawn behind, not synced. The renderer now skips a frame for every stream at once when the last frame's draws have not all started (the pool's backlog). Behind, it serves fewer frames, and all streams hold the same t. With this change the same runner switched back to direct within 0.6 s in each of 13 switches (run 40366842617), passed all 49 Chromium tests, and the mock used 1.1 to 2.1 cores of its 3. - mock/test/mock.test.ts: one render thread asked for eight 360-high tiles at 30 fps; every frame number it sends reaches all eight streams. On the previous renderer: 0 of 71 frame numbers reached all eight. - ci.yml: the Fleet Web job's comment says what a switch costs and how the mock behaves when behind.
|
@codex review |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
main's Fleet Web job failed "switching streams" on 47ec5fc (
wall.spec.ts:586: after the switch back to direct,ui.shownModestayed "aggregated" for 20 s). The same test failed on #1332's runs. This fixes the mechanism in the mock renderer.Cause, measured on the CI runner. I ran the Fleet Web job on a throwaway branch with a CPU sampler and the switch logged every 0.5 s (run 40365237969, 4 vCPU EPYC 7763):
shown=aggregatedthroughout, every tile streaming).Change. When the last frame's draws have not all started (the render pool's backlog), the renderer skips the frame for every stream at once. Behind, it serves fewer frames, and every stream holds the same t, as a page drawing all tiles at one t needs.
Evidence
mock renderer, more streams than it can draw(one render thread, eight 360-high tiles at 30 fps): on main's renderer 0 of 71 frame numbers reached all eight streams; with the change every one does.Received: "aggregated"). This renderer: 3 of 3 passed all 56 tests.pnpm lint && pnpm typecheck && pnpm test(225) andpnpm test:e2e(56 passed).ci.yml: the Fleet Web job's comment now says what a switch costs and how the mock behaves when behind.Downstream impact. Test infrastructure only (the mock fleet). The page is unchanged.