Skip to content

feat(memory): track changes to mapped address ranges - #1360

Closed
TibixDev wants to merge 3 commits into
boykopovar:mainfrom
TibixDev:pr/mapping-change-history
Closed

TibixDev wants to merge 3 commits into
boykopovar:mainfrom
TibixDev:pr/mapping-change-history

Conversation

@TibixDev

@TibixDev TibixDev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Status after review

Do not land range history without its first consumer.

Audited the actual compare commits: this head contains the two targeted-lease/range-update commits from #1324 and the history commit. Its new validation API is used by its tests, with no production history consumer in this PR. That remains a valid blocker.

Main now has a range invalidator for page states and a GPU-map observer for imports. These are not automatically equivalent to retained-history queries, but a proposed import consumer must show why the existing notifications are insufficient before adding another 1,024-entry mechanism. #1366 was rebuilt without this API, so it is no longer a reason to land it. If #1361 is revived with measured benefit, its first real history consumer should be developed and reviewed together with the history implementation. No mechanical adoption of the suggested rebase or standalone API push.

What

Record a bounded history of changed guest address ranges. A cache can keep an unchanged range across unrelated mapping changes. Overlapping changes and expired history reject the proof. Record changes during main-image registration too.

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

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 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on 58a1bd66. Kept both mapping-history and upstream time-conversion test registration, including Windows paths. The upstream mprotect tests are also retained.

Fedora 44 Release: guest_memory_tests, guest_mapping_history_tests, guest_time_conversion_tests build; the targeted tests and contribution checks pass. Windows CI is pending.

@TibixDev
TibixDev force-pushed the pr/mapping-change-history branch from b067d5e to 7fe5e9c Compare October 8, 2026 07:23
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 8, 2026
@rmz-24

rmz-24 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Windows run: Windows 11 Pro 25H2 (10.0.26200.9457), RTX 5060 Ti (NVIDIA 617.42), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON, full build plus libs. 7fe5e9c cherry-picked onto current main d70b899 (applies cleanly).

  • guest_mapping_history and agc_driver_graphics (which carries the GuestAllocations.cpp cases) pass.
  • Full ctest: 511/511.

AI-assisted: yes

@blawesom

Copy link
Copy Markdown
Contributor

Hi! This PR currently conflicts with main, so I rebased its 3 commits onto the latest main. This one interleaved heavily with main's newer GuestAllocations work, so the rebased branch combines both:

  • replaceRange: this PR's RangeReplacement/fragments machinery is kept, with main's newer gpu parameter threaded through the insert lambda and Range literals.
  • GuestAllocationsAcquireRange: renamed to _nid_no_patch as in the PR, alongside main's AcquireAll.
  • UploadFinish import lookup: the PR's targeted-lease logic is kept, with its importAllocation call adapted to main's current 4-argument signature.
  • GuestAllocationsEnd: the PR's publishChanges (which now bumps the generation itself) replaces main's raw fetch_add, with ended loaded after it for main's gpu-map observer.
  • registerImage: main's newer multi-image refactor is kept — it supersedes this PR's main-image-only registration export.
  • The tests were updated for main's gpu parameter on Mutation::Add/Protect.

It builds, and guest_mapping_history_tests plus the affected agc suites (bda_device, graphics, mesh, shader_memory, write_tracking) all pass locally.

The rebased branch is available to pull:

git fetch https://github.com/blawesom/AnyPS5 rebase/pr-1360
git reset --hard FETCH_HEAD

Feel free to ignore this if you'd rather rebase yourself.

@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.

Thanks for the work here. This can't be merged as is:

  • It is stacked on #1324, #1357 and #1359. All three are still open and conflict with main, so nothing here can land before them.
  • This head also conflicts with main (GuestBufferMemory.cpp, libc/CMakeLists.txt, GuestAllocations.hpp, GuestAllocations.cpp). The conflicts are in replaceRange and the host-import lookup, so they need the author, not a mechanical rebase (the rebase suggested in the thread merges both versions of that logic and hasn't been pushed here).
  • GuestAllocationsValidateMapping_nid_no_patch and the 1024-entry history have no caller in this PR; only the new test uses them. Please land it together with its first consumer (#1361/#1364/#1366), or say which existing code uses it.
  • GuestAllocationsAcquireRange_nid_no_patch is wired into GuestBufferMemory.cpp as a perf change. Please name a title/scene where it helps: on #1324 @oneandonlydean measured no frame-time change and the question is still unanswered.

Please rebase once the base PRs are merged (or fold the stack so each PR keeps one topic) and we will review it again.

@TibixDev

Copy link
Copy Markdown
Contributor Author

Review-pass update:

Audited the actual compare commits: this head contains the two targeted-lease/range-update commits from #1324 and the history commit. Its new validation API is used by its tests, with no production history consumer in this PR. That remains a valid blocker.

Main now has a range invalidator for page states and a GPU-map observer for imports. These are not automatically equivalent to retained-history queries, but a proposed import consumer must show why the existing notifications are insufficient before adding another 1,024-entry mechanism. #1366 was rebuilt without this API, so it is no longer a reason to land it. If #1361 is revived with measured benefit, its first real history consumer should be developed and reviewed together with the history implementation. No mechanical adoption of the suggested rebase or standalone API push.

I have updated the PR description to match this status. The PR remains open; the outstanding review requirements are not being marked resolved. AI-assisted audit.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing for now: no title is known to need it. With the queue this large we're closing stalled perf PRs that don't show a gain in a title; reopen it with before/after numbers from a title scene (the summary lines from the performance report form) if you pick it up again.

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

Labels

agc libSceAgc* build libc needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants