From 618cfc01f4c136794ca0330da417fbf02b1bf40d Mon Sep 17 00:00:00 2001 From: chrg6 Date: Fri, 13 Nov 2026 19:42:03 -0500 Subject: [PATCH] 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 --- docs/architecture.md | 14 +++--- docs/building.md | 11 ++--- nx2navi.ini.example | 5 ++- runtime/vk.h | 4 ++ runtime/vk_backend.c | 82 ++++++++++++++++++++++++++---------- runtime/vk_frame_owed.h | 29 +++++++++++-- runtime/vk_sync.c | 6 +++ tests/frame_owed_check.c | 89 +++++++++++++++++++++++++++++++++------ tests/frame_owed_check.py | 15 ++++--- 9 files changed, 196 insertions(+), 59 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 0b0a2e08..336fcfc9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -94,13 +94,13 @@ keeps per-core data without locks and relies on that, so the host models it (`ru - Several submitting threads. The game submits from several threads and presents from one of them. Their replays take turns on the submission lock (`vk_sync.c`). The state a present closes for the frame (the DLSS state, the motion tables, the frame count conditional draws go by) is reset between two replays: by the present when the lock is free, - else first thing in the next submission (`vk_frame_owed.h`, `NX2NAVI_PRESENT_ORDER`). A present held for - event-ordered work is put out once and in order by whichever thread's submission finds its work done - (`vk_present_held.h`); submitting threads only try its lock, and the presenting thread waits for it only while - another thread puts a held present out. A recording two threads submit at once gets a copy of its own for the - second, for its deferred binds (`nvn_submit_guard.h`). Held work is asked about without the lock from a published - copy (`vk_evo_peek.h`). The log counts the overlaps with `NX2NAVI_TRACE=perf` ("enters a submit ... while"), and at - exit what they cost ("frame bookkeeping: ..."). + else first thing in the next submission or by the present thread before the frame's blit, whichever comes first + (`vk_frame_owed.h`, `NX2NAVI_PRESENT_ORDER`). A present held for event-ordered work is put out once and in order by + whichever thread's submission finds its work done (`vk_present_held.h`); submitting threads only try its lock, and + the presenting thread waits for it only while another thread puts a held present out. A recording two threads + submit at once gets a copy of its own for the second, for its deferred binds (`nvn_submit_guard.h`). Held work is + asked about without the lock from a published copy (`vk_evo_peek.h`). The log counts the overlaps with + `NX2NAVI_TRACE=perf` ("enters a submit ... while"), and at exit what they cost ("frame bookkeeping: ..."). - Driver data. The NVN driver keeps a per-stage constant bank (c0). The back end fills it as the driver does: storage and uniform tables, push constants and, for NVN 2 compute, the dispatch's group counts in bytes 0-0xb. - Storage buffers the shaders reach through pointer tables in memory are bound at the address the table holds diff --git a/docs/building.md b/docs/building.md index b2108b6d..ce103f42 100644 --- a/docs/building.md +++ b/docs/building.md @@ -775,11 +775,12 @@ held elsewhere a submitting thread gives up at once while the presenting thread game files. A change to where a frame's bookkeeping at its present runs (`runtime/vk_frame_owed.h`, `runtime/vk_backend.c` -`present_image` and `frame_end`, `runtime/vk_sync.c` `submit` and `vk_sync_try_locked`) passes -`python tests\frame_owed_check.py`, which builds the header with the platform layer: one presenting and three -replaying threads, some replays holding the lock 40 ms, over 20,000 frames; every frame's bookkeeping runs once, in -order, never during a replay and before any replay that took the lock after its present, and no present waits for a -replay. No GPU, no game files. +`present_image`, `present_frame_closed` and `frame_end`, `runtime/vk_sync.c` `submit`, `vk_sync_try_locked` and +`vk_sync_locked`) passes `python tests\frame_owed_check.py`, which builds the header with the platform layer: one +presenting thread, three replaying threads, some replays holding the lock 40 ms, and a present thread that blits each +frame handed to it, over 20,000 frames; every frame's bookkeeping runs once, in order, never during a replay, before +any replay that took the lock after its present and before the frame's blit, and no present waits for a replay. No +GPU, no game files. A change to a synchronous submission's deferred binds (`runtime/nvn_submit_guard.h`, `runtime/nvn_api_cmd.c` `nvn_submit_prepare`, `nvn_submit_done`, `resolve_deferred`) passes `python tests\submit_clone_check.py`, which builds the diff --git a/nx2navi.ini.example b/nx2navi.ini.example index 15be47fe..4f50d304 100644 --- a/nx2navi.ini.example +++ b/nx2navi.ini.example @@ -1013,8 +1013,9 @@ # wait_watch_s = 0 # 1: what the game's present closes for the frame (the DLSS state, the motion tables, the frame count conditional # draws go by) is closed between two replays: when another game thread is replaying at the present, the next -# submission does it first thing, and the present goes on at once. 0: at the present, as in 0.10.1, whatever another -# thread replays (a rare one-frame fault at area changes). Read at start. +# submission does it first thing, or the present thread before the picture's blit, and the game's present goes on at +# once. 0: at the present, as in 0.10.1, whatever another thread replays (a rare one-frame fault at area changes). +# Read at start. # present_order = 1 # A report in the log when nothing moves for this many seconds: no frame shown while the window is in front, or a GPU # submission not finished. It names who holds the present and queue locks and what the game's threads wait for, at diff --git a/runtime/vk.h b/runtime/vk.h index e24b06de..09884bfd 100644 --- a/runtime/vk.h +++ b/runtime/vk.h @@ -898,6 +898,10 @@ uint64_t vk_sync_submit_own(void (*record)(VkCommandBuffer, void *), void *arg); // Runs fn() under the submission lock when it is free now and returns 1; returns 0 at once, without running it, when // another thread holds the lock (vk_backend.c present_image: a frame's bookkeeping never waits for another replay). int vk_sync_try_locked(void (*fn)(void)); +// Runs fn() under the submission lock, waiting for it. For the present thread only, which is not one of the game's +// threads and waits for the lock for its blit's submission anyway (vk_backend.c present_frame_closed: a frame's +// bookkeeping still owed runs before the frame's blit). +void vk_sync_locked(void (*fn)(void)); // vk_backend.c (vk_frame_owed.h): the bookkeeping of the frames presented while another thread held the submission // lock. vk_sync.c submit calls it right after taking the lock, before its replay; a no-op without the back end. void vk_present_frames_owed(void); diff --git a/runtime/vk_backend.c b/runtime/vk_backend.c index 94ad5cf8..20a955aa 100644 --- a/runtime/vk_backend.c +++ b/runtime/vk_backend.c @@ -42,12 +42,20 @@ #define WEAK __attribute__((weak)) -// handoff_ns: how long the presenting thread waited to hand this job over (present_image), for the present lines -typedef struct { VkImage image; VkFormat format; int w, h, has_crop, crop[4]; uint64_t frame, at, queued, value, handoff_ns; } PresentJob; +// handoff_ns: how long the presenting thread waited to hand this job over (present_image), for the present lines. +// owed: the presenting thread left the frame's bookkeeping to the next holder of the submission lock (present_image, +// vk_frame_owed.h); the present thread makes sure it ran before the blit (present_frame_closed). +typedef struct { + VkImage image; + VkFormat format; + int w, h, has_crop, crop[4], owed; + uint64_t frame, at, queued, value, handoff_ns; +} PresentJob; static PlatMutex pl = PLAT_MUTEX_INIT; static PlatCond pcv = PLAT_COND_INIT; // job queued, job taken, job submitted static PresentJob job; +static void present_frame_closed(uint64_t frame); // "a frame's bookkeeping in replay order" static int pending, busy, present_thread; // Sleeps on a high-resolution timer to 1 ms before at, then yields until at: the timer alone wakes up to ~0.5 ms late. @@ -115,6 +123,7 @@ static void present_main(void *arg) { unsigned tr = rt_trace; // read once: the stamps and both lines below follow the same flags int timed = (tr & (RT_TRACE_PRESENT | RT_TRACE_PERF)) != 0; uint64_t b0 = timed ? kern_now_ns() : 0; + if (j.owed) present_frame_closed(j.frame); // vk_dlaa_frame's state of this frame, before anything reads it if (flight_on || vk_dump_white) vk_thumbs_frame(j.frame); // NX2NAVI_DUMP_WHITE reads them too vk_dlssg_present_frame(j.frame); vk_lowlat_present_frame(j.frame); // the frame's number for the blit's submit and the present's markers @@ -277,11 +286,14 @@ static int frame_hold(uint32_t skipped, int resized, uint64_t frame) { // then reset it in the middle of that replay: a DLSS frame reset halfway (a shake, a frame without the upscaler), the // motion tables swapped halfway (a ghost; with NX2NAVI_MVPREV_CHECK=1 a write out of bounds), the conditional draws // counted against the next frame. So it runs under the submission lock: at the present when the lock is free, else -// first thing in the next submission, on whichever thread makes it (vk_sync.c submit, vk_present_frames_owed), the -// present thread's blit submission included. The present never waits for another thread's replay for it. Nothing in -// frame_end may wait for the present thread, submit or flush with a wait: the hand-off, vk_texture_get with its upload -// flush and vk_present_drain stay in present_image, outside the lock. NX2NAVI_PRESENT_ORDER=0: at the present, whatever -// another thread replays, as in 0.10.1. +// first thing in the next submission, on whichever thread makes it (vk_sync.c submit, vk_present_frames_owed), or on +// the present thread before the frame's blit (present_frame_closed), whichever takes the lock first. The blit reads what +// vk_dlaa_frame closed (the inputs frame generation must not get across an upscaler or frame generation switch), so a +// frame's bookkeeping always comes before its blit, as in 0.10.1. The presenting thread never waits for another +// thread's replay for it; the present thread may, for a frame still owed, as it waits for the lock for the blit's +// submission a moment later anyway. Nothing in frame_end may wait for the present thread, submit or flush with a wait: +// the hand-off, vk_texture_get with its upload flush and vk_present_drain stay in present_image, outside the lock. +// NX2NAVI_PRESENT_ORDER=0: at the present, whatever another thread replays, as in 0.10.1. static void frame_end(uint64_t frame, const tex_t *t) { // t: the presented texture, for the trace = gfx line vk_exec_frame_log(t, frame); dynres_frame(); // before vk_dlaa_frame reads the DLSS mode it may override @@ -296,6 +308,8 @@ static void frame_end(uint64_t frame, const tex_t *t) { // t: the presented te static FrameOwed FO; static const tex_t *fo_tex[FO_TEX]; // the newest frames' presented textures, by frame % FO_TEX (atomics) static uint64_t fo_owed_n, fo_late_n; // presents that left it to a submission; frames a submission caught up (atomics) +static uint64_t fo_blit_n, fo_blit_ns; // frames the present thread caught up before their blits, and the time it took + // with the wait for the lock (the present thread's; read by order_log: atomics) static int present_order_on(void) { // read once; present_image runs one at a time static int on = -1; @@ -322,6 +336,23 @@ void vk_present_frames_owed(void) { if (fo_waiting(&FO)) __atomic_add_fetch(&fo_late_n, fo_run(&FO, fo_end, NULL), __ATOMIC_RELAXED); } +// The present thread, before it uses frame (present_main): frame_end of a frame the presenting thread left owed has run +// by then. When no submission came between, the present thread runs it itself, under sub_lock, waiting for a replay in +// progress. Not one of the game's threads, it waits for the same lock for the blit's submission a moment later anyway; +// a frame whose bookkeeping ran (the presenting thread got the lock, or a submission came first) takes no lock here. +static void fo_blit_here(void) { + __atomic_add_fetch(&fo_blit_n, fo_run(&FO, fo_end, NULL), __ATOMIC_RELAXED); +} +static void fo_blit_locked(void *arg) { + (void)arg; + vk_sync_locked(fo_blit_here); +} +static void present_frame_closed(uint64_t frame) { + uint64_t t0 = kern_now_ns(); + if (fo_close(&FO, frame, fo_blit_locked, NULL)) + __atomic_add_fetch(&fo_blit_ns, kern_now_ns() - t0, __ATOMIC_RELAXED); +} + static void order_log(uint64_t frame, int total); // after the held present's state (PH) // The present after the frame count: resolves the image and hands the blit to the present thread. queued: when the game @@ -331,9 +362,10 @@ static void present_image(const tex_t *t, const int crop[4], uint64_t at_ns, uin static volatile int32_t n_skipped; static int vram_probe; if (!vram_probe) vram_probe = 1, perfmon_set_vram_game(vk_mem_device_used_mb); // the overlay's "game" beside VRAM + int owed = 0; // the frame's bookkeeping may still be owed when the present thread takes it (PresentJob.owed) if (present_order_on()) { __atomic_store_n(&fo_tex[frame % FO_TEX], t, __ATOMIC_RELAXED); - if (!fo_present(&FO, frame, fo_try, NULL)) __atomic_add_fetch(&fo_owed_n, 1, __ATOMIC_RELAXED); + if (!fo_present(&FO, frame, fo_try, NULL)) owed = 1, __atomic_add_fetch(&fo_owed_n, 1, __ATOMIC_RELAXED); } else { frame_end(frame, t); } @@ -355,7 +387,7 @@ static void present_image(const tex_t *t, const int crop[4], uint64_t at_ns, uin return; } PresentJob j = {.image = img->image, .format = img->format, .w = (int)img->width, .h = (int)img->height, .frame = frame, - .at = at_ns, .queued = queued, .value = value}; + .at = at_ns, .queued = queued, .value = value, .owed = owed}; vk_hdr_present_source(img, &j.image, &j.format); // NX2NAVI_HDR: the float picture the game copied into it, same size if (crop) { j.has_crop = 1; @@ -795,27 +827,31 @@ static void replay_log(uint64_t frame, int total) { } // What the game's threads met of each other at the present (synchronous replay): presents whose frame bookkeeping -// went to the next submission because another thread held the submission lock, frames a submission caught up, held -// presents another thread's submission put out (ph_out), and the presenting thread's waits for PH.lock while one went -// out (vk_present_held.h ph_lock). total: the run, at exit (gfx_exit_log); else NX2NAVI_TRACE=perf every 600 presents -// (present_image), the window since the last line, logged only when a count moved. +// went to the next submission because another thread held the submission lock, frames a submission caught up, frames +// the present thread caught up before their blits (present_frame_closed) and how long that took it, the wait for the +// lock included, held presents another thread's submission put out (ph_out), and the presenting thread's waits for +// PH.lock while one went out (vk_present_held.h ph_lock). total: the run, at exit (gfx_exit_log); else +// NX2NAVI_TRACE=perf every 600 presents (present_image), the window since the last line, logged only when a count moved. static void order_log(uint64_t frame, int total) { - static uint64_t last[5], last_frame; // present_image's (one at a time): the counts at the last line - uint64_t v[5] = {__atomic_load_n(&fo_owed_n, __ATOMIC_RELAXED), __atomic_load_n(&fo_late_n, __ATOMIC_RELAXED), + static uint64_t last[7], last_frame; // present_image's (one at a time): the counts at the last line + uint64_t v[7] = {__atomic_load_n(&fo_owed_n, __ATOMIC_RELAXED), __atomic_load_n(&fo_late_n, __ATOMIC_RELAXED), __atomic_load_n(&ph_other, __ATOMIC_RELAXED), __atomic_load_n(&PH.lock_waits, __ATOMIC_RELAXED), - __atomic_load_n(&PH.lock_wait_ns, __ATOMIC_RELAXED)}; - uint64_t d[5], presents = total ? frame : frame - last_frame; - for (int i = 0; i < 5; i++) d[i] = total ? v[i] : v[i] - last[i]; + __atomic_load_n(&PH.lock_wait_ns, __ATOMIC_RELAXED), __atomic_load_n(&fo_blit_n, __ATOMIC_RELAXED), + __atomic_load_n(&fo_blit_ns, __ATOMIC_RELAXED)}; + uint64_t d[7], presents = total ? frame : frame - last_frame; + for (int i = 0; i < 7; i++) d[i] = total ? v[i] : v[i] - last[i]; if (!total) { - if (!d[0] && !d[1] && !d[2] && !d[3]) return; + if (!d[0] && !d[1] && !d[2] && !d[3] && !d[5]) return; memcpy(last, v, sizeof last), last_frame = frame; } rt_log("%s frame %llu: frame bookkeeping: %llu of %llu presents left it to the next submission (another thread held " - "the submission lock), %llu frames caught up by a submission; held presents put out by another thread's " - "submission %llu, the presenting thread waited for one %llu times, %.1f ms in all, longest %.1f ms so far", + "the submission lock), %llu frames caught up by a submission, %llu by the present thread before their blits " + "(%.1f ms in all with its wait for the lock); held presents put out by another thread's submission %llu, the " + "presenting thread waited for one %llu times, %.1f ms in all, longest %.1f ms so far", total ? "vk: over the run to" : "perf: since the last such line, at", (unsigned long long)frame, - (unsigned long long)d[0], (unsigned long long)presents, (unsigned long long)d[1], (unsigned long long)d[2], - (unsigned long long)d[3], d[4] / 1e6, __atomic_load_n(&PH.lock_wait_max, __ATOMIC_RELAXED) / 1e6); + (unsigned long long)d[0], (unsigned long long)presents, (unsigned long long)d[1], (unsigned long long)d[5], + d[6] / 1e6, (unsigned long long)d[2], (unsigned long long)d[3], d[4] / 1e6, + __atomic_load_n(&PH.lock_wait_max, __ATOMIC_RELAXED) / 1e6); } // The run total, once: the game exits through prof_write (boot.c _exit, the window close), probes through diff --git a/runtime/vk_frame_owed.h b/runtime/vk_frame_owed.h index b3e9ec18..4428ac7f 100644 --- a/runtime/vk_frame_owed.h +++ b/runtime/vk_frame_owed.h @@ -7,26 +7,39 @@ // it marks its frame owed and runs the bookkeeping itself only when it gets the lock at once; otherwise the next thread // to take the lock (vk_sync.c submit) runs every owed frame's bookkeeping, in frame order, before its own replay. A frame // caught up behind a later present runs as two presents with no submission between them always did. +// +// The present thread's blit of a frame reads what that frame's bookkeeping closed (vk_dlaa.c vk_dlaa_frame: the +// inputs frame generation must not get across an upscaler or frame generation switch), so the bookkeeping must come +// before it, as it always did when it ran at the present. A frame left owed may still be owed when the present thread +// takes it: fo_close runs what is owed then, under the lock, waiting for a replay in progress as the present thread's +// own blit submission does a moment later. The present thread is not one of the game's threads. #pragma once #include typedef struct { uint64_t owed; // the newest presented frame (atomic: the presenting thread raises it, the lock's holder reads it) - uint64_t done; // the newest frame whose bookkeeping ran (the lock's holder only) + uint64_t done; // the newest frame whose bookkeeping ran (the lock's holder writes it; atomic for fo_ran) } FrameOwed; typedef void (*FrameEnd)(uint64_t frame, void *arg); // The lock's holder: whether a frame waits for its bookkeeping (one atomic load, for every submission). -static inline int fo_waiting(FrameOwed *o) { return __atomic_load_n(&o->owed, __ATOMIC_ACQUIRE) != o->done; } +static inline int fo_waiting(FrameOwed *o) { + return __atomic_load_n(&o->owed, __ATOMIC_ACQUIRE) != __atomic_load_n(&o->done, __ATOMIC_RELAXED); +} // The lock's holder: the bookkeeping of every frame presented since the last run, in frame order; returns how many. +// done moves past a frame only once its bookkeeping returned (fo_ran reads it without the lock). static inline uint64_t fo_run(FrameOwed *o, FrameEnd end, void *arg) { - uint64_t owed = __atomic_load_n(&o->owed, __ATOMIC_ACQUIRE), n = 0; - while (o->done < owed) end(++o->done, arg), n++; + uint64_t owed = __atomic_load_n(&o->owed, __ATOMIC_ACQUIRE), d = __atomic_load_n(&o->done, __ATOMIC_RELAXED), n = 0; + while (d < owed) end(++d, arg), __atomic_store_n(&o->done, d, __ATOMIC_RELEASE), n++; return n; } +// Any thread: whether frame's bookkeeping ran; when it did, what it wrote is visible to the caller (the release of done +// in fo_run). +static inline int fo_ran(FrameOwed *o, uint64_t frame) { return __atomic_load_n(&o->done, __ATOMIC_ACQUIRE) >= frame; } + // The presenting thread, frame numbers rising (a smaller one than owed leaves it: that frame's bookkeeping ran or is // owed already). try_run(arg) takes the lock without waiting, runs fo_run under it and returns 1, or returns 0 when // another thread holds it. Returns whether the bookkeeping ran here. @@ -35,3 +48,11 @@ static inline int fo_present(FrameOwed *o, uint64_t frame, int (*try_run)(void * while (was < frame && !__atomic_compare_exchange_n(&o->owed, &was, frame, 1, __ATOMIC_RELEASE, __ATOMIC_RELAXED)) {} return try_run(arg); } + +// The present thread, before it uses a presented frame (its blit): when the frame's bookkeeping has not run, run_locked +// (arg) takes the lock, waiting for it, and runs fo_run under it. Returns whether it had to; afterwards fo_ran(frame). +static inline int fo_close(FrameOwed *o, uint64_t frame, void (*run_locked)(void *), void *arg) { + if (fo_ran(o, frame)) return 0; + run_locked(arg); + return 1; +} diff --git a/runtime/vk_sync.c b/runtime/vk_sync.c index 3d057c10..5b00b120 100644 --- a/runtime/vk_sync.c +++ b/runtime/vk_sync.c @@ -1582,6 +1582,12 @@ int vk_sync_try_locked(void (*fn)(void)) { return 1; } +void vk_sync_locked(void (*fn)(void)) { + plat_mutex_lock(&sub_lock); + fn(); + plat_mutex_unlock(&sub_lock); +} + // vk_backend.c's, which run the frames' bookkeeping (vk_frame_owed.h) and set the place in the frame of the submission // being recorded; builds without the back end have neither. WEAK void vk_present_frames_owed(void) {} diff --git a/tests/frame_owed_check.c b/tests/frame_owed_check.c index 8001bf19..8fb8641f 100644 --- a/tests/frame_owed_check.c +++ b/tests/frame_owed_check.c @@ -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 #include #include @@ -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; } diff --git a/tests/frame_owed_check.py b/tests/frame_owed_check.py index 168db679..94767e02 100644 --- a/tests/frame_owed_check.py +++ b/tests/frame_owed_check.py @@ -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.