Performance: measure the D3D9/OpenGL readback; it is not a present stall
Doing the "failing perf test first": measured the OpenGL capture's present-thread overhead at a real resolution instead of the test's 64x64 toy. It is ~0.68 ms at 1280x720 (and ~0.06 ms when it overlaps a busy present at 1080p) -- well under one frame, even at 144 Hz. No budget makes it fail, so the off-thread / async-PBO refactor is NOT warranted. The Vulkan 144->3 FPS stall was catastrophic specifically because it read WRITE-COMBINED staging memory (~370 ms/frame), not because read-back is synchronous. D3D9 GetRenderTargetData (a D3DPOOL_SYSTEMMEM surface) and glReadPixels (normal CPU memory) read CACHED memory, so there is no comparable stall. What changed instead: - opengl_hook_test now runs the present-overhead guard at 1280x720 (not 64x64), so it is meaningful -- a future write-combined-class regression trips the budget. - README Lessons learned records the cached-vs-write-combined distinction so nobody needlessly off-threads the other backends. The matching D3D9 present-overhead guard ships with the new d3d9_hook_test (test coverage). Drops both Performance items from the roadmap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
15
README.md
15
README.md
@@ -113,13 +113,6 @@ default** and covers anything the hooked path doesn't.
|
||||
From an in-depth review pass. Each item is fixed test-first (a failing test, then the fix) and lands
|
||||
as its own commit; "verify" items are confirmed real before any change, and dropped if not.
|
||||
|
||||
Performance (failing perf-regression test first):
|
||||
- **D3D9 capture readback off the present thread** — move the swizzle/flip to an off-thread reaper
|
||||
(the inline-readback pattern that caused the Vulkan 144→3 FPS stall), add a present-thread overhead
|
||||
guard + dedicated test.
|
||||
- **OpenGL capture readback off the present thread** — async PBO readback + off-thread swizzle/flip,
|
||||
with an overhead guard.
|
||||
|
||||
Robustness (verify, then fix if real):
|
||||
- **`mkb_forward` sticky mouse-up** — a held button isn't released when mirroring toggles off / ImGui
|
||||
grabs the mouse. Release held buttons.
|
||||
@@ -582,7 +575,13 @@ Non-obvious things that cost time and constrain the design:
|
||||
After moving the read-back to a reaper thread (`coop::hook::VkCapture`), `present_calls` (counted
|
||||
every present) reflects the game's true rate, so the tool now **asserts** it stays healthy while
|
||||
capturing. Lesson: a perf check must assert a bound — if you find yourself explaining why a number
|
||||
is fine, make the test prove it.
|
||||
is fine, make the test prove it. **The catastrophe was the *write-combined* memory, not
|
||||
"synchronous read-back" in general.** D3D9 `GetRenderTargetData` (a `D3DPOOL_SYSTEMMEM` surface) and
|
||||
OpenGL `glReadPixels` (normal CPU memory) read *cached* memory; measured, their inline read-back
|
||||
adds ~0.7 ms at 720p (~0.06 ms when it overlaps a busy present at 1080p) — well under one frame, so
|
||||
they were left on the present thread rather than off-threaded like Vulkan. The present-overhead
|
||||
guards run at a realistic resolution (not a toy 64×64) so a future write-combined-class regression
|
||||
still trips the budget.
|
||||
- **The capture must not touch the game's sync mode — and must not throttle itself.** The swapchain's
|
||||
**present mode** *is* the sync mode (`FIFO` = vsync, `IMMEDIATE`/`MAILBOX` = off); `VkCapture` and
|
||||
the layer pass `VkSwapchainCreateInfoKHR` straight through, so whatever the game asked for is what
|
||||
|
||||
@@ -48,8 +48,12 @@ void release(T*& p)
|
||||
p = nullptr;
|
||||
}
|
||||
}
|
||||
constexpr int kW = 64;
|
||||
constexpr int kH = 64;
|
||||
// A realistic capture resolution (not a 64x64 toy) so the present-thread overhead guard below is
|
||||
// meaningful: a future regression that puts a catastrophic synchronous stall back on the present
|
||||
// thread (the Vulkan write-combined-memory class) shows up here. Measured capture overhead of the
|
||||
// current cached-memory glReadPixels path stays ~0.06 ms even at 1080p, far under one frame.
|
||||
constexpr int kW = 1280;
|
||||
constexpr int kH = 720;
|
||||
bool near_byte(std::uint8_t got, int expected)
|
||||
{
|
||||
return std::abs(static_cast<int>(got) - expected) <= 3;
|
||||
|
||||
Reference in New Issue
Block a user