Repository navigation
perf(vk): an import mostly outside every game pool is trimmed back to its live parts - #2825
Conversation
… its live parts Imports only grew: a binding across an import's end makes it again larger, and a pool the game binds whole is imported whole. A pool's VRAM mirror and its write-back shadow are its import's whole range. After the game frees a scene's large pools and makes the next scene's smaller ones at the same addresses, the import keeps the old range, bound every frame through the small live pool inside it, and its mirror holds the dead rest for the session. Castle Town Market entered from Hyrule Field on an 8 GB card: a 224 MB mirror where the Market loaded from a save has 48, and 152 MB where it has 104. While VRAM is over 80% full, a mirrored import that has been mostly outside every game pool for 1800 submissions is retired and each of its live parts imported again with a mirror made in the same submission's uploads, taking the GPU's contents from the old mirror as a pool made again larger does (gpu/vk/mirror_trim.h, mirrors_trim). RX_VK_MIRROR_TRIM=0: off. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
CI note: the one red check (Python source checks, ubuntu part 2/2) is |
|
A correction to the measurements above, found an hour after opening this: this PC's DLSS model row was set to Transformer 2 (L), not the default (K), for every run in the description. L costs ~4 ms more GPU time a frame on this card (#2836) and, judging by the count, more VRAM too, so it is the harder case. The same test with the default model (
|
hckrman101
left a comment
There was a problem hiding this comment.
Thanks, this is a well-argued change, and the honest correction about the DLSS model row helps a lot. The rules header is easy to follow, and so is the "import all parts first, then retire" order. Reusing retire_pool and the existing inherit means in-flight work is covered the same way a pool made again larger is: the old import and mirror stay until retired_value is done, the page table entries go at retirement, and the parts' mirrors are made in this submission's uploads before anything is recorded. I don't see a sync problem there.
One real issue (inline): the old mirror stops waiting to be inherited even when a part didn't get its mirror. Then what the GPU wrote there is lost for that part, which is the Kokiri-foliage class of bug that the inherit was added for.
Other points:
- Chunk size. The description, the header and the tests all assume 4 MB chunks ("each out to its 4 MB chunks"), but
import_chunk()is still 16 MB on main and at this head. 4 MB comes from #2685, which was in your local stack, as is themirror_on()stub this PR adds tonx2/test_import_windows.py(the same line #2685 adds). On main alone the parts round out to 16 MB, so less is given back (the culling lists' 152 MB import is close to the quarter line), and the measured numbers don't describe this PR on its own. I'd rather land #2685 first and this after it. Please say that in the description; I'll handle the order. - Measurements. Hardware is named and the A/B is done properly, thanks. But at the default DLSS model (K) the gain is ~1 fps and a few fewer long frames, from two runs an arm, with #2825 and #2826 measured together. Could you add three runs an arm with K for main+#2685 against main+#2685+this PR (without #2826)? That would show what this PR does on its own at the default. The L numbers can stay as the hard case.
- Not tested on AMD, Intel or Linux. The code path is the same one a pool made again larger takes, so I don't expect a problem there. On Linux, the parts are imported while the old import still holds the same pages, as happens today when a pool grows. Still, one run on an AMD card (Windows, where retired imports wait 30 s) or on Linux would be good before this goes on by default. Even a short session log with the
trimmed tolines and no device loss would do. - Conflict. GitHub reports a conflict with main. It's only the include line next to #2731's
#include "wb_learn.h", so a merge of main fixes it. - CI. The red check is
tests/test_release_scripts.py's two-nightlies-at-once case, which has nothing to do with this PR. I'll re-run it. - Overlaps. #2795 (give back retired imports whose mirror was refused) and #2839 (Linux cap for idle live imports) touch other hunks and are independent. If a part is ever refused a mirror, #2795 lets its import go back once that part is later retired. #2759 also edits
memory_begin_submissionand retirement (finalized pools, no inherit), so whichever lands second will need a small rebase. Your description already explains how the two cover different cases.
| for (Pool* p : mirror_wanted) mirror_make(*p); | ||
| if (trimmed) // (its parts' mirrors are made, each with its own part of the old one: that one waits for no more) | ||
| for (auto& nh : retired_pools) | ||
| if (nh.mapped().trimmed) nh.mapped().trimmed = false, nh.mapped().inherit_pending = false; |
There was a problem hiding this comment.
This clears inherit_pending on the old pool whether or not each part actually got a mirror. mirror_make can leave a part without one: there's mirror_budget(), where the check in mirrors_trim counts only kept and not the other pools already in mirror_wanted, which come first in the list; there's RX_VK_MIRROR_TIGHT; and there's create_device_buffer failing, which is more likely here because this only runs over 80% VRAM. A part left like that points at its import, and the old mirror is then freed once the GPU is done with it. If the old pool was gpu_only or gpu_written, what the GPU wrote in the live part is gone, and the part's mirror is later made from guest memory.
Suggestion: clear trimmed/inherit_pending only when every part's mstate is MIRROR_ON. Otherwise leave it pending, so that the part's next mirror_make still inherits, and the existing kIdleEpochs limit still bounds how long that can take. You could also count the current mirror_wanted in the room check (comment below). A check for this in test_mirror_trim.py would be good too.
| } | ||
| if (!pick) return false; | ||
| const uint64_t kept = mirror_trim::bytes(pick_parts), size = pick->hi - pick->lo; | ||
| if (mirror_bytes + kept > mirror_budget()) return false; // (no room for both yet: looked at again in 64 submissions) |
There was a problem hiding this comment.
The room check should also count what's already in mirror_wanted (as memory_begin_submission does just after). Otherwise the parts can be the ones refused for budget, as described above.
| int64_t st_trims = 0; | ||
| uint64_t st_trim_bytes = 0; | ||
| bool mirrors_trim() { // (pools_mu held; true when one was trimmed: its parts are in mirror_wanted) | ||
| static const bool on = !getenv("RX_VK_MIRROR_TRIM") || atoi(getenv("RX_VK_MIRROR_TRIM")) != 0; |
There was a problem hiding this comment.
Nit: with atoi, RX_VK_MIRROR_TRIM=on (or true/yes) turns the trim off. The docs' on/off rule accepts those words, so I'd parse it the way the other switches do: for example off only when the value is one of 0/off/false/no, like the env_switch helpers in vk_dlss.cpp/vk_fsr4.cpp.
… own After a trim, memory_begin_submission let the old mirror go even when a live part was refused its own (the mirror budget, RX_VK_MIRROR_TIGHT, or a failed allocation), so what the GPU wrote there was lost. trims_settle now lets it go only once every pool over its range is mirrored; meanwhile a part's mirror made later still inherits from it, and kIdleEpochs bounds the wait. A part refused for a failed allocation is tried again at its next binding. The trim's room check also counts the mirrors already wanted, which are made first, and does not trim while RX_VK_MIRROR_TIGHT refuses every mirror. RX_VK_MIRROR_TRIM is read by the switches' rule (env_value_off), so on, true or yes no longer turn it off. test_mirror_trim runs trims_settle with a part refused its mirror.
hckrman101
left a comment
There was a problem hiding this comment.
I pushed two commits on top of yours (a fast-forward, nothing of yours rewritten):
- daf6b17: merges main. The only conflict was the include line, and both
mirror_trim.handwb_learn.hare kept. - 18a733a: the fix for the refused-mirror case from my last review. A new
trims_settle()keeps the old mirror waiting to be inherited until every pool over its range has a mirror of its own. A part refused one (budget,RX_VK_MIRROR_TIGHT, a failed allocation) still inherits from it when its mirror is made later, as a pool made again larger does, andkIdleEpochsstill bounds the wait. A part refused for a failed allocation is tried again at its next binding. The room check now also counts what's already inmirror_wanted, and no trim runs while the tight rule would refuse every mirror.RX_VK_MIRROR_TRIMis read withenv_value_off, soon/true/yesno longer turn it off.test_mirror_trim.pynow also compilestrims_settleand checks that a part refused its mirror keeps the old one.
The related checks pass locally, and so does the renderer build with the ABI check. Approved as the maintainer. Still welcome but not blocking: the K-model runs I asked for, and an AMD or Linux run with the trimmed to lines in the log.
… an eighth of the texture cache (#2826) **Why.** On an 8 GB card the texture cache sits right at the 90% line in the larger scenes. When it is a few MB over, the pass lets go of an eighth of the cache, textures that were on screen a second ago, and they are made again within the camera's turn: a loop of evictions, uploads and compaction runs that shows as a long frame every second or two. - `rule_target` cuts to 7/8 of the rules' budget once the textures hold more than it. That is right for a budget that is a size (the card's or a preset's). - The 90% rule's budget and the driver-limit (97%) rule's are not sizes: each is "what the textures hold, less the excess over a line". The comment on the hold's own pass already says so for the hold ("what goes is the excess alone ... crossed again whenever anything else grows", the 4,589 -> 2,312 MB case), and the hold was changed for it. These two rules still went through 7/8. - Measured: Castle Town Market entered from Hyrule Field, RTX 3060 Ti 8 GB. `2,873 MB` of textures, 62 MB over the 90% line. The pass's target was `7/8 x 2,811 = 2,460 MB`: ~400 MB of textures unused for 60 presents go, all in view again a few seconds later. The log shows it as a sawtooth, `textures retired ... budget 386 / 362 / 295` and the cache going 2.95 -> 2.5 -> 2.95 GB, with 8-9 uploads a frame, where the same scene loaded from a save (just under the line) has 0. ## What - `vram_compact::rule_target(total, size_budget, rule_budget)`: the budget's size still asks for its 7/8. A rule budget below the size (the 90% rule, the driver's limit) asks for its excess and `kExcessSlack` (64 MB) more, so that not every pass is one. The lower of the two when both apply. - `texture_frame` keeps the budget as it was before the 90% rule (`size_budget`) and passes both. - `RX_VK_EVICT_EXCESS=0`: to 7/8 of their budget, as before. - Only the compacted path (`RX_VK_COMPACT`, the default) goes through `rule_target`; with the compaction off nothing changes. ## Measured **Note:** every run below had this PC's DLSS model row on Transformer 2 (L), not the default K: the harder case (more GPU time and VRAM, #2836). With K the gain is smaller; those numbers are in the comment below. Windows 11, RTX 3060 Ti 8 GB (driver 32.0.16.2114), Ryzen 5 5600X, 2560x1440 DLSS. `main` ba005ca with #2711, #2731 (at bce3d17, where it was still on by default), #2685 and #2709 merged locally. Load the save, teleport to Hyrule Field, 10 s there, teleport to the Market, settle, then **80 s** of the camera turning. `RX_VK_FRAMEGEN=0`, `HOS_FPS_MAX=240`. Three runs an arm, arms alternated. The other half of the same stutter is #2825 (the previous scene's oversized mirrors), so both are shown alone and together. | arm | fps mean | frames over 50 ms in 80 s | after the first 20 s: fps | after the first 20 s: frames over 50 ms per 20 s | |---|---|---|---|---| | main stack | 44.4 / 45.0 / 45.0 | 100 / 69 / 60 | 46.5 | 12.3 | | **this PR** | 43.3 / 46.6 / 46.2 | 75 / 25 / 60 | 47.0 | 7.0 | | #2825 alone | 45.7 / 47.2 / 47.4 | 66 / 37 / 28 | 48.5 | 4.7 | | this PR + #2825 | 49.7 / 48.8 / 49.3 | 10 / 15 / 22 | 50.6 | 0.3 | - Alone it is a modest gain and noisy (one of its three runs was the slowest of all twelve in its first 40 s). The clear result is the pair: with the mirrors trimmed the cache is close enough to the line that taking the excess alone ends the loop, 0 or 1 frame over 50 ms in each 20 s after the arrival, in all three runs. - Loaded from a save in the Market, two 30 s runs an arm: 47.1 / 47.9 fps and 4 / 1 frames over 50 ms with this and #2825 on, 46.5 / 46.0 and 6 / 7 with both off; 0-1 uploads a frame against 2-7. (In the morning the same load was 53-55 fps and under the line; by these runs the driver's allowance was ~100 MB lower, 6,944 against 7,043 MB, which puts it over the line as well. No trim runs in a direct load, so this difference is this PR's.) ## Not tested - AMD, Intel, Linux; cards of 4-6 GB, where the count sits past 90% all the time and the excess is larger. The rule there lets go of the same excess as before plus 64 MB instead of an eighth, so the count stays nearer the line; if a card needs the deeper cut to stay clear of the driver's limit, the 97% rule and the hold are unchanged and still take theirs. That is reasoning, not a measurement: a result from a 6 GB card would be welcome. - With the compaction off. ## Tests - `tests/test_vram_compact.cpp`: the three-argument `rule_target` (no rule, the excess alone with the Market's numbers, the size's 7/8, both, a budget under the slack), next to the existing cases, which are unchanged. - `tests/test_vram_compact.py` checks `texture_frame` keeps `size_budget` and calls it, with the switch. - `python -I -B tests/run_all.py vram_compact texture_squeeze vram_pressure vram_budget preset_texture target_image` (the checks that read this code): 6 passed. The full suite was run on #2825's branch (460 passed; the 2 that fail there fail on `main` here too), not again on this one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
The per-block idle stamps are block_dead_since now: NotRexed#2825 added a per-pool dead_since for the trim. Both docs rows kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Why. On an 8 GB card, Castle Town Market runs at ~54 fps with almost no long frames when it is loaded from a save, and at ~45 fps with a long frame every second or two when the player walks in from Hyrule Field, for as long as they stay. Part of the difference is VRAM the previous scene's pools still hold: imports never shrink.
0x108c711000leaves a 224 MB mirror where the Market loaded from a save has 48 MB; the culling lists' pool is 152 MB where it has 104 MB. With their shadows that is ~260 MB the scene does not bind, on a card where the Market's textures sit right at the 90% line.This is the launch-to-launch "fast or slow Market" from #995 and my note on #2685: it was not the launch, it was whether the save was in the Market or in the field.
What
gpu/vk/mirror_trim.h(rules only) andmirrors_trim()invk_memory.cpp, called frommemory_begin_submissionevery 64 submissions:retire_pool, as a pool made again larger is, and each part gets its page table entries and a mirror in the same submission's uploads, before anything is recorded. Each new mirror takes what the GPU wrote from the old one throughmirror_make's existing inherit (pages the CPU stamped since come from guest memory, as there). The old mirror goes once the GPU is done with it.RX_VK_MIRROR_TRIM=0: off. One log line a trim.What is given up: what the GPU wrote into the dead part of the old mirror and never wrote back. No game pool covers that memory any more, so nothing can bind it; a pool the game makes there later gets a mirror from guest memory, as any new pool does.
Measured
Note: every run below had this PC's DLSS model row on Transformer 2 (L), not the default K: the harder case (more GPU time and VRAM, #2836). With K the gain is smaller; those numbers are in the comment below.
Windows 11, RTX 3060 Ti 8 GB (driver 32.0.16.2114), Ryzen 5 5600X, 2560x1440 DLSS.
mainba005ca with #2711, #2731 (at bce3d17, where it was still on by default), #2685 and #2709 merged locally (the VRAM picture on this card without #2711 is the #2690 regression). Load the save, teleport to Hyrule Field, 10 s there, teleport to the Market, settle, then 80 s of the camera turning.RX_VK_FRAMEGEN=0,HOS_FPS_MAX=240. Three runs an arm, arms alternated. The fourth arm adds #2826 (the 90% rule evicting its excess instead of an eighth of the cache), which is the other half of the same stutter.pool 0x108c000000..0x109a400000 (228 MB) trimmed to its 1 live part(s), 48 MBand0x1178400000..0x1181c00000 (152 MB) ... 104 MB. Mirrors at the end of a run: 1,584 MB against 1,820 MB. The new imports took under 2 ms each (noimported inline, which is logged from 2 ms).A session: two rounds of 12 scenes by teleport (Hyrule Field, Kokiri Forest, Market, Kakariko, Lost Woods, Goron City, Zora's Domain, Graveyard, Death Mountain Trail, Zora's River, Sacred Forest Meadow, Gerudo Valley), 20 s of camera turning in each, with the flash detector on (
RX_VK_FLASH=1,RX_VK_SHOT_EVERY=1): 24 of 24 scenes reached, 13 trims (600 MB given back in all), no flash detected, no device loss, no failed import. The mirrors moved between 1.2 and 2.0 GB and ended at 1,656 MB; the same tour before this sat at the 2,000 MB budget. Screenshots after the camera turn in Kokiri Forest (foliage, grass), Hyrule Field and the Market look right; I did not watch the runs live.Not tested
retired_give_back.h), which this does not change but does feed.RX_VK_PARALLEL(the trim does not run inside its segments, and was not run with it on).Tests
tests/test_mirror_trim.pycompiles and runstests/test_mirror_trim.cpp(the Market's case, imports their pools fill, a small dead share, no live pool, parts joined and kept apart, too many parts, the waits and the count that starts over, the VRAM gate) and checksvk_memory.cppkeeps the order: parts imported before the retire, mirrors made before the old one stops waiting to be inherited, room in the budget first.nx2/test_import_windows.py's model ofPoolgets the two new fieldspool_lookupcarries over;tests/test_texel_buffer_ownership.pycounts the trim's rollback among the callers ofdestroy_import_buffer(10).python -I -B tests/run_all.pyon Windows: 460 passed, 2 failed, 34 skipped; the two (nx2/test_nup_paths.py,tests/test_input_background.py) fail on unchangedmainhere too.🤖 Generated with Claude Code