Repository navigation
perf(libc): start the guest allocation overlap queries at the range that touches the address - #6321
mariuszkreft wants to merge 2 commits into
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Independent Windows CPU validation of head I compiled each version's complete, unchanged Correctness: 177,125 deterministic scenarios per execution, zero mismatches on each of four executions (base, PR, PR, base). Registry sizes 0, 1, 32, 512, 4,096 and 16,384; non-overlapping ranges have 4/8/12/16 KiB lengths on a 32 KiB stride. For each size, 12,000 random valid queries (seed 6321) check both Timing: these are deliberately worst-position free queries beyond the last registration, excluding setup, mutations and exceptions. Five batches of 3,000 calls per function per registry size per execution; table is the median of ten batches per version, ns/call, base -> PR:
The growing-registry result supports the intended removal of a linear prefix scan on Windows too. Empty/single-range registries cost a few more nanoseconds, so this is not a universal speedup. The timing workload is synthetic, cache-warmed, free-only and near the end; it is not a representative frame workload or an FPS result. Attached: probe source, reproducible Windows runner (set Limits: no full emulator build, project CTest/GuestMemory suite, hardware GPU/title run, concurrency stress, OS mappings, zero-sized registrations, invalid-input validation, lease-acquire benchmark or composition with current main/other PRs tested. This independently validates the two overlap-query changes, not the whole PR or resolution of #4634 on a title. |
|
Minimal cleanup for the seven current Conventions failures on head The patch removes only the four prose-comment lines before --- a/core/libs/prx/libc/src/GuestAllocations.cpp
+++ b/core/libs/prx/libc/src/GuestAllocations.cpp
@@ -355,10 +355,6 @@
}
}
-// The lowest registered range that can touch `address`: the one that starts at or
-// after it, or the one before it when that one reaches into the address. Starting a
-// scan at lower_bound alone misses a range the address falls inside, which is exactly
-// the overlap these queries have to report.
decltype(registry().ranges)::iterator firstRangeTouching(std::uintptr_t address) {
auto& ranges = registry().ranges;
auto it = ranges.lower_bound(address);
--- a/core/libs/tests/GuestMemory.cpp
+++ b/core/libs/tests/GuestMemory.cpp
@@ -2330,9 +2330,6 @@
}
#endif
-// The overlap queries must start at the range that reaches into the queried address,
-// not at the first range whose base is above it: a query landing inside a registered
-// range is the case these two functions exist to report.
static void CheckOverlapQueriesSeeRangesStartingBelowTheAddress() {
constexpr std::size_t page = 0x4000;
constexpr std::size_t many = 4096;I reproduced all seven This checks the affected file delta, not the complete upstream tree, PR template or hosted CI. No native tests/benchmarks were repeated for this comment-only change; my earlier CPU probe remains evidence only at its recorded head. No source-checkout edit, push to your branch or separate PR. The author still needs to apply the patch and rerun the hosted check. |
|
Tested on Linux x86-64 (Release build) at a52b723, which merges cleanly with main 03c4cb5:
The new test also passes on main, since main returns the same results, just slower. To show the speedup itself, here is a cost test modelled on
The patch below also removes the 7 comments, so Patch against a52b723diff --git a/core/libs/prx/libc/src/GuestAllocations.cpp b/core/libs/prx/libc/src/GuestAllocations.cpp
index a6ec3cd96..824cfdec3 100644
--- a/core/libs/prx/libc/src/GuestAllocations.cpp
+++ b/core/libs/prx/libc/src/GuestAllocations.cpp
@@ -355,10 +355,6 @@ void GuestAllocationsRequireUnpinned_nid_postfix(void* mutation, const void* poi
}
}
-// The lowest registered range that can touch `address`: the one that starts at or
-// after it, or the one before it when that one reaches into the address. Starting a
-// scan at lower_bound alone misses a range the address falls inside, which is exactly
-// the overlap these queries have to report.
decltype(registry().ranges)::iterator firstRangeTouching(std::uintptr_t address) {
auto& ranges = registry().ranges;
auto it = ranges.lower_bound(address);
diff --git a/core/libs/tests/GuestMemory.cpp b/core/libs/tests/GuestMemory.cpp
index eb5ba5948..cd2ae57ac 100644
--- a/core/libs/tests/GuestMemory.cpp
+++ b/core/libs/tests/GuestMemory.cpp
@@ -2328,11 +2328,46 @@ static void CheckMappingCostFollowsTheMappingCount() {
Require(large <= small * 48);
Require(sceKernelReleaseDirectMemory(phys, page * many) == 0);
}
+
+static double OverlapQueriesSeconds(std::size_t ranges) {
+ constexpr std::size_t page = 0x4000;
+ constexpr std::size_t queries = 2000;
+ constexpr std::uintptr_t first = 0x7e0000000000ull;
+ for (std::size_t index = 0; index < ranges; ++index) {
+ GuestAllocations::Mutation mutation;
+ mutation.Add(reinterpret_cast<void*>(first + page * 2 * index), page, true, true, false);
+ }
+ const auto gap = reinterpret_cast<const void*>(first + page * 2 * (ranges - 1) + page);
+ const auto threadSeconds = [] {
+ timespec now{};
+ Require(clock_gettime(CLOCK_THREAD_CPUTIME_ID, &now) == 0);
+ return static_cast<double>(now.tv_sec) + static_cast<double>(now.tv_nsec) * 1e-9;
+ };
+ double best = 1e9;
+ for (int attempt = 0; attempt < 3; ++attempt) {
+ const auto before = threadSeconds();
+ for (std::size_t query = 0; query < queries; ++query) {
+ Require(!GuestAllocations::GuestAllocationsOverlaps_nid_postfix(nullptr, gap, page));
+ GuestAllocations::GuestAllocationsRequireAvailable_nid_postfix(nullptr, gap, page);
+ }
+ best = std::min(best, threadSeconds() - before);
+ }
+ for (std::size_t index = 0; index < ranges; ++index) {
+ GuestAllocations::Mutation mutation;
+ mutation.Remove(reinterpret_cast<const void*>(first + page * 2 * index));
+ }
+ return best;
+}
+
+static void CheckOverlapQueryCostFollowsTheTouchedRanges() {
+ constexpr std::size_t few = 1024;
+ constexpr std::size_t many = few * 16;
+ const auto small = OverlapQueriesSeconds(few);
+ const auto large = OverlapQueriesSeconds(many);
+ Require(large <= small * 4);
+}
#endif
-// The overlap queries must start at the range that reaches into the queried address,
-// not at the first range whose base is above it: a query landing inside a registered
-// range is the case these two functions exist to report.
static void CheckOverlapQueriesSeeRangesStartingBelowTheAddress() {
constexpr std::size_t page = 0x4000;
constexpr std::size_t many = 4096;
@@ -2657,6 +2692,7 @@ int main(int argc, char** argv) {
CheckDirectMemoryWriteWatch();
CheckDirectMemoryWriteWatchFollowsRemovedMappings();
CheckMappingCostFollowsTheMappingCount();
+ CheckOverlapQueryCostFollowsTheTouchedRanges();
CheckDirectMemoryBackingNeedsNoFilesystem();
CheckDirectMemorySharedBacking();
CheckVirtualQueryFindsNeighbours();AI-assisted: yes (Claude Code) |
github-actions
Bot
added
perf
…hat touches the address GuestAllocationsOverlaps and GuestAllocationsRequireAvailable walked the whole range registry from begin(), so every mprotect-sized query paid for the registrations below it. A title that keeps tens of thousands of live 64 KiB direct mappings turns that into queue-worker time: boykopovar#4634's samples put the wait in exactly these two calls. Start both at the lowest range that can touch the queried address: the first one at or after it, or the one before it when that one reaches into the address. A query landing inside a registered range is the case these functions exist to report, so the step back is what keeps the result identical. Measured on this host with 35,000 registered 64 KiB ranges and 20,000 probes mixed across range starts, interiors and gaps: Overlaps 1723.6 ms -> 1.8 ms, RequireAvailable 1672.9 ms -> 5.6 ms, same hit counts (4375 each). Acquire gains a reserve() over the registry size: 982 ms -> 849 ms for 2,000 leases. The added case pins the interior-address case; it fails on a scan that starts at lower_bound alone.
a52b723 to
0533ecc
Compare
|
This pull request is labeled |
|
AI-assisted independent validation of a local refresh of this PR on main The attached PPSA05347, Record of Lodoss War: Deedlit in Wonder Labyrinth, forest sword/jump tutorial: 26.068 FPS main → 27.946 FPS candidate (+7.205% observed). Frame p50/p90/p99: main 36.525/46.630/59.935 ms; candidate 34.471/41.212/54.146 ms. Both reached the same tutorial location in the 190/240-second screenshots and exited cleanly. Ryzen 9 7950X / RX 6650 XT, RADV Mesa 26.2.4, Linux 7.2.9; trace resolution 3840×2160. One candidate-then-main pair, identical starting warm-cache clones, same scripted inputs, pipelined draws and DMA enabled on both, trace window Evidence archive includes both complete traces, original summary lines, Main summary: {"commit": "252cbc29beac", "dirty": false, "env": {"APS5_DRAW_CACHE_ENTRIES": "4096", "APS5_HOST_IMPORT_MIB": "16384", "APS5_PAD_INPUT_FILE": "<path>", "APS5_PIPELINED_DRAWS": "1", "APS5_PIPELINE_DMA": "1", "APS5_SPIRV_OPT": "full", "APS5_TRACE_SECONDS": "200:30"}, "gpu": "AMD Radeon RX 6650 XT (RADV NAVI23)", "driver": "radv Mesa 26.2.4-arch1.1", "os": "Linux 7.2.9-arch1-1", "resolution": "3840x2160", "window": {"spec": "seconds 200:30", "first_present": 9518, "ms": 30036.52}, "presents": 783, "frame_ms": {"p50": 36.525, "p90": 46.63, "p99": 59.935, "max": 75.545}, "gpu_busy_ms_per_present": 2.195, "worker_busy_pct": {"queue worker 0x0": 29.237}, "top_phases_ms_per_frame": {"Driver.Packet": 11.205, "Graphics.Draw": 5.481, "Graphics.Draw.validate": 3.77, "Recorder.Sync": 1.926, "Graphics.Draw.draw_and_resource_release": 1.699, "VideoOut.Flip": 1.658, "Driver.Present": 1.622, "VideoOut.Flip.present": 1.622}, "trace_dropped": false, "trace": "aps5-trace-252cbc29beac-995367.json.gz"}Candidate summary: {"commit": "0e448213e27e", "dirty": false, "env": {"APS5_DRAW_CACHE_ENTRIES": "4096", "APS5_HOST_IMPORT_MIB": "16384", "APS5_PAD_INPUT_FILE": "<path>", "APS5_PIPELINED_DRAWS": "1", "APS5_PIPELINE_DMA": "1", "APS5_SPIRV_OPT": "full", "APS5_TRACE_SECONDS": "200:30"}, "gpu": "AMD Radeon RX 6650 XT (RADV NAVI23)", "driver": "radv Mesa 26.2.4-arch1.1", "os": "Linux 7.2.9-arch1-1", "resolution": "3840x2160", "window": {"spec": "seconds 200:30", "first_present": 9793, "ms": 30021.747}, "presents": 839, "frame_ms": {"p50": 34.471, "p90": 41.212, "p99": 54.146, "max": 63.89}, "gpu_busy_ms_per_present": 2.189, "worker_busy_pct": {"queue worker 0x0": 28.939}, "top_phases_ms_per_frame": {"Driver.Packet": 10.344, "Graphics.Draw": 5.025, "Graphics.Draw.validate": 3.373, "VideoOut.Flip": 1.696, "Recorder.Sync": 1.671, "VideoOut.Flip.present": 1.66, "Driver.Present": 1.66, "Graphics.Draw.draw_and_resource_release": 1.64}, "trace_dropped": false, "trace": "aps5-trace-0e448213e27e-900224.json.gz"} |
This branch has not been deployed
5 participants
What
GuestAllocationsOverlapsandGuestAllocationsRequireAvailablewalked the whole guestallocation range registry from
begin(), so every mapping-sized query paid for everyregistration below the queried address. #4634's samples put the
sceKernelBatchMap2threads' time in exactly these two calls, reached from
DoMapDirect, with ~35,000 live64 KiB direct mappings in the registry.
Both now start at
firstRangeTouching(address): the first range at or after the address,or the one before it when that one reaches into the address. The step back is the point — a
query landing inside a registered range is the overlap these functions exist to report, and
a scan that starts at
lower_boundalone misses it.GuestAllocationsAcquirealso reservesthe registry size before filling its vector.
Acquireis unchanged in behaviour: the vector is still rebuilt when its weak_ptr cachewas reset, which a mutation does. Making that cheaper needs the pin test to stop relying
on
use_count(), which is what perf(memory): restrict registry work to affected ranges #1324 is about.Tested
Linux, Ubuntu 24.04, GCC 13.3, Release, full build (
-DBUILD_TESTING=ON) on this branchwip/gta-v-4634-registry-scanoverorigin/main:ctest --test-dir build --output-on-failure -j 12→ 1233/1234 passed.The single failure,
agc_pending_key_stores, reproduces on cleanorigin/mainon thishost: it needs host write protection, and the NVIDIA driver here reports
host imports resolve write protection: no (0 of 4 scratch pages written).guest_memory_testspasses, including the added case that pins the interior-addresscase: with a scan that starts at
lower_boundalone the suite aborts atGuestMemory.cpp:2009.python3 tools/check_conventions.py --base origin/main→ all conventions checks passed.Measured on the same machine, the real translation unit, 35,000 registered 64 KiB ranges
and 20,000 probes mixed across range starts, interiors and gaps:
GuestAllocationsOverlaps×20,000GuestAllocationsRequireAvailable×20,000GuestAllocationsAcquire×2,000Identical results before and after: 4375 overlaps found, 4375 mappings refused.
No title was run. The win is measured on the registry with the title's range count,
which is the cost the profile attributes to these calls; it is not an end-to-end frame-rate
claim. #4634's own title runs belong to whoever owns that dump.
Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository