Repository navigation
feat(vk): NVIDIA Reflex: each queue submit tagged with the game frame its GPU work belongs to - #2499
Conversation
… its GPU work belongs to Reflex's own timings read 243 to 442 ms "to the GPU's end" under DLSS frame generation, against ~21 ms from simulation start to present end (an RTX 5080 at 4x, NotRexed#2118's review). Hookshot chained no VkLatencySubmissionPresentIdNV to its submits, so the driver guessed each submit's frame; with frame generation the display thread's copies and DLSS-G's work sit between the game's frames. reflex_submit_id(next, frame) chains one to a VkSubmitInfo2 (the struct extends it): the game's rendering (submit_recording, the segment jobs) the frame being made; a batch the frame of its first command buffer (kept in pend.reflex when the batch starts); present_image's copy the frame it shows (reflex_present_frame: the set's game frame, none for a made frame, the frame being made without frame generation). No tag when Reflex is off, the frame is not marked, or it was presented already. Each tag is taken right before its vkQueueSubmit2, after any flush (which tags its own batch). RX_VK_REFLEX_SUBMIT_IDS=0 leaves them off, for an A/B. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Joey-Magnus
left a comment
There was a problem hiding this comment.
The tagging looks right. Each tag is taken under dev.present_mu right before its vkQueueSubmit2, after any flush (which tags its own batch), so nothing can overwrite the thread_local struct in between. Taking dev.present_mu before r.mu is the order reflex_submit_ends() already uses. Made frames and ids already presented get no tag, and the A/B numbers are convincing. On other GPUs the cost is one uncontended lock per submit, which is fine.
One rule issue: gpu/vk/vk_reflex.cpp:245 reads RX_VK_REFLEX_SUBMIT_IDS with atoi(...) == 0. Since #2042 the convention is env_value_off/env_on from compat/hookshot_env.h. With atoi, =true or =on turns the tags off. const char* e = getenv(...); off = e && env_value_off(e); would fix it. Please also add the switch to the Reflex row in docs/development.md:170.
After that it's ready from my side. A batch that spans two game frames is credited to the first frame's id. That seems an acceptable approximation, but the comment should say so.
|
Ran the input-to-screen test on ca483d8 with the same PC, method and spot as on #2118: RTX 5080, 360 Hz G-SYNC, DLAA, DLSS-G 4x, Reflex On + Boost,
Reflex's own timing over the runs:
One thing that isn't this PR: compared with my #2118 run on 3a39e3f, this build's game frame rate is lower in roughly the same spot (about 68 vs 77 fps with DLSS-G, and 87 vs 97 with it off). The sword and walk are also slower in every mode. Main moved about 100 commits between the two, so that's somewhere in main, not the tags. |
hckrman101
left a comment
There was a problem hiding this comment.
Nice, and the numbers back it up: your 5090 run and @nullifyshot's input-to-screen run both show the GPU-end timing going from hundreds of ms to a few ms past present end, with a likely 6–7 ms gain on sword/walk.
I checked the four sites. Each tag is taken under dev.present_mu right before its vkQueueSubmit2, so the thread_local struct can't be overwritten in between. present_image flushes the batch (tagged with its own frame) before tagging its copy. The lock order (present_mu then r.mu) matches the existing callers. Made frames, unmarked frames and ids already presented get no tag. VkLatencySubmissionPresentIdNV is allowed on VkSubmitInfo2, and on() covers the extension being enabled. The test stand-ins match the new signature. CI is green.
I agree with @Joey-Magnus's review and won't repeat it. It's still open at this head: RX_VK_REFLEX_SUBMIT_IDS should use env_value_off from gpu/vk/compat/hookshot_env.h rather than atoi(...) == 0, the switch needs adding to the Reflex row in docs/development.md, and the comment should say a batch is credited to its first frame. Once those are in, this is good to go from my side.
One small addition to that last point, inline.
| std::lock_guard lk(dev.present_mu); // (one queue: submits and presents take turns) | ||
| if (batch_on()) { | ||
| if (pend.cbs.empty()) batch_next++, pend_since = std::chrono::steady_clock::now(); | ||
| if (pend.cbs.empty()) batch_next++, pend_since = std::chrono::steady_clock::now(), pend.reflex = reflex_frame(); |
There was a problem hiding this comment.
Nit, on top of Joey's note: the batch's frame is read only when it starts. So a batch started before the frame's sleep returned (reflex_frame() = 0) stays untagged, even when the rest of its command buffers belong to the new frame. That's harmless, since batching is off by default outside RADV, so on NVIDIA this only matters with RX_VK_BATCH=1. It's worth a word in the same comment, though.
There was a problem hiding this comment.
Done in fa75d05: the comment at the batch start says it now. Thanks.
|
This now conflicts with main in |
# Conflicts: # gpu/vk/vk_reflex.cpp
…ches' frame said Review notes (Joey-Magnus, hckrman101): - RX_VK_REFLEX_SUBMIT_IDS is read with env_value_off (hookshot_env.h, the switches' rule since NotRexed#2042) instead of atoi(...) == 0, which took =on or =true as off. - vk_reflex.cpp and the batch start in vk_replay.cpp say how a batch is credited: to the frame of its first command buffer, read when the batch starts, so one spanning two game frames counts as the first's and one begun before the frame's sleep returned goes untagged. Batching is on by default only with RADV, which has no Reflex. - docs/development.md: the switch in the Reflex row. - test_reflex_swapchain_lifetime.py runs its fixture with RX_VK_REFLEX_SUBMIT_IDS=on (tags on; the atoi reading fails it, check 21) and with =0 (untagged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks both, and thanks @nullifyshot for the input-to-screen run. Updated: main merged first (8678b65). #2325's
On the frame-rate drop between the two builds (about 68 vs 77 fps with DLSS-G, 87 vs 97 without): agreed, that's somewhere in main, not here. It's next on my list for the frame-rate work. |
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, all three of @Joey-Magnus's asks are in, plus my nit:
RX_VK_REFLEX_SUBMIT_IDSis read withenv_value_offfromhookshot_env.h. The test now runs with=on(which the oldatoireading would have turned off) and=0.- The switch is in the Reflex row of
docs/development.md. - The header of
vk_reflex.cppand the batch start invk_replay.cppboth say a batch is credited to its first command buffer's frame, and that one started before the frame is marked stays untagged.
The merge is clean. The PR is 0 behind main. Against main, vk_reflex.cpp has only additions: the header comment, the include and reflex_submit_id, sitting next to #2325's reflex_present_id_features. None of main's lines were dropped. vk_replay.cpp differs from main only at the four tag sites and pend.reflex. With #2325's presenter thread, the copy's submit and its tag stay in present_image on the game's thread. A deferred present carries reflex_present_frame with it, so the tag still matches the frame that's shown.
CI was still pending at this head when I looked. The test_env_on failure you mention comes from main's #2470, not from this PR. Good to merge once CI is green.
Follows @nullifyshot's lead on #2118 (comment). Under DLSS frame generation, Reflex's own "to the GPU's end" read 243–442 ms there (RTX 5080, 4x), next to ~21 ms from simulation start to present end. Hookshot chained no
VkLatencySubmissionPresentIdNVto its queue submits, so the driver had to guess which frame each submit's GPU work belongs to. Without frame generation it guesses right. With it, the display thread's copies and DLSS-G's work sit between the game's frames and get counted as the next game frame's rendering.What changes
reflex_submit_id(next, frame)(vk_reflex.cpp) chains aVkLatencySubmissionPresentIdNVto aVkSubmitInfo2(vk.xml:structextends="VkSubmitInfo,VkSubmitInfo2"). It is used at the four submit sites in vk_replay.cpp:submit_recording's direct submit and the parallel segment jobs' submit: the frame being made.queue_flush_locked): the frame of the batch's first command buffer, kept inpend.reflexwhen the batch starts.present_image's copy: the frame it shows, by the same rule as the present id: the set's game frame (reflex_present_frame), no tag for a made frame, and the frame being made without frame generation.There is no tag when Reflex is off, when no frame is marked, or when that present id was presented already (present ids only go up). Each tag is taken right before its
vkQueueSubmit2, after any flush, and that flush tags its own batch.RX_VK_REFLEX_SUBMIT_IDS=0leaves the tags off, for an A/B comparison.In game: RTX 5090, 5120x2160 swapchain, DLAA and DLSS-G 3x at 4416x1864 (600 of 600 presents made by DLSS-G), V-Sync Auto, Reflex On + Boost,
RX_VK_REFLEX_LOG=1, Kokiri Forest. Same build both runs; run 2 only addsRX_VK_REFLEX_SUBMIT_IDS=0.RX_VK_REFLEX_SUBMIT_IDS=0)With the tags, "to the GPU's end" lands a few ms after present end, the same pattern as without frame generation (12.1 vs 10.1 ms in nullifyshot's run), so the driver now paces on real GPU timings. The tags-off run also felt worse to play. Its simulation-to-present numbers are a bit higher too, but it was a short run (5 reports), so I wouldn't read much into that gap yet.
What this doesn't measure: input to screen. @nullifyshot, if you have time, your
HOS_INPUT_LIVEmethod on this head would show what it's actually worth. The ~⅔ of a game frame that interpolation holds the real frame back stays as it was; Reflex can't remove that.Tests:
test_reflex_swapchain_lifetime.pycopiesreflex_submit_idand gains 8 checks (the frame being made, a kept frame, frame 0, no frame marked, one already presented, Reflex off, retired before the lock).draw_stream_visibility_test.cpp,fsr_image_retirement_test.cppandparallel_init_order_test.cppget stand-ins.tests/run_all.py(Windows, llvm-mingw clang++): 391 passed, 31 skipped, 1 failed:test_vram_budget_row, which reads onlyvk_menu.cpp,vk_menu.handvk_memory.cpp(not touched here) and fails the same way on main.🤖 Generated with Claude Code