Repository navigation
metal: per-draw translation cache (skip capture and recompile on a hit) - #506
RINNECODER wants to merge 3 commits into
Conversation
ANYPS5_AGC_CAPTURE=<dir> records one frame the Metal AGC driver executes: the pre-frame guest memory (resident GPU-visible ranges, LZ4 chunks), each queue's state, every submission with its shader registry and the guest pages that changed before it ran, and the scanout pixels the game showed. Hooks sit at the submission boundary (queue worker loop) and in Present. anyps5_agc_replay maps the capture at its guest addresses, feeds the recorded command buffers through the same driver K times and reports worker CPU ms, worker instructions, wall ms, GPU busy ms and the image hash, with --expect-hash / --expect-png regression checks and --dump-png. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
… mid-frame The replay hook now waits until a submission is the next one in the capture's global order before applying its page delta (5 s escape hatch, counted and reported). The capture records a range that appears after the snapshot with its contents before the submission that first sees it. The residency filter's reliance on MINCORE_PAGED_OUT is documented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
The Metal draw path captured and recompiled every stage of every draw, then copied each RecompileResult. Draws now look up a per-draw cache first, the Metal counterpart of the Vulkan driver's DrawLookup: - key: every register-derived input of BuildDrawRecompileRequest (shader snapshot identity, user data, stage and vertex info, pixel info, mesh/tessellation state, the draw parameters the recompile reads), stored whole and compared exactly, not only by hash; - validation: the guest dwords the resource capture read (descriptors, SRT tables, everything it followed), read again through GuestMemory and compared on every lookup, so changed descriptor or constant contents miss; up to 4 variants per key, most recently hit first; - a capture that met unmapped or partly mapped memory is never cached (ShaderMemory::Partial), since its result depends on the mapping. A hit reuses the shared RecompileResults (rect-list stages included); deferred jobs share them instead of copying. FoldDrawOffsets still runs per draw. ANYPS5_METAL_DRAW_CACHE=0 turns the cache off; ANYPS5_METAL_DRAW_CACHE_STATS=1 prints its counters. Test: metal_draw_translation_cache (changed descriptor, constant or key words miss; restored bytes hit their variant). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7d072bf98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (auto& range : capture.ranges) { | ||
| const auto begin = range.guest & ~(pageSize - 1); | ||
| const auto end = (range.guest + range.bytes + pageSize - 1) & ~(pageSize - 1); | ||
| void* mapped = mmap(reinterpret_cast<void*>(begin), end - begin, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); | ||
| if (mapped == MAP_FAILED) Fail("cannot map guest memory"); | ||
| if (mapped != reinterpret_cast<void*>(begin)) { | ||
| std::fprintf(stderr, "note: guest range 0x%llx (+0x%llx) is in use in this process; mapped at %p\n", | ||
| static_cast<unsigned long long>(range.guest), static_cast<unsigned long long>(range.bytes), mapped); | ||
| range.host = static_cast<std::byte*>(mapped) + (range.guest - begin); | ||
| } else { | ||
| range.host = reinterpret_cast<std::byte*>(range.guest); | ||
| } | ||
| ranges.push_back({range.guest, {range.host, range.bytes}, range.writable}); |
There was a problem hiding this comment.
When a guest range is first observed during Deltas, AgcFrameCapture.mm emits it as a new Range/Chunk immediately before that submission, but the loader collects every such record and this loop maps all of them before replaying the first submission. Consequently, any capture where the title allocates, unmaps, or reuses GPU memory during the recorded frames replays earlier submissions with memory that did not yet exist (and potentially with later contents), silently invalidating output and benchmark results. Preserve the range's activation point and add/remove it at the corresponding submission rather than configuring every captured range up front.
Useful? React with 👍 / 👎.
| ++presents; | ||
| if (phase == Phase::Done || phase == Phase::Waiting || buffer == nullptr) return; | ||
| // Present k scans out flip k: the presents of the recorded flips are target .. target + frames - 1. | ||
| if (presents < target || presents >= target + frames) return; |
There was a problem hiding this comment.
If one of the target presents has buffer == nullptr (for example, a blanked output), this increments presents and returns without emitting a Display or incrementing displays. Later presents eventually fall outside target .. target + frames - 1, so displays can never reach frames; the End record is never written and the replay tool rejects the permanently incomplete capture. Either record/skip the null frame while advancing completion or avoid consuming its target present index.
Useful? React with 👍 / 👎.
|
Closing: no measurable gain on the frame replay; the shader-hit cost it targeted is addressed by #507. |
Depends on #505 (the AGC frame replay tool; this branch is cut from it, so the first two commits are #505's).
What
A per-draw translation cache on the Metal draw path, the counterpart of the Vulkan driver's
DrawLookup. Every draw used to capture and recompile each stage and then copy eachRecompileResult. Now a draw looks up a cache first:BuildDrawRecompileRequest: shader snapshot identity, user data, vertex and pixel stage info, mesh and tessellation state, and the draw parameters the recompile reads. The key is stored whole and compared exactly, not only by hash.GuestMemoryand compared on every lookup. Changed descriptor or constant contents therefore miss. Each key keeps up to 4 variants, most recently hit first.ShaderMemory::Partial), because its result depends on the mapping and not only on the bytes.RecompileResults, rect-list stages included. Deferred jobs share them instead of copying.FoldDrawOffsetsstill runs per draw.ANYPS5_METAL_DRAW_CACHE=0turns the cache off;ANYPS5_METAL_DRAW_CACHE_STATS=1prints its counters.metal_draw_translation_cachechecks that changed descriptor, constant or key words miss, that restored bytes hit their variant again, and that unreadable memory misses. A mutation that drops the byte comparison turns it red.Measured (frame replay, live 3D map capture, 11 CBs)
Replay-safety rules applied: no game running, one replay process at a time, each run a separate process (
--repeat 2: warm-up plus one timed repeat), 10 runs, 2 s pauses. The machine was loaded by other builds (load 7-12), so spreads are wider than the replay's usual 1.2%.An interleaved A/B on one binary (cache off/on, 10 pairs) ran on an even busier machine. It gave a paired median of -3.5% worker instructions, with an IQR of -18% to +9%: inconclusive.
sampleprofiles of the worker over the timed repeats show that capture and recompile are already cheap on a warm replay (the recompile memo and metal: per-draw shader conversion lookup without key rebuilds or MSL copies #503): with the cache off they are about 5% of the worker's active time, and the draw path outsideMetalDraw::Drawis 3.7%. With the cache on, that share falls to about 3%. Most of the remaining per-draw cost is insideMetalDraw::Draw:ConvertToMetalhit path: about 40% ofDraw(conversion key build, lookup, andReturned()copying theResultand the guestRecompileResultper draw);textureKeys): about 25-30%;Bindingsand pipeline construction: about 10% each.Correctness
--expect-hash(reference hash): 8/10 passes on the baseline and 8/10 plus 10/10 (A/B) with the cache. Every failure, on both builds, is the known GPU-order flicker: 1-2 pixels at max delta 3 (the same pixel at 508,770 in all of them).--expect-png(tools: AGC frame capture and replay benchmark #505's reference, max 8 pixels): passes, 2 pixels differ at max delta 3.metal_draw_translation_cachepasses (shader-bridge build).Upstream draw-cache PRs
Upstream boykopovar#5906, boykopovar#5542, #494 and boykopovar#6093 (and #493, already synced) change only the Vulkan backend (
Driver/Draw,Dispatch,Recorder,ShaderResources,GuestBufferMemory,VulkanDevice). None of those files are compiled into the Rosetta/Metal driver, so they cannot change these numbers.perf/upstream-draw-cache, with test-registration and one-line conflict resolutions. That branch is pushed but has no PR: it is only syntax-checked (3 of 6 touched files; the rest need an x86_64 Linux/Windows build).Review findings
tools/agc-replay/AgcReplay.mm: ranges first seen mid-frame are mapped before the first submission instead of at the submission that introduced themAgcFrameCapture.mm: a target present with no buffer consumes a target index without a Display, so the End record is never written🤖 Generated with Claude Code
https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj