Test NVN readback lifetime and document general Metal fixes

dm/games/01007EF00011E000/README.md

@@ -110,6 +110,7 @@ Used at `DM_FPS_TARGET=0` and for the interval the framework keeps while the VFR
 
 General host changes first needed here; each applies to every game:
 
+- **Resolved: GPU readback temporaries retained by a game thread's outer pool** (`mtl_read_texels`): [GPU readback without an autorelease pool](../../../docs/reference/memory.md#gpu-readback-without-an-autorelease-pool). Ported from MacNativeGames; checked here with a synthetic Metal readback lifetime test, without a new gameplay memory measurement.
 - **The host frame step in whole vsyncs** (`nvn_fstep.h`, `fstep_tick`, `FRAME_STEP` stubs, `fps_target`), explained in full under [Frame rate](#the-frame-step-current); its probes (`nvn_fstep_gpu_slowed`, `nvn_fstep_same`): [development.md, Power](../../../docs/development.md#power).
 - **Real GPU timestamps per segment** (`gpu_timestamps`, `rep_ticks`), **submissions in segments** (`sub_cut`), **deferred texture binds** (`bh_fresh`), **duplicate colour slots** (`rt_dup_slot`), **cube textures as 2D arrays** (`tex_fit`), **compressed render targets through shadows** (`bcrt_attach` / `bcrt_flush`), **pool lookups** (`pool_find`), **memory fixes** (`defer_drain`'s pool, `g_ghost`, growing pool table), **`nvnTextureCompare` with the channel mapping**, **first-use fill of flushed storage** (`nvn_flushed`, `tex_init_fill`), **partial-cover uniform shadows** (`ubp_get`, `cb_extent.tsv`): [GPU timestamps per submission segment](../../../docs/reference/frame-rate.md#gpu-timestamps-per-submission-segment), [Submissions handed to the GPU in segments](../../../docs/reference/frame-rate.md#submissions-handed-to-the-gpu-in-segments), [Deferred texture binds read at execution](../../../docs/reference/shaders.md#deferred-texture-binds-read-at-execution), [The same texture attached at two colour slots](../../../docs/reference/rendering.md#the-same-texture-attached-at-two-colour-slots), [Cube textures held as 6-layer 2D arrays](../../../docs/reference/rendering.md#cube-textures-held-as-6-layer-2d-arrays), [Compressed textures as render targets through an uncompressed shadow](../../../docs/reference/rendering.md#compressed-textures-as-render-targets-through-an-uncompressed-shadow), [Address to memory pool lookups by binary search](../../../docs/reference/frame-rate.md#address-to-memory-pool-lookups-by-binary-search), [Deferred-release drain without an autorelease pool](../../../docs/reference/memory.md#deferred-release-drain-without-an-autorelease-pool), [Dead texture serials kept in a ghost table](../../../docs/reference/nvn-objects.md#dead-texture-serials-kept-in-a-ghost-table), [Memory-pool tracking table grows in blocks](../../../docs/reference/nvn-objects.md#memory-pool-tracking-table-grows-in-blocks), [`nvnTextureCompare` compares the channel mapping](../../../docs/reference/rendering.md#nvntexturecompare-compares-the-channel-mapping), [First-use fill of storage the game flushed](../../../docs/reference/nvn-objects.md#first-use-fill-of-storage-the-game-flushed), [Partial-cover uniform shadows keep passes open](../../../docs/reference/rendering.md#partial-cover-uniform-shadows-keep-passes-open).
 - **Display-link waits per present interval** (`mtl_link_wait_tick`): [Frames of a dynamic present interval held on screen and at the acquire](../../../docs/reference/frame-rate.md#frames-of-a-dynamic-present-interval-held-on-screen-and-at-the-acquire).

dm/games/0100F2C0115B6000/README.md

@@ -68,6 +68,7 @@ Game-specific findings. The general host changes they led to are listed under [F
 
 General host changes first needed here; each applies to every game:
 
+- **Resolved: registry lock priority inversion** (`g_rmu`): [Registry lock priority inheritance](../../../docs/reference/frame-rate.md#registry-lock-priority-inheritance). Ported from MacNativeGames; checked here through the Metal registry view tests, without a new gameplay contention measurement.
 - **Conditional rendering** evaluated at replay (`OP_RENDER_COND`, profile key `render_cond`) and **CPU-written textures with no flush** (`tex_dirty_upload`, `ft_add`): [Conditional rendering evaluated at replay](../../../docs/reference/rendering.md#conditional-rendering-evaluated-at-replay), [CPU writes with no flush, caught by page protection](../../../docs/reference/nvn-objects.md#cpu-writes-with-no-flush-caught-by-page-protection-tex_dirty_upload).
 - **Mid-pass, partial-mask and partial-scissor clears** drawn as a triangle (`clear_color_tri` / `clear_ds_tri`), **8-bit indices widened on the GPU** when written in the same submission (`wide_run`), **`ReadTexels` from guest memory on any thread** with `nvn_ft_poll` marking textures written after first use, **attachment-less passes** (`bare_dims`): same section.
 - **Rewriter:** out-of-line atomic retry blocks (`retry_block` / `site_retry_window`, 8 sites) and jump tables relative to an `adr` (`jump_adr_offsets`, `main+0x393bb0`): [Out-of-line retry blocks of atomic updates](../../../docs/reference/rewriter.md#out-of-line-retry-blocks-of-atomic-updates), [Offset tables relative to an adr](../../../docs/reference/rewriter.md#offset-tables-relative-to-an-adr).

dm/gfx/nvn/nvn_mtl.m

@@ -2897,9 +2897,8 @@ static int clear_rect_zero(int i) {
 
 
 // ================= Shaders / descriptor pool / pipelines / draws =================
-// The texture / sampler registry lock: game threads take it to register textures and samplers, the submit thread for every argument encode. An os_unfair_lock lends a
-// waiter's priority to its owner: with a mutex, a game thread (priority 46) preempted while holding it kept the submit thread (97) waiting, 1.8 ms a frame in a Tears of
-// the Kingdom shrine (MacNativeGames 5f5de5e: 155 -> ~0 ms/s of waiting)
+// Game threads register textures and samplers, the submit thread encodes arguments: lend a waiting submitter's priority to the owner.
+// Measurements: docs/reference/frame-rate.md, Registry lock priority inheritance.
 static os_unfair_lock g_rmu = OS_UNFAIR_LOCK_INIT;
 // ---------- Shader mapping table: Switch control section hash → PC metallib (produced by dm/gfx/tools/shader_map.py) ----------
 static NSMutableDictionary<NSString *, NSString *> *g_shmap;
@@ -3770,8 +3769,8 @@ void arena_begin(void) {
         if (a->fence && a->fence <= done && a->len > ARENA_KEEP && g_fence > a->fence + ARENA_IDLE) { res_remove(a->buf); [g_arenas removeObjectAtIndex:i]; } }
 }
 void arena_commit(uint64_t fence) { for (NvArena *a in g_sub_arenas) { a->fence = fence; [g_arenas addObject:a]; } [g_sub_arenas removeAllObjects]; g_cur_arena = nil; }
-// buf is __strong: an unqualified id * out-parameter is __autoreleasing under ARC, which put the arena buffer in the autorelease pool at every call (a few per draw;
-// MacNativeGames 0531f28: autorelease 2.63% -> 1.73% of replay time)
+// An unqualified id * out-parameter is __autoreleasing under ARC; __strong avoids autoreleasing the arena buffer at every allocation.
+// Measurements: docs/reference/frame-rate.md, Arena buffer ownership on allocation.
 static size_t arena_alloc(size_t n, __strong id<MTLBuffer> *buf) {
     n = (n + 255) & ~(size_t)255;
     if (!g_cur_arena || g_cur_arena->used + n > g_cur_arena->len) {
@@ -7629,8 +7628,8 @@ void mtl_read_texels(const tex_t *t, unsigned level, unsigned layer0, const NVNc
         free(lin);
         return;
     }
-    // The game's own thread, outside a submission: no pool drains what Metal autoreleases here (the command buffer, the encoder, the read-back buffer), and the thread lives
-    // for good, so without one every GPU read-back stayed alive (MacNativeGames 635fb31: Breath of the Wild's 130 read-backs a second held 3.7 GB after 30 minutes)
+    // A guest thread can live indefinitely without draining its outer pool: release readback temporaries here, after the GPU and CPU copies finish.
+    // Finding: docs/reference/memory.md, GPU readback without an autorelease pool.
     @autoreleasepool {
     id<MTLTexture> mt = tex_mtl(t);
     if (!mt) { memset(dst, 0, img * nz); return; }

dm/gfx/tests/t_readback.m

@@ -15,6 +15,21 @@ uint64_t now_ns(void) { return bp_now(); }
 static int cases, failed;
 #define EXPECT(n,w,v) do { int a=(w), b=(v); cases++; printf("%s expected %d -> got %d %s\n",n,a,b,a==b?"PASS":"FAIL"); failed+=a!=b; } while (0)
 
+// Observe the real readback buffer without retaining it. The caller's outer pool stays open,
+// as on a long-lived guest thread; ReadTexels must drain its own temporary Metal objects.
+@interface ReadbackDevice : NSObject
+@property(nonatomic, strong) id<MTLDevice> device;
+@property(nonatomic, weak) id<MTLBuffer> lastBuffer;
+@property(nonatomic) NSUInteger allocations;
+@end
+@implementation ReadbackDevice
+- (id<MTLBuffer>)newBufferWithLength:(NSUInteger)length options:(MTLResourceOptions)options {
+    id<MTLBuffer> buffer = [_device newBufferWithLength:length options:options];
+    _lastBuffer = buffer; _allocations++; return buffer;
+}
+- (id)forwardingTargetForSelector:(SEL)selector { (void)selector; return _device; }
+@end
+
 int main(void) { @autoreleasepool {
     mtl_init();
     void *mem = NULL; if (posix_memalign(&mem, getpagesize(), 262144)) abort(); memset(mem, 0, 262144);
@@ -41,9 +56,13 @@ int main(void) { @autoreleasepool {
     for (int y=0; y<4; y++) { for (int x=0; x<8; x++) bad += memcmp(out+y*64+x*4, expected, 4) != 0;
         for (int x=32; x<64; x++) bad += out[y*64+x] != 0xEE; }
     EXPECT("pending clear -> GPU copy, literal pixels and untouched row padding", 0, bad);
+    ReadbackDevice *device = [ReadbackDevice new]; device.device = g_dev; g_dev = (id)device;
     uint8_t cpu[128]; memset(cpu, 0xEE, sizeof cpu); h_nvnTextureReadTexels(&tex, NULL, &region, cpu); bad = 0;
+    g_dev = device.device;
     for (int i=0; i<32; i++) bad += memcmp(cpu+i*4, expected, 4) != 0;
     EXPECT("GPU clear -> CPU ReadTexels, 32 literal pixels", 0, bad);
+    EXPECT("ReadTexels used the GPU readback path", 1, device.allocations);
+    EXPECT("ReadTexels releases its readback buffer before the caller's pool drains", 1, device.lastBuffer == nil);
     h_nvnTextureFinalize(&tex); mtl_pool_del(mem); free(mem);
     printf("READBACK cases=%d failed=%d\n", cases, failed); return failed != 0;
 } }

docs/agents/symptoms.md

@@ -61,6 +61,7 @@ One line per symptom in the [subsystem references](../reference/): what you see
 - the frames stop at the first loading screen, a graphics worker at 100% CPU (17 s in, `UnityGfxDeviceWorker` running, the main thread waiting on a condition variable, no red line). → [Unity's GPU pool block allocation spins at size 0](../reference/memory.md#unitys-gpu-pool-block-allocation-spins-at-size-0)
 - SIGBUS inside a host hook (`snprintf` under `mount_ex` / `h_fs_MountSave`), then the process hangs (Super Mario 3D World + Bowser's Fury); SIGBUS inside Metal (`AGCDeserializedReply`, an AGX compiler block) at an address in the guest stack region (Breath of the Wild). → [Host calls on small guest stacks](../reference/memory.md#host-calls-on-small-guest-stacks)
 - the iPhone app is killed for memory; resident memory grows steadily (about 0.2 MB a frame) while the residency set and the texture counts stay flat. → [Deferred-release drain without an autorelease pool](../reference/memory.md#deferred-release-drain-without-an-autorelease-pool)
+- GPU memory grows with repeated `nvnTextureReadTexels` calls on a long-lived game thread, even after each readback completes. → [GPU readback without an autorelease pool](../reference/memory.md#gpu-readback-without-an-autorelease-pool)
 
 ## Threads
 
@@ -211,6 +212,8 @@ One line per symptom in the [subsystem references](../reference/): what you see
 - a steady game whose frame-rate counter wanders between 50 and 60 on the iPhone only; the `display link up to tick` lines show about as many empty ticks as frames dropped by merging: frames arrive two in one tick and none in the next. → [Vulkan acquire and vsync event follow the display link](../reference/frame-rate.md#vulkan-acquire-and-vsync-event-follow-the-display-link)
 - asked for 60 Hz, the system fires the display link on every third refresh of a 165 Hz screen, 55 Hz, so games paced by it ran at 55 fps (27.5 at 30). → [Virtual 60 Hz ticks on screens that are not a multiple of 60](../reference/frame-rate.md#virtual-60-hz-ticks-on-screens-that-are-not-a-multiple-of-60)
 - virtual ticks keep the average, but a frame still stays up two or three refreshes in turn (13.9 / 20.8 ms at 144 Hz), and Apple's HUD showed the frame interval jumping between them. → [Fullscreen switches the screen to a rate the frame rate divides](../reference/frame-rate.md#fullscreen-switches-the-screen-to-a-rate-the-frame-rate-divides)
+- autorelease work takes a measurable share of replay time, with temporary arena-buffer ownership traffic a few times per draw. → [Arena buffer ownership on allocation](../reference/frame-rate.md#arena-buffer-ownership-on-allocation)
+- the submit thread waits on the texture / sampler registry lock while a lower-priority registration thread is preempted holding it. → [Registry lock priority inheritance](../reference/frame-rate.md#registry-lock-priority-inheritance)
 - the GPU sits idle each frame while the host translates the submission (a game that does not submit frame N+1 until frame N's GPU work is done). → [Submissions handed to the GPU in segments](../reference/frame-rate.md#submissions-handed-to-the-gpu-in-segments)
 - the submit thread's CPU samples dominated by `mtl_addr` / `mtl_gpu_addr` (vertex buffers in `draw`, uniform buffers in `encode_args`). → [Address to memory pool lookups by binary search](../reference/frame-rate.md#address-to-memory-pool-lookups-by-binary-search)
 - stalls of 30–60 ms about once a second, at 30 fps and at 60 (99th percentile 63–127 ms); the main thread waits 20–34 ms for the render thread once a second. → [Time-constraint computation declared below the thread's peaks](../reference/frame-rate.md#time-constraint-computation-declared-below-the-threads-peaks)

docs/reference/frame-rate.md

@@ -89,6 +89,22 @@ Frame pacing: the host's frame step, engine frame limiters, present intervals an
 
 ## CPU cost and pacing
 
+### Arena buffer ownership on allocation
+
+- **Symptom:** autorelease work takes a measurable share of replay time, with temporary arena-buffer ownership traffic a few times per draw.
+- **Cause:** under ARC an unqualified `id<MTLBuffer> *` out-parameter is `__autoreleasing`, so assigning the arena buffer in `arena_alloc` autoreleases it. Reading `buf.length` also sends an Objective-C message on each allocation and while selecting or trimming arenas.
+- **Fix:** both declarations of `arena_alloc` use a `__strong` buffer out-parameter, matching every caller's strong local. `NvArena` caches its buffer length when created; allocation, reuse selection and idle trimming use that value. Buffer ownership and fence-based reuse remain the same.
+- **Found on:** MacNativeGames commit `0531f28`, ported in [PR #195](https://github.com/seedds/nxnative/pull/195), reports autorelease work falling from 2.63% to 1.73% of replay time; the PR does not identify the game for that measurement. It has not been repeated here.
+- **Check:** `t_ub`, `t_vbuf` and `t_geometry` exercise arena-backed uniform shadows, vertex conversion and geometry scratch on the Mac with Metal validation.
+
+### Registry lock priority inheritance
+
+- **Symptom:** the submit thread waits on the texture / sampler registry lock while a lower-priority registration thread is preempted holding it.
+- **Cause:** game threads register textures and samplers under `g_rmu`, which the submit thread also takes for argument encoding. The previous mutex allowed those owners to delay the higher-priority submitter.
+- **Fix:** `g_rmu` is an `os_unfair_lock`, lending a waiter's priority to its owner. All registration, lookup, cache, diagnostic and test callers use its lock API; the critical sections and lock order stay the same.
+- **Found on:** [The Legend of Zelda: Tears of the Kingdom](../../dm/games/0100F2C0115B6000/README.md), measured upstream in MacNativeGames commit `5f5de5e` and ported in [PR #195](https://github.com/seedds/nxnative/pull/195): in a shrine, game threads at priority 46 delayed the submit thread at priority 97 by about 1.8 ms a frame. Waiting fell from 155 to about 0 ms/s there, with unchanged frame time. Those game measurements have not been repeated here.
+- **Check:** `t_stencil_view`, `t_view_levels` and `t_view_swizzle` exercise production registry lookups and cached views with the new lock on the Mac.
+
 ### Submissions handed to the GPU in segments
 
 - **Symptom:** the GPU sits idle each frame while the host translates the submission (a game that does not submit frame N+1 until frame N's GPU work is done).

docs/reference/memory.md

@@ -270,3 +270,11 @@ The guest address space, heaps, pools, virtual address memory and the allocators
 - **Fix:** `defer_drain` has its own `@autoreleasepool`.
 - **Found on:** [The Legend of Zelda: Breath of the Wild](../../dm/games/01007EF00011E000/README.md): GitHub issue #9 (iPhone 16 Pro Max, handheld), killed in the first shrine. An 8 GB iPhone gives this app about 3.4 GB (resident plus `headroom to the system memory limit` stayed at 3,375 MB); resident memory grew from 2.6 GB at the shrine load until the kill. On the Mac a host without that pool grew 1 MB a frame in the shrine (GPU 3.0 → 4.4 GB in 2,400 frames); with it the shrine stays at 2.7–2.9 GB resident and 1.5–1.7 GB GPU, idle or walking (vmmap at 2.8 GB: about 1.6 GB guest memory, 0.75 GB private textures, 0.17 GB IOAccelerator).
 - **Note:** found on the way: dead texture serials and the fixed-size pool tracking table ([Dead texture serials kept in a ghost table](nvn-objects.md#dead-texture-serials-kept-in-a-ghost-table), [Memory-pool tracking table grows in blocks](nvn-objects.md#memory-pool-tracking-table-grows-in-blocks)).
+
+### GPU readback without an autorelease pool
+
+- **Symptom:** GPU memory grows with repeated `nvnTextureReadTexels` calls on a long-lived game thread, even after each readback completes.
+- **Cause:** the GPU path in `mtl_read_texels` runs outside a submission's autorelease pool. Metal's temporary command buffer and blit encoder keep the readback buffer alive in the calling thread's undrained pool.
+- **Fix:** `mtl_read_texels` encloses texture resolution, readback encoding, the GPU wait and the CPU copy in its own `@autoreleasepool`. The guest-memory path needs no Metal objects.
+- **Found on:** [The Legend of Zelda: Breath of the Wild](../../dm/games/01007EF00011E000/README.md), measured upstream in MacNativeGames commit `635fb31` and ported in [PR #195](https://github.com/seedds/nxnative/pull/195): about 130 readbacks a second retained 3.7 GB of GPU memory after 30 minutes. Those game measurements have not been repeated here.
+- **Check:** `make -C dm/gfx/tests GAME=010015100B514000 check_t_readback` checks literal GPU and CPU readback pixels and observes the real readback buffer through a weak reference while the caller's outer pool remains open. Removing only the local pool leaves that buffer alive and fails the lifetime check.