Keep the graphics lock between graphics hooks (main thread)

docs/architecture-migration.md

@@ -115,6 +115,7 @@
     - hook：其餘 30 個 `SFR_HOOK` 都改掉了。NUI 裝置（初始化、骨架追蹤、影像串流、身分辨識）拿輸入鎖；選單的 hook（按鈕、語音、轉盤、對話框、選單管理）拿新的選單鎖（`SFR_MENU_HOOK`），順序固定「選單→輸入→圖形」；只碰 guest 記憶體、參數與 atomic 的（相機影像槽、純虛函式工作、Avatar 零件、深度檢視、淡出、`LockRect` 唯讀旗標）不拿鎖；影片進度表、Avatar 動作片段各有自己的鎖，Avatar 模型的繪製拿圖形鎖。建立／重設裝置與 present 只拿圖形鎖：它們碰到的其他主機狀態（視窗、計時）屬於擁有視窗的那個執行緒。
     - GPU 資源同步（`synchronize_resource_memory`：等 GPU 閒置、丟掉 renderer 的副本）拿圖形鎖，不再拿全域許可（原本等閒置時對 queue 送出也沒有和 render thread 互斥）。
     - 結果：`all` 模式的整場比賽裡，沒有任何執行緒回到全域許可（`SFR_PARALLEL_HOOK_STATS=1` 的 import／hook 統計一次都沒有印出）。全域許可只剩給非 `all` 模式的排程用。
+  - 圖形鎖留著（2026-10-20，`SFR_GRAPHICS_LOCK_DEFER`，預設開）：比賽中只有主執行緒拿圖形鎖、從不競爭（`SFR_GRAPHICS_LOCK_STATS=1`），但每幀拿放約 16,000 次。圖形 hook 返回時主執行緒保留這把鎖，下一個圖形 hook 直接接手；在 hook 之外只在跑生成程式碼時拿著，遇到 import、其他 hook（含 `SFR_CONCURRENT_HOOK`）、許可檢查點、慢速記憶體存取、`traced_host_wait` 之前先放掉，有人在等時每次 hook 返回都放。鎖的順序不變。Thor 上 mutex 的時間每幀 0.52 → 0.18 ms（`docs/performance.md`）。
 
 ### 第 5 階段：重編譯期的 mid-asm hook 與幀率
 

docs/performance.md

@@ -2383,3 +2383,92 @@ The constant writes themselves (`SFR_CONSTANT_TIMING=1`, one run each): 561
 ns and 8192 bytes a draw before, 237 ns and 623 bytes after, at about 820
 draws a drawn frame. Windows (i9, Vulkan / D3D12, a machine with other load):
 776 / 778 ns a draw before, 199 / 140 ns after.
+
+## 2026-10-20: The graphics lock on the main thread
+
+The pass above left the graphics lock alone as "an uncontended mutex pair,
+~2%". This pass measured what it protects, who takes it, and what it costs.
+
+**What it protects.** One thread at a time in the native device: every
+Direct3D replacement in `guest_graphics_hooks.cpp` (`SFR_GRAPHICS_HOOK`,
+`SFR_GRAPHICS_HOST_HOOK`) takes it at its observed entry and gives it back as
+its `HookScope` ends; `HostGraphicsScope` takes it for host code outside those
+hooks that changes the device (`synchronize_resource_memory`, the renderer's
+`invalidate` after `MmFreePhysicalMemory`, the Avatar model's draw). Order:
+menu, input, graphics; graphics before the permits.
+
+**Who takes it.** `SFR_GRAPHICS_LOCK_STATS=1` prints `GRAPHICS_LOCK` every 120
+frames (per frame, per guest thread: acquisitions, waits and wait time, nested
+hooks, `HostGraphicsScope` acquisitions, and how far apart the acquisitions
+are). Full Dolphin Resort races on Windows (Vulkan) and the AYN Thor:
+
+| | menus | race |
+| --- | --- | --- |
+| main-thread acquisitions per frame | ~700 | 14,000-20,000 (Thor 16,000-20,600) |
+| acquisitions that found it held | 0 | 0 |
+| other threads' acquisitions | none | none |
+| `HostGraphicsScope` | <1 | 0 |
+| no import, other hook or permit checkpoint since the last release | 85% | 91% |
+
+So in a race it is only ever the main thread's, and it is never contended:
+the cost is the uncontended pair itself. A loop of try_lock/unlock costs 33 ns
+a pair on the Thor (18 ns on the desktop) against 14.5 ns (6 ns) for a bare
+atomic pair: bionic's `pthread_mutex_trylock`/`unlock` are calls through the
+PLT that check the mutex type and then run the outline atomics
+(`__aarch64_cas2_acq`, `__aarch64_swp2_rel`), which simpleperf showed as the
+hot part. Hooks are 4-15 guest entries apart for most of them.
+
+**Change: `SFR_GRAPHICS_LOCK_DEFER`** (on unless 0). The main thread keeps the
+lock when a graphics hook returns (`hook_scope_exit`), and the next graphics
+hook takes it over without an atomic. Outside a hook it is then held only
+while generated code runs, and it is given back before anything that may
+wait or take another lock: an import (`dispatch_import`), any other hook
+(`HookScope`, `SFR_CONCURRENT_HOOK`, the observed entry of a hook, before its
+menu or input lock), the permit's checkpoint (`guest_checkpoint_permit`, at
+least every 256 entries and loop labels, before the permit can hand off the
+core), a slow memory access (`parallel_slow_access`) and `traced_host_wait`.
+`HostGraphicsScope` takes a kept lock over, so nothing inside it gives the
+lock back early. While another thread waits for the lock (`graphics_waiters`,
+its own cache line), it is given back at every hook's return as before, and
+a waiter waits at most until the main thread's next checkpoint. Everything
+the generated code calls besides those is a memory, vector or reservation
+helper (leaf locks only), `read_time_base`, the job mid-asm hooks (their
+waits go through `traced_host_wait`) or `synchronize_resource_memory`
+(`HostGraphicsScope`). The register save/restore helpers, which never wait,
+are `SFR_REGISTER_HOOK` without the check. `parallel_function_entry` also
+returns before the permit's thread_local when the subsystem locks leave it
+nothing to do. `GRAPHICS_LOCK_DEFER 1` in the log says it is on. With it, a
+race frame takes the lock ~1,100-1,500 times instead of ~16,000.
+
+**Thor A/B** (simpleperf, 40 s of the moving race from ~frame 13800, same APK
+with `SFR_GRAPHICS_LOCK_DEFER=0` and on, main thread):
+
+| | lock off (defer 0) | kept (default) |
+| --- | --- | --- |
+| `__aarch64_cas2_acq` + `__aarch64_swp2_rel` | 2.44% (0.35 ms/frame) | 0.87% (0.12 ms/frame) |
+| `pthread_mutex_*` + `std::mutex` | 1.17% (0.17 ms/frame) | 0.44% (0.06 ms/frame) |
+| mutex total | 0.52 ms/frame | 0.18 ms/frame |
+| main-thread CPU in the 40 s | 34.2 s | 32.6 s |
+
+The 0.18 ms left is other locks (critical sections, the layout's
+shared_mutex, events). Frame times (HOST_THREADS, frames 13000-21000,
+`!SFR_FRAME_METRICS=0`) did not resolve the 0.34 ms against their noise: the
+main thread moves between the X3 and the A715s from run to run, and drawn
+frames sit at the 16.7 ms budget, so `skipped` swings 34-176 between runs of
+the same APK. Runs (drawn_ms / undrawn_ms / cpu_ms / skipped of 9600):
+
+| Build | runs |
+| --- | --- |
+| release-equivalent (0.10.13 code; one with the stats on) | 16.03/8.60/13.85/34, 15.99/9.66/14.33/176 |
+| kept off (`SFR_GRAPHICS_LOCK_DEFER=0`, profiled) | 15.89/9.53/14.24/157 |
+| kept | 16.23/9.88/14.04/168, 15.62/9.42/14.26/117, 16.18/8.97/13.70/52 (profiled), 15.87/9.33/13.91/87 |
+
+**Checked:** full races with the lock kept on Windows (Vulkan with the
+Khronos validation layer, 0 messages, 455/455 pipelines; D3D12, 465/465) and
+four on the Thor (and one with it off), each to the present limit with no
+hang, stop or `LOCK_LONG_WAIT`; screenshots render normally. Two fixed-step (`SFR_REALTIME_RACE=0`) Windows races of the same
+build already differ at the same presents (AI positions), as much as kept
+against not, so pictures are compared by eye rather than bit for bit.
+
+**Not done:** the observed entry of each graphics hook
+(`enter_function_observed` ~1%, its TLS and bit tests) is still per hook.

src/diagnostic_hooks.h

@@ -121,6 +121,15 @@ inline void guest_checkpoint() {
 void enter_function_observed(PPCContext&, const char*, uint32_t);
 // Gives back the permit and graphics lock the outermost hook's entry took.
 void hook_scope_exit(GuestEntryState& entry) noexcept;
+// The main thread keeps the graphics lock from one graphics hook to the next
+// (SFR_GRAPHICS_LOCK_DEFER): a race frame enters some 15,000 of them, nearly
+// all a few guest calls apart. Kept, it is given back before anything that
+// may wait or take another lock: an import, any other hook, the permit's
+// checkpoint, a slow memory access (diagnostic_main.cpp).
+void give_back_kept_graphics(GuestEntryState& entry) noexcept;
+inline void give_back_kept_graphics_lock(GuestEntryState& entry) noexcept {
+    if (entry.graphics_kept) [[unlikely]] give_back_kept_graphics(entry);
+}
 // The calling thread's state, kept in the context's reservation register,
 // which the generated code never uses (sfr::load_reserved_word keeps the
 // reservation in GuestThreadState). On Android libmain is loaded after start,
@@ -139,10 +148,16 @@ inline GuestThreadState& entry_thread_state(Context& ctx) {
     }
 }
 // Around every hook, with the state its context holds (thousands of graphics
-// hooks a frame paid the TLS resolver twice each on Android).
+// hooks a frame paid the TLS resolver twice each on Android). A hook other
+// than a graphics one gives back a kept graphics lock first: its host code may
+// wait or take a lock of its own (and not every hook names itself with
+// enter_function, which would give it back too).
 struct HookScope {
     template <class Context>
-    explicit HookScope(Context& ctx) : entry_(entry_thread_state(ctx).entry) { ++entry_.hook_depth; }
+    HookScope(Context& ctx, bool graphics) : entry_(entry_thread_state(ctx).entry) {
+        if (!graphics) give_back_kept_graphics_lock(entry_);
+        ++entry_.hook_depth;
+    }
     ~HookScope() {
         if (!--entry_.hook_depth && (entry_.hook_stack_pointer || entry_.graphics_stack_pointer || entry_.input_stack_pointer))
             hook_scope_exit(entry_);
@@ -396,18 +411,26 @@ inline constexpr sfr::NoFastPath sfr_fast{};
 // thread that calls the hook from a loop in the same frame: the Kinect
 // skeleton thread then held the global permit for good after its first
 // NuiSkeletonGetNextFrame, stalling the whole game for its time slices.)
-#define SFR_SCOPED_HOOK(x, registration) [[maybe_unused]] static const bool x##_hook = registration; \
+#define SFR_SCOPED_HOOK(x, registration, graphics) [[maybe_unused]] static const bool x##_hook = registration; \
     static void x##_hooked(PPCContext& __restrict ctx, uint8_t* base); \
-    PPC_FUNC(x) { ::sfr::HookScope scope(ctx); x##_hooked(ctx, base); } \
+    PPC_FUNC(x) { ::sfr::HookScope scope(ctx, graphics); x##_hooked(ctx, base); } \
     static void x##_hooked(PPCContext& __restrict ctx, uint8_t* base)
-#define SFR_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_hook(#x))
+#define SFR_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_hook(#x), false)
 // A Direct3D replacement that touches only the native device and guest memory
 // (see graphics_hook_bits), and one that also reaches other host state.
-#define SFR_GRAPHICS_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_graphics_hook(#x, false))
-#define SFR_GRAPHICS_HOST_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_graphics_hook(#x, true))
-#define SFR_INPUT_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_input_hook(#x))
-#define SFR_MENU_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_menu_hook(#x))
+#define SFR_GRAPHICS_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_graphics_hook(#x, false), true)
+#define SFR_GRAPHICS_HOST_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_graphics_hook(#x, true), true)
+#define SFR_INPUT_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_input_hook(#x), false)
+#define SFR_MENU_HOOK(x) SFR_SCOPED_HOOK(x, ::sfr::register_menu_hook(#x), false)
 // A replacement that touches only guest memory, its arguments, atomics and
 // host objects that keep their own lock, so a detached guest runs it without
 // taking any lock of the runtime's (docs/architecture-migration.md, phase 4).
-#define SFR_CONCURRENT_HOOK(x) PPC_FUNC(x)
+// Such host code may still wait (the job system's patches) or take a lock of
+// its own, so it gives back a kept graphics lock first.
+#define SFR_CONCURRENT_HOOK(x) static void x##_concurrent(PPCContext& __restrict ctx, uint8_t* base); \
+    PPC_FUNC(x) { ::sfr::give_back_kept_graphics_lock(::sfr::entry_thread_state(ctx).entry); x##_concurrent(ctx, base); } \
+    static void x##_concurrent(PPCContext& __restrict ctx, uint8_t* base)
+// A replacement that touches only this thread's context and guest memory,
+// never waits and takes no lock (the register save and restore helpers,
+// called at most function entries).
+#define SFR_REGISTER_HOOK(x) PPC_FUNC(x)