Skip to content

perf(memory): restrict registry work to affected ranges - #1324

Open
TibixDev wants to merge 2 commits into
boykopovar:mainfrom
TibixDev:pr/targeted-allocation-leases
Open

TibixDev wants to merge 2 commits into
boykopovar:mainfrom
TibixDev:pr/targeted-allocation-leases

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Status after review

Awaiting a title profile showing registry cost.

Rechecked the motivation against current main. Host imports now have eager GPU-map observation and later span/lifetime fixes. The microbenchmark with empty host callbacks does not model those costs. The existing two-title review measured only −2.3% to +1.0%, within noise; no new isolated title gain has been established.

This pass removed the artificial downstream dependency: #1345, #1357, #1359 and #1366 now have standalone changes on main and do not require these targeted leases. A rebase of this optimization should follow a title/scene profile showing remaining full-registry work, with real callbacks included. No code push or renewed performance claim here.

What

Limit allocation lookup and mapping updates to the requested range. Host-import lookups lease one containing allocation when the registry is current. Protection and unmap changes prepare only affected fragments, then commit after the host operation succeeds.

Tested

Earlier branch-specific tests and measurements remain in the discussion. They have not been rerun on a redesigned current-main version of this PR. The review audit and shared NGS-assisted title exploration do not establish that this head is merge-ready. No full Linux/Windows platform CI was dispatched in this pass.

Checklist

  • Current-main implementation and outstanding review requirements complete.
  • Isolated title benefit / reference validation established where required.
  • Depends on: none.
  • AI-assisted: yes.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Could you measure main against main plus this PR alone, ideally frame time in a title? Perf PRs need before/after numbers for the change itself (#218); the 2,354 → 0.764 ms figure is a component run on the combined fork. The same goes for #1309, #1310, #1315, #1316, #1330 (against main plus #157), #1331, #1334 and #1335: their figures are component or combined-branch runs, or there are none.

Use a locked ordered lookup for one containing readable allocation when the import registry is current. Recheck the registry generation after acquiring the lease, and retain the full reconciliation path when mappings change.

Tests cover bounds, gaps, read-only and inaccessible ranges, address reuse, changed protection, pinning the requested allocation, and releasing an unrelated allocation. agc_driver_graphics passes on Fedora 44. The preceding Skyrim profile attributed 76% of sampled CPU cycles to full allocation leases.

Written with AI assistance.
Bound allocation coverage and availability lookups, stage only changed mapping fragments before host operations, and retain atomic registry updates on host failure. Descriptor uploads acquire a targeted lease when an unchanged registry has no cached import.

Validation: agc_driver_graphics and guest_memory passed, including host-failure rollback, gaps, split mappings and unrelated leases. A 20000-entry mapping microbenchmark dropped from 2354 ms to 0.764 ms for 1000 localized operations; this is not a game benchmark. AI-assisted implementation.
@TibixDev
TibixDev force-pushed the pr/targeted-allocation-leases branch from a19d598 to 1daa7f6 Compare October 8, 2026 06:05
@TibixDev

TibixDev commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on e9846679 with no production changes.

Measured upstream e9846679 against the same revision plus this PR (1daa7f69). Fedora 44, GCC 16 Release, Ryzen 5 5600, CPU 3. Six processes per variant in ABBA × 3 order. Each process has one warmup and nine timed batches: 128 lookups or 64 mapping updates per batch. No game or build ran during timing.

The same fixture calls RegisteredReadableCovers, Mutation::Protect, and Mutation::Unmap followed by Add. It registers 1, 1,000, or 20,000 disjoint 64-byte ranges. Requests rotate through the registry. Host-operation callbacks are empty, so the figures measure registry work only. Both variants verify permissions, allocation release, and matching result counts.

Ranges Operation Upstream median (µs) PR median (µs) Upstream range PR range
1,000 readable 8.1769 0.0358 7.7434–10.9490 0.0347–0.0625
1,000 protect 28.6280 0.1572 27.8199–51.5058 0.1357–0.2791
1,000 unmap 34.3257 0.1808 32.9848–57.8596 0.1731–0.3402
20,000 readable 508.6185 0.0676 483.5950–651.1650 0.0669–0.1020
20,000 protect 744.2820 0.1814 706.8740–783.7310 0.1781–0.3431
20,000 unmap 1048.7200 0.2327 973.9930–1140.5500 0.2256–0.4195

The one-range case has overlapping timing ranges; no gain is established there. The larger cases show that unrelated registry entries no longer dominate these operations. This does not measure Vulkan import creation or title frame time.

Graphics and guest-memory tests pass, including range boundaries, split mappings, permissions, unrelated leases, address reuse, and failed-host-operation rollback. Contribution checks pass. Windows was not tested. AI-assisted.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

I measured this alone in two titles: frame time didn't change beyond run-to-run noise (-2.3 % to +1.0 % across three scenes in one, none in the other). Which title does it speed up?

@popcoder565

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 1daa7f69, merged onto main d70b899 together with #2039, #2193 and #2247.

No title was run.

🤖 Addressed by Claude Code

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked on current main: build + ctest green locally (308/308 in the touched components); the new tests need the new API, and I found no logic problems in the registry changes (iterator ranges, Commit(), last computation in Unmap, boundary cases).

  • The open question from @oneandonlydean is still unanswered: his measurements in two titles show frame time unchanged (-2.3 % to +1.0 %, run-to-run noise). The numbers in the description are registry microbenchmarks with empty host callbacks, not before/after numbers for a title (CONTRIBUTING, perf PRs). Please name a title/scene where main vs main + this PR alone shows a measurable difference (frame time or the time spent in these functions in a profile).
  • If there is no measurable gain in a real title, this adds complexity to the registry without a benefit and should be dropped, or reopened when a case shows up.

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still open after the last review:

  • Perf PRs need before/after numbers for the change alone in a real title (CONTRIBUTING). @oneandonlydean measured this PR alone in two titles and frame time did not change beyond noise (-2.3 % to +1.0 %). The figures in the description are registry microbenchmarks with empty host callbacks. Please name a title/scene where main vs main + this PR shows a measurable difference (frame time, or time spent in these functions in a profile).
  • The branch now conflicts with main; it needs a rebase (and a rerun of guest_allocations/agc_driver_graphics) once the benefit is shown.

Without a measurable gain in a real title this adds complexity to the registry for no benefit; it can be reopened when a case shows up.

This was referenced Oct 15, 2026
@github-actions github-actions Bot added perf Performance change perf numbers missing perf pull request without a before and after summary line labels Oct 18, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 20, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* libc needs rebase Conflicts with main, rebase needed perf numbers missing perf pull request without a before and after summary line perf Performance change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants