Run a frame's bookkeeping that is still owed on the present thread before the frame's blit, so the blit never reads the upscaler state of a frame vk_dlaa_frame has not closed.

The review of cd2fd454 found that with present_order = 1 the present thread could record and queue frame N's blit
before frame_end(N) ran: when the presenting thread's try of the submission lock failed, frame_end(N) waited for the
next submission, and that could be the present thread's own blit submission, which present_main makes only after
vk_present_blit. The blit reads what vk_dlaa_frame resets when the upscaler or frame generation is switched on or off
(gfx_present.c hudless_record through vk_dlaa_fg_hudless; vk_fg.cpp, vk_fg4.cpp and vk_dlssg.c through
vk_dlaa_fg_inputs, vk_dlaa_fg_views and vk_dlaa_fg_camera). vk_dlaa_frame drops those inputs at a switch because
handing them to frame generation ended an AMD process before, and in 0.10.1 vk_dlaa_frame(N) always ran before blit
N. So a switch frame inside an overlap window could hand frame generation the inputs the switch should have dropped.

Now present_image tells the present thread when it left the frame owed (PresentJob.owed), and present_main first
makes sure that frame's bookkeeping ran (present_frame_closed, vk_frame_owed.h fo_close): when a submission did it
meanwhile, done says so (it is now stored with release after each frame's bookkeeping and read with acquire, fo_ran)
and no lock is taken; otherwise the present thread takes the submission lock, waiting for a replay in progress
(vk_sync.c vk_sync_locked), and runs what is owed. frame_end(N) therefore always comes before blit N, as in 0.10.1.
The present thread is not one of the game's threads, and it waits for the same lock for the blit's submission a
moment later anyway. It waited for that lock with its busy flag set before too, and nothing that holds the lock waits
for the present thread, so no deadlock is added. The cost, only for a frame still owed when the
present thread takes it: the blit's recording no longer overlaps the replay it waits for, which can lengthen a drawing
thread's existing wait in vk_present_drain by that recording. The common frame, whose present got the lock, takes no
lock here. The log's frame bookkeeping line (exit, and trace = perf every 600 presents) counts these frames and the
time they took the present thread, the wait included.

tests/frame_owed_check now has a present thread that takes each frame from the presenting thread (one in waiting at
most), blits it and submits: no blit comes before its frame's bookkeeping. Its "old" argument leaves the catch-up out
and fails, as the branch had it: 2608 and 2706 blits before their frame's bookkeeping in two runs of 20,000 frames.
docs/building.md, docs/architecture.md and nx2navi.ini.example say where the bookkeeping runs.

Not reproduced before the change. Checks that passed: frame_owed_check (20,000 frames; 12,695 owed, 1,888 of them
caught up by the present thread, 0 blits before their bookkeeping, the longest present 0.003 ms), pass_cc_check of
every runtime file (headers changed), no new -Wall -Wextra warnings in vk_backend.c and vk_sync.c.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
chrg6
2026-11-13 19:42:03 -05:00
parent effdbd8dc0
commit 618cfc01f4
9 changed files with 196 additions and 59 deletions

View File

@@ -1,7 +1,11 @@
// tests/frame_owed_check.py builds this: runtime/vk_frame_owed.h, a frame's bookkeeping in replay order, with one
// presenting thread and three replaying threads that hold the lock for their replays, a few of them for HOLD_MS. Every
// frame's bookkeeping must run once and in frame order, never during a replay, and before the body of any replay that
// took the lock after the frame was presented; the presenting thread must never wait for a replay.
// presenting thread, three replaying threads that hold the lock for their replays, a few of them for HOLD_MS, and a
// present thread that takes each presented frame from the presenting thread (one in waiting at most, as vk_backend.c
// present_image hands it over), blits it and then submits under the lock (vk_backend.c present_main). Every
// frame's bookkeeping must run once and in frame order, never during a replay, before the body of any replay that took
// the lock after the frame was presented, and before the present thread's blit of the frame; the presenting thread
// must never wait for a replay. "frame_owed_check old" leaves out the present thread's fo_close, as the branch first
// had it, to show that the blit check sees a blit before its frame's bookkeeping (it then fails).
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -22,6 +26,10 @@ static FrameOwed FO;
static volatile int32_t in_replay, in_long, stop;
static volatile uint64_t presented, ends, bad_order, during_replay, late, ran_here, long_holds, during_long;
static uint64_t last_end, present_max_ns;
static uint64_t ended; // the newest frame whose bookkeeping returned, kept apart from FO.done (atomic)
static uint64_t slot; // the frame handed to the present thread << 1 | left owed (PresentJob.owed); 0: none (atomic)
static uint64_t blits, blit_early, closed_here; // the present thread's
static int old_mode;
// vk_backend.c frame_end's stand-in, run with the lock held.
static void frame_end(uint64_t frame, void *arg) {
@@ -31,6 +39,7 @@ static void frame_end(uint64_t frame, void *arg) {
last_end = frame;
__atomic_add_fetch(&ends, 1, __ATOMIC_RELAXED);
for (volatile int i = 0; i < 300; i++) {}
__atomic_store_n(&ended, frame, __ATOMIC_RELEASE);
}
// vk_sync.c vk_sync_try_locked with vk_backend.c fo_here
@@ -50,7 +59,7 @@ static void replayer(void *arg) {
uint64_t p0 = __atomic_load_n(&presented, __ATOMIC_ACQUIRE);
plat_mutex_lock(&sub_lock);
if (fo_waiting(&FO)) fo_run(&FO, frame_end, NULL);
if (FO.done < p0) __atomic_add_fetch(&late, 1, __ATOMIC_RELAXED);
if (__atomic_load_n(&FO.done, __ATOMIC_RELAXED) < p0) __atomic_add_fetch(&late, 1, __ATOMIC_RELAXED);
__atomic_store_n(&in_replay, 1, __ATOMIC_RELEASE);
if (r % 997 == 0) {
__atomic_store_n(&in_long, 1, __ATOMIC_RELEASE);
@@ -66,7 +75,43 @@ static void replayer(void *arg) {
}
}
int main(void) {
// vk_backend.c fo_blit_locked: the lock, waiting for it, and what is owed
static void run_locked(void *arg) {
(void)arg;
plat_mutex_lock(&sub_lock);
fo_run(&FO, frame_end, NULL);
plat_mutex_unlock(&sub_lock);
}
// vk_backend.c present_main: each frame handed over, its bookkeeping made sure of when it was left owed
// (present_frame_closed), the blit, which must see that frame's bookkeeping done, then the blit's submission (vk_sync.c
// submit).
static void present_thread(void *arg) {
(void)arg;
for (;;) {
int stopping = __atomic_load_n(&stop, __ATOMIC_ACQUIRE);
uint64_t j = __atomic_exchange_n(&slot, 0, __ATOMIC_ACQ_REL);
if (!j) {
if (stopping) break;
plat_yield();
continue;
}
uint64_t frame = j >> 1;
if ((j & 1) && !old_mode && fo_close(&FO, frame, run_locked, NULL)) closed_here++;
blits++;
if (__atomic_load_n(&ended, __ATOMIC_ACQUIRE) < frame) blit_early++;
for (volatile int i = 0; i < 2000; i++) {} // the blit's recording
plat_mutex_lock(&sub_lock);
if (fo_waiting(&FO)) fo_run(&FO, frame_end, NULL);
__atomic_store_n(&in_replay, 1, __ATOMIC_RELEASE);
for (volatile int i = 0; i < 500; i++) {}
__atomic_store_n(&in_replay, 0, __ATOMIC_RELEASE);
plat_mutex_unlock(&sub_lock);
}
}
int main(int argc, char **argv) {
old_mode = argc > 1 && !strcmp(argv[1], "old");
// one thread first: the present runs its own frame's bookkeeping when the lock is free, and two presents with the
// lock held elsewhere leave both frames to the next holder, which runs them in order
int here = fo_present(&FO, 1, try_run, NULL);
@@ -77,23 +122,38 @@ int main(void) {
CHECK(fo_run(&FO, frame_end, NULL) == 2 && ends == 3 && !bad_order && !fo_waiting(&FO),
"the next holder did not catch up both frames in order");
CHECK(fo_present(&FO, 2, try_run, NULL) == 0 && ends == 3 && !fo_waiting(&FO), "an older frame number ran again");
CHECK(fo_ran(&FO, 3) && !fo_ran(&FO, 4), "fo_ran does not follow the bookkeeping that ran");
CHECK(fo_close(&FO, 3, run_locked, NULL) == 0, "fo_close took the lock for a frame whose bookkeeping ran");
plat_mutex_unlock(&sub_lock);
last_end = 3, ends = 0;
CHECK(fo_present(&FO, 4, try_run, NULL) == 1 && fo_close(&FO, 4, run_locked, NULL) == 0 && ends == 4,
"a frame whose present ran its bookkeeping was not seen as closed");
plat_mutex_lock(&sub_lock);
CHECK(fo_present(&FO, 5, try_run, NULL) == 0 && !fo_ran(&FO, 5), "a frame left owed was seen as closed");
plat_mutex_unlock(&sub_lock);
CHECK(fo_close(&FO, 5, run_locked, NULL) == 1 && fo_ran(&FO, 5) && ends == 5 && !bad_order,
"fo_close did not run the owed frame's bookkeeping");
FO.owed = FO.done = 3, last_end = 3, ends = 0, __atomic_store_n(&ended, 3, __ATOMIC_RELAXED);
PlatThread *t[REPLAYERS];
PlatThread *t[REPLAYERS], *pt = plat_thread_new(present_thread, NULL, "present");
for (int i = 0; i < REPLAYERS; i++) t[i] = plat_thread_new(replayer, (void *)(uintptr_t)(i + 1), "replay");
for (uint64_t f = 4; f < 4 + FRAMES; f++) {
int lng = __atomic_load_n(&in_long, __ATOMIC_ACQUIRE);
uint64_t t0 = plat_now_ns();
if (fo_present(&FO, f, try_run, NULL)) ran_here++;
int here = fo_present(&FO, f, try_run, NULL);
ran_here += here;
uint64_t d = plat_now_ns() - t0;
if (d > present_max_ns) present_max_ns = d;
if (lng) during_long++;
__atomic_store_n(&presented, f, __ATOMIC_RELEASE);
// the hand-off: the present thread took the frame before (vk_backend.c present_image's wait while pending)
while (__atomic_load_n(&slot, __ATOMIC_ACQUIRE)) plat_yield();
__atomic_store_n(&slot, f << 1 | (uint64_t)!here, __ATOMIC_RELEASE);
for (volatile int i = 0; i < 20000; i++) {}
}
while (__atomic_load_n(&slot, __ATOMIC_ACQUIRE)) plat_yield();
__atomic_store_n(&stop, 1, __ATOMIC_RELEASE);
for (int i = 0; i < REPLAYERS; i++) plat_thread_join(t[i]);
plat_thread_join(pt);
plat_mutex_lock(&sub_lock);
fo_run(&FO, frame_end, NULL);
plat_mutex_unlock(&sub_lock);
@@ -103,9 +163,14 @@ int main(void) {
CHECK(!late, "%llu replays began before the bookkeeping of a frame presented earlier", (unsigned long long)late);
CHECK(present_max_ns < PRESENT_MAX_NS, "a present took %.2f ms", present_max_ns / 1e6);
CHECK(ran_here < FRAMES, "no present found the lock held, so nothing was owed");
printf("frame_owed_check: %d frames: %llu ends ran at the present, %llu owed to a replay; %llu replays held the lock "
"%d ms, %llu presents came during one; the longest present %.3f ms; %s\n", FRAMES,
(unsigned long long)ran_here, (unsigned long long)(FRAMES - ran_here), (unsigned long long)long_holds,
HOLD_MS, (unsigned long long)during_long, present_max_ns / 1e6, fails ? "FAILED" : "passed");
CHECK(during_long, "no present came during a long hold");
CHECK(blits == FRAMES, "the present thread blitted %llu of %d frames", (unsigned long long)blits, FRAMES);
CHECK(!blit_early, "%llu blits came before their frame's bookkeeping", (unsigned long long)blit_early);
printf("frame_owed_check%s: %d frames: %llu ends ran at the present, %llu owed to a replay, %llu of those caught up "
"by the present thread before their blits; %llu replays held the lock %d ms, %llu presents came during one; the "
"longest present %.3f ms; %llu blits before their frame's bookkeeping; %s\n", old_mode ? " old" : "", FRAMES,
(unsigned long long)ran_here, (unsigned long long)(FRAMES - ran_here), (unsigned long long)closed_here,
(unsigned long long)long_holds, HOLD_MS, (unsigned long long)during_long, present_max_ns / 1e6,
(unsigned long long)blit_early, fails ? "FAILED" : "passed");
return fails ? 1 : 0;
}

View File

@@ -1,10 +1,13 @@
"""Build and run tests/frame_owed_check.c: runtime/vk_frame_owed.h, a frame's bookkeeping at its present in replay order
(vk_backend.c "a frame's bookkeeping in replay order", vk_sync.c submit). One presenting thread and three replaying
threads that hold the lock for their replays, a few of them for 40 ms, over 20,000 frames: every frame's bookkeeping
runs once and in frame order, never during a replay, and before the body of any replay that took the lock after the
frame was presented, and no present takes 20 ms, so the presenting thread never waits for a replay. Step by step: a
present with the lock free runs its own frame's, presents past a held lock leave theirs to the next holder, which
runs them in order, and an older frame number runs nothing again.
(vk_backend.c "a frame's bookkeeping in replay order", vk_sync.c submit). One presenting thread, three replaying
threads that hold the lock for their replays, a few of them for 40 ms, and a present thread that takes each frame
from the presenting thread, blits it and submits, over 20,000 frames: every frame's bookkeeping runs once and in frame
order, never during a replay, before the body of any replay that took the lock after the frame was presented and
before the present thread's blit of the frame (vk_backend.c present_frame_closed), and no present takes 20 ms, so the
presenting thread never waits for a replay. Step by step: a present with the lock free runs its own frame's, presents
past a held lock leave theirs to the next holder, which runs them in order, an older frame number runs nothing again,
and the present thread takes the lock only for a frame still owed. The exe's "old" argument leaves the present
thread's catch-up out and fails on the blits that come before their frame's bookkeeping.
No GPU, no game files, any Python.