Repository navigation
Conversation
…index pendingAlias and otherPendingOverlaps scanned every pending storage image on each Refresh. PendingList now keeps its images in an address-ordered range index (RangeIndex) beside the vector: the images starting at an address come back in registration order and the images overlapping a range in order of start, in O(log n + k) instead of O(n). The answers are those of the scans (the registry's membership and each image's range are fixed while it is pending); the new test compares overlap and start queries with the linear scan over random registries. APS5_NO_REFRESH_LOOKUP_CACHE=1 scans as before.
…amped A Refresh of an image with block units, unchanged DCC keys and no pending alias whose surface has no stamp since the image's generation answers unchanged whatever the per-unit state says: no unit is changed, nothing is compared and nothing is borrowed. It no longer builds the per-unit changed list or walks it. APS5_NO_REFRESH_LOOKUP_CACHE=1 builds it as before.
Refresh looks up the host import of the image's range on every call. Each StorageTexture now remembers the import its last lookup returned, with the import table's epoch (bumped when an import is dropped or the device changes) and its insert count. HostImportForCached answers from it, after the same settle and staleness checks and with the same last-use stamps as the full lookup, while the range, the epoch and the insert count are unchanged and the registry has not changed since the imports were reconciled; otherwise it takes the full lookup and remembers its result. A lookup that finds no import is not remembered. APS5_NO_REFRESH_LOOKUP_CACHE=1 takes the full lookup every time.
|
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.
|
|
Windows runs at head |
This was referenced Oct 18, 2026
Open
Contributor
|
AMD on Windows: the tests this PR names pass on an RDNA 3.5 GPU. Radeon 860M (RDNA 3.5, driver 32.0.13058.8001), Windows 11 26300, WinLibs GCC 15.2.0 r7, Release.
AI-assisted |
Contributor
Author
|
@akkadbakkad89 @LeandroLP23 thanks for the Windows runs on NVIDIA, Intel and RDNA 3.5. They are the first non-lavapipe data for this head. The speed A/B (this PR against AI-assisted: yes (Claude Code) |
github-actions
Bot
added
perf
Collaborator
|
@AlxcBTW #6364 (flush-wait-newer-writers) has merged, and #7514 now conflicts with it in
Both change |
12 tasks done
12 tasks done
Contributor
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
StorageTexture::Refreshruns for every storage image a dispatch or draw reads or writes, tens of thousands of times per 10 s in a gameplay scene, and looks up the same facts each time: which images pending in the same memory overlap it, which pending image sits at its address, and which host import holds its range. Three commits, each on by default and each exact (the same answers, only cheaper to get):perf(agc): answer Refresh's pending registry queries from an address index.PendingListkeeps its images in aRangeIndex(newGraphics/include/RangeIndex.hpp) beside the vector: items keyed by[begin, end), ordered by (begin, registration order), overlap queries in O(log n + k) over an implicit balanced tree that holds the largest end of each subtree (rebuilt in O(n) on the first query after a change).otherPendingOverlapsasks the index for images overlapping the range (any image butthis),pendingAliasasks for the images starting at its address. Before, both scanned every pending image under the registry mutex (std::any_ofover the vector; the loop ofpendingAlias). The index is updated where the vector is (push_back,erase,remove).perf(agc): skip the per-unit state of a Refresh that finds nothing stamped. A Refresh of an image with block units, unchanged DCC keys and no pending alias, whose surface has no stamp since the image's generation (GuestMemory::UnchangedSince, the test the code already makes), answers "unchanged" whatever the per-unit state says: no unit is changed, nothing is compared, nothing is borrowed. It no longer allocates the per-unitchangedlist or walks it for pending units. The host-import lookup and everything after the early return stay.perf(agc): remember the host import a storage image's Refresh found. EachStorageTextureremembers the import its last Refresh lookup returned, with the range, the import table's epoch (bumped whenever an import is dropped or the device changes) and a new insert count (bumped whenever an import is added).HostImportForCachedtakes the import mutex and does the Windows settle step and the registry staleness check exactly asHostImportFordoes, and answers from the remembered import when range, epoch and insert count are unchanged and the registry has not moved since the imports were reconciled, with the same last-use stamps as a full hit (findImport's and the explicit one, 2 per lookup); otherwise it runs the full lookup (nowhostImportSettled, the old body ofHostImportForafter the settle) and remembers its result. A lookup that finds no import is not remembered.All three are turned off together by
APS5_NO_REFRESH_LOOKUP_CACHE=1(set at all means off, like the otherAPS5_NO_*switches); with it set the code paths are the old ones (the index is not even filled). I considered two PRs: the index and the lookup cache touch different data and could stand alone, but they are one topic (Refresh's lookups), share the switch, and the only measurements I have are of the stack that has them together, so one PR with the three commits in the order that each builds and passes alone.Why the answers are the same:
[descriptor.baseAddress, baseAddress + guestBytes)and both are fixed at construction, so the range an entry is filed under is the rangeoverlaps()tests.RangeIndex::Overlappingvisits exactly the entries withaddress < end && begin < address + bytes(the predicate ofStorageTexture::overlaps, zero-length queries return nothing as before),StartingAtvisits the entries with that begin in registration order, which is the vector's order for equal begins (an erase and re-push goes behind the others in both).otherPendingOverlapsandpendingAliasonly ask for existence or collect candidates in that order, andpendingAliasstill filters by size and block units as before. Theflushinglist is untouched.quicktrue the code that is skipped only reads and writeschanged, which is all false when no unit is stamped,comparedandpendingChangedstay false, and the alias branch needs a non-null alias; theunchangedreturn is the one the old code reaches.findImport(the import containing the range, by base) plus a stamp. Imports enter the table in one place (the insert count) and leave it inretireImport, the device reset and the shutdown clear (each bumps the epoch), so with epoch and insert count unchanged and the registry not stale the import found by range is the remembered one, and the stamps makelastUseanduseCounterend up as after a full lookup (the eviction rule reads both).Not covered by this PR: equivalence of the whole Refresh is argued from the code and measured by the lab verify runs below (compare mode that is not in the PR); the title speed gain is not established (below).
Tested
b3120a214plus the CI scaffolding commit38f169c31; run 39113200466, test filter^agc_): 709 of 709 passed, among themagc_range_index(0.51 s) andagc_host_import_retire(0.01 s). Not run on Windows.agc_range_index(tests/RangeIndex.cpp, new executable): the index against a brute-force list over random registries (four seeds, address spaces from 16 to 2^40 bytes, 3000 to 4000 rounds each of random inserts, removals of a random live entry and inserts at a fixed begin, with four random overlap queries and a start query after every round, and a final drain of all entries); fixed cases for the strict overlap rule at both ends, empty query, zero-length entry, registration order at one begin, a removed entry, a re-registered entry and a query running past the end of the address space.agc_host_import_retire(tests/HostImportRetireHold.cpp) has two new cases: the cached lookup agrees with the full lookup, a repeat hits the remembered import, another range and an empty range are not answered from it, and a cached hit and a full lookup movelastUseby the same amount; and the cache follows the import table: an import added between two lookups (which also moves the registry) leaves the answer equal to the full lookup, a dropped import (the heap grows, the import is replaced by one over both ranges) is not answered from the cache and the cache refills, a cache filled after the drop agrees, and a range outside every registered range answers null and is not remembered. The rewatch of a settled import (Windows only) is not driven by a test; it does not change the import pointer.best10and its successors, a run of lab changes of which the lab switchAPS5_LAB_REFRESH_LEVERis the only difference between the arms. The lab version of the cache is the same as commit 3 here without the switch and without the lab counters; the lab version of the index is commit 1; its quick path is commit 2. Lab arm=1had in addition an unexact variant, the host-import lookup skipped on the unchanged path (it also skips the last-use stamps), which this PR does not contain. Arm=2(the cached lookup) contains the index and the quick path as well, so it is this PR; arm=1is not.Job 95 (A/B/C, trace-digest figures, no profile counters; a: best10, b:
=1index + quick path + skipped lookup, c:=2index + quick path + cached lookup, which is this PR; arm order a b c, c b a, b a c; gameplay is a 180 s walk traced from 170 s, title is 200 s traced from 100 s):Row r3-g-a-2 is a world-load crash and retry (
FAsyncLoadingThread0xc0000005, the title's own load-time crash, not this change); one b row ran out of memory (0.45 GiB free, stopped), so b has two gameplay rows. Gameplay means: a 4.02 FPS (5.45 without the crash row), b 5.80, c 6.93. Title means: a 8.67, b 8.26, c 9.08. These were daytime rows: they ended at 0.5 to 6.4 GiB free with 46 to 56 GiB committed and about 35 browser processes open, which is the memory-noisy state this machine produces gameplay figures in (31 GiB), and the spread inside arm a alone is 1.15 to 5.95 FPS. I read them as: c is ahead of a in every gameplay row against a's rows that did not crash, and the title shows no difference (8.38 and 9.78 in c against 9.27 and 8.07 in a); I do not claim a gain from them. There is no clean A/B of this change on its own.Job 91zz-best12 (best11 against best12, both with
APS5_LAB_REFRESH_LEVER=2andAPS5_LAB_DEPTH_CLEAR_HLE=1; it compares two integration stacks and is not an A/B of this change, listed because the lever is on in both arms and its counters are the largest sample of the cached lookup):Gameplay rows ended at 4 to 6.5 GiB free (noisy), the title rows at 9.3 GiB free (clean). The difference between the stacks is not this change.
Counters that bear on exactness and on how often each lookup is taken (lab builds, per 10 s, the lab counters are not in the PR):
verify2(job 91zz of 17.10, gameplay: each cached answer compared with the full lookup made right after it), last three windows of the row: 131,395 hits and 3,841 misses; 116,834 and 3,725; 122,069 and 4,041; epoch moves seen at a miss 221, 22, 256; verify2 differences 0 (since start), noDISAGREEline, stop reasonprogram-done. Cached mode, title row of the same job: 112,204 hits and 2,641 misses; 111,038 and 2,620; 113,122 and 2,644, no epoch move, 0 differences. In the best12 rows (cached mode only, last window per row): gameplay best11 92,390 hits / 5,037 misses, 108,903 / 4,128, 95,631 / 3,804; best12 180,725 / 7,338, 150,467 / 5,934, 122,903 / 4,141; title 133,818 / 3,178, 134,236 / 4,151, 136,771 / 4,125, 136,252 / 3,464. So 96 to 98 % of the lookups are hits.verify(job 91z of 17.10, gameplay: the old scans run beside the index and the quick answer is compared with the full path): "disagreements 0 (since start)" on every one of the 26 windows of the row, for example 117,271 refreshes, 58,310 index queries and 45,056 entries returned in the last 10 s. Inonmode of the same stack (arm=1, gameplay rows of job 91z), 9,900 to 28,800 of 66,000 to 180,000 refreshes per 10 s took the quick path, and the index answered 32,000 to 96,700 queries per 10 s returning 0.7 to 1.0 entries on average.Refresh's cost, from the hold profile (lab build best10 with the profile switch on, so no lever; gameplay row g1 of job 88, summed over every thread and hold site, per 10 s, spans nested inside
storage-refresh-self; the profile costs two clock reads per span, so its FPS is not comparable):StorageTexture::Refresh1,139 ms over 59,137 calls (19.4 us per call) in the earlier profile; the finer one,rf-host-import(theHostImportForlookup) 519.3 ms over 20,497 calls (25.3 us per call),rf-scan-blocks154.2 ms,wb-windows386.6 ms,rf-alias(pendingAlias) 10.3 ms over 20,497 calls (0.5 us),rf-unchanged-tail10.4 ms over 17,781 calls,refresh-proved18.2 ms over 32,147 calls,rf-decide-loops6.6 ms over 20,497 calls, what remains instorage-refresh-self47.8 ms. The pending-registry scans were cheap to start with (24,000 scans over 2.9 M entries, about 120 per scan). Two things follow, and I want them read as limits of the claim: the span the cache can touch is the biggest one, but the profile does not split it further, and the cached hit still takes the import mutex, so how much of the 25 us is the map descent and how much is waiting for the mutex is not known; and the index and the quick path address spans that were already small (about 10 ms and 7 ms per 10 s). I have no hold profile of a build with the changes, so the reduction of these spans is not measured.Not measured: Windows CI, other GPUs, other titles, the tests with the switch set (the new tests run with the default, the cache test also compares against the full lookup). The
RangeIndexheader also has no user besidesPendingList.main. One build (maine5c2b3c with this branch, perf(agc): skip the staging, write-back and label-run leading barriers a covering barrier already orders #7489 and perf(recompiler): leave the descriptor-range comparison out of direct buffer loads when the device zeroes past the range #7484 merged in) and arms that differ only by theAPS5_NO_*off switches, three title rows per arm and two gameplay rows, with the trace summary lines. The lab machine runs it as soon as it has the memory free for clean rows; I will post every row here.python3 tools/check_conventions.py --base origin/mainpasses. Based on e5c2b3c (origin/main).tools/pr_overlap.py(against the open pull requests on main) lists textual overlaps with perf(agc): share one image table per heap and write epoch through capture, compare, resolution and descriptors #6454 (PendingList, aroundMayOverlapin Texture.cpp), perf(agc): import adjacent host ranges as aligned blocks and stop re-trying budget refusals #6457, perf(agc): take the host import budget from the device's memory budget #6479 and perf(agc): remember an evicting import request's budget refusal until it could succeed #6730 (all four:HostImportFordeclaration and body, GuestBufferMemory.cpp lines around 3002, and the signature in the header), perf(agc): remember structural host import misses #7457 (HostImportsstruct, the import insert andHostImportFor, which my commit 3 splits intohostImportSettled), perf(agc): avoid redundant texture-array CPU transfers #6718 and fix(agc): import selected texture layers within registered ranges #6820 (Texture.cpp, the line of thedirectlookup in Refresh that commit 3 changes), fix(agc): keep failed import handles out of cleanup #7082 (HostImportRetireHold.cpp, the end of the file where the new cases go), perf(agc): hand pending results over to an image whose DCC keys are a clear code #7427 (Texture.hpp near the members) and feat(agc): trace shader failures through draw submission batches #5431 (CMakeLists.txt, the test block); the other entries of the tool's list are shared files it could not compare. None implements an address index of the pending storage registry or a per-image cache of Refresh's host-import lookup. perf(agc): remember structural host import misses #7457 remembers structural misses ofHostImportFor; the cache here remembers hits and runs the sameHostImportForbody on a miss, so they compose.Checklist
main; no other open PR implements the same functionsNotImplemented_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)