Repository navigation
perf(agc): compare sampled snapshots outside the write watch only after a write-watch mark - #5892
Conversation
063c45a to
1e5d3ab
Compare
|
Independent confirmation of the third commit (ea8a7b9, sampled snapshots over untracked memory), plus a test for it. I had written the same change for ASTRO BOT (PPSA21564) before seeing this PR, and only measured that commit, not the other three. Host: Windows 11 Pro 26200, Ryzen 7 5800X3D, Radeon RX 9070 XT (driver 32.0.23033.1002), WinLibs GCC 15.2.0, Release. Effect on ASTRO BOT's title screen
A test It is in SoRadGaming/AnyPS5@f75234e9d, as an
Results:
My version takes the key before the snapshot is read and stores it with a new entry, unless
AI-assisted (Claude Code); the builds and runs are on my machine. |
CnRJay
left a comment
There was a problem hiding this comment.
These four commits are separate changes to three libraries. CONTRIBUTING asks for one topic per PR, and here it matters beyond form: each part has a different risk and a different reviewer.
- Commit 3 (snapshot compares over untracked memory) is the clear win: 44 → 11 ms p50 here. SoRadGaming independently confirmed it on ASTRO BOT with a test. On its own it would be easy to review against #4878's
compareUntrackedstamp, which it reuses. - Commit 4 (a per-thread cache of freed blocks behind the driver's
operator new/delete) has the widest reach: it replaces the global allocation operators oflibSceAgcDriveron Windows. It needs a native TLS slot,--exclude-symbolsso other libraries don't bind to the operators through the import library, and a size-class choice based on_msize. That deserves its own PR and its own look at blocks that cross module boundaries (allocated in one.prx, freed in another), plus thread exit (what happens to a thread's cached blocks when the thread ends). - Commit 2 (latest pending writer, and superseding aliased units) changes which results
FindPendingreturns and which get stored. That is a correctness change to the storage write-back that every title relies on, andAPS5_NO_ALIAS_SUPERSEDE=1is a new switch to keep the old behaviour. Maintainers have asked for that kind of fallback switch to stay offmain. - Commit 1 (AudioOut per-port locks,
THREAD_PRIORITY_HIGHEST,APS5_AUDIO_LATENCY_MS) is audio, with a new tuning variable.
Splitting them in this order (3, then 4, then 2, then 1), each with its own Tested section, would let commit 3 land quickly. The others would then get the scrutiny they need. The frame-rate table already isolates commits 3 and 4 through their switches, so that evidence carries over.
1e5d3ab to
3379246
Compare
|
@CnRJay thanks, split as suggested. This PR now carries only the sampled-snapshot change. The allocator, storage write-back and audio commits are ready on their own branches and will follow in that order once this one lands; my account can have only one open PR until one is merged. The storage write-back one no longer has @SoRadGaming thanks for the test and the store-at-creation version. I took your commit as is, with your authorship, and only rebased it onto current |
|
Checked the key logic against The only conflict with |
3379246 to
5121acc
Compare
…er a write-watch mark On Windows every direct host import leaves the write watch, and a sampled texture snapshot over it was compared with guest memory at every lookup: its generation stays 0, so UnchangedSince never holds. ASTRO BOT (PPSA21564) compared about 31 GB per 10 s at the title. The lookup now keys such a snapshot by UntrackedHint, NewestStamp, ForgetSerial and the recorder's newest write note, taken before the snapshot is read or compared, as StorageTexture::compareUntracked does, and skips the compare while the key is unchanged and no recorded work writes the surface. The byte compare still decides after any mark. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
5121acc to
404075b
Compare
…er a write-watch mark (PR boykopovar#5892)
What
Split as asked in the review: this PR now carries only the sampled-snapshot change. The audio, storage write-back and allocator commits will come as separate PRs, in the order CnRJay suggested (allocator, storage write-back, audio), once this one is merged. My account has no merged PR yet, so the contributing rules allow only one open PR at a time.
perf(agc): compare sampled snapshots outside the write watch only after a write-watch mark. The commit is SoRadGaming's (f75234e, authorship kept), offered in their comment above. It replaces my earlier version of the same idea, because it:The only change I made is rebasing it onto current
main: the[textures]counter line now prints both the image-table counters frommainand the new untracked-snapshot counters.On Windows every direct host import leaves the write watch. A sampled texture snapshot over that memory has no write generation, so every cache hit compared the whole snapshot with guest memory. In Cat Quest III's overworld that was about 63,000 compares (150 GiB) every five seconds, two thirds of the queue worker. The snapshot is now keyed like
StorageTexture::compareUntracked(#4878):It is compared again only when that key changes.
Tested
Windows 11 Pro 26200, MinGW GCC 15.2.0, NVIDIA GeForce RTX 5070 (driver 616.92).
agc_driver_untracked_snapshot_hint(the new test) and theagc_driver_partially_resident*tests: 3 of 3 pass; the new test passes 20 of 20 repeated runs.Cat Quest III (PPSA15065): overworld, standing still after loading the save,
APS5_TRACE_SECONDS=240:20, warm caches, one build, same scene in both runs.APS5_DBG_UNTRACKED_MODE, not in this PR) turns the key off (0, as onmain) or on (1, this commit).compareUntrackeddoes, measured 8.98 ms p50, within noise of this commit, so I left it out.mainbehaviour)Summary lines (
APS5_PAD_INPUT_FILEremoved):{"commit":"6786710dec77","dirty":true,"env":{"APS5_AUDIO_LATENCY_MS":"100","APS5_DBG_UNTRACKED_MODE":"0","APS5_TRACE_SECONDS":"240:20"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","resolution":"1920x1080","window":{"spec":"seconds 240:20","first_present":15221,"ms":20021.077},"presents":545,"frame_ms":{"p50":36.304,"p90":39.931,"p99":46.441,"max":52.634},"gpu_busy_ms_per_present":2.271,"worker_busy_pct":{"queue worker 0x0":99.543},"top_phases_ms_per_frame":{"Driver.Packet":35.987,"Driver.Packet.draw":35.138,"Driver.Draw":35.075,"Driver.Draw.draw_and_resource_release":31.54,"Graphics.Draw":30.857,"Graphics.Draw.draw_and_resource_release":29.997,"Driver.Draw.shader_compile_and_link":2.296,"Shader.DrawStage":1.448},"trace_dropped":true,"trace":"aps5-trace-6786710dec77-19516.json.gz"} {"commit":"6786710dec77","dirty":true,"env":{"APS5_AUDIO_LATENCY_MS":"100","APS5_DBG_UNTRACKED_MODE":"1","APS5_TRACE_SECONDS":"240:20"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","resolution":"1920x1080","window":{"spec":"seconds 240:20","first_present":27133,"ms":20000.03},"presents":2167,"frame_ms":{"p50":9.075,"p90":10.31,"p99":12.025,"max":18.678},"gpu_busy_ms_per_present":2.302,"worker_busy_pct":{"queue worker 0x0":98.669},"top_phases_ms_per_frame":{"Driver.Packet":8.834,"Driver.Packet.draw":8.135,"Driver.Draw":8.088,"Driver.Draw.draw_and_resource_release":5.354,"Graphics.Draw":5.21,"Graphics.Draw.draw_and_resource_release":4.58,"Driver.Draw.shader_compile_and_link":1.593,"Shader.DrawStage":0.888},"trace_dropped":true,"trace":"aps5-trace-6786710dec77-35268.json.gz"}The traces (
aps5-trace-6786710dec77-19516,-35268) are attached in a comment.SoRadGaming's ASTRO BOT (PPSA21564) numbers and test results are in their comment above.
Checklist
main; no other open PR implements the same functionsdocs/dev/debt/libSceAgcDriver.mdchange)NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code