Skip to content

perf(agc): size the device staging pool from the device-local heap - #1147

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
alfalfa-star:perf/agc-staging-pool-heap-budget
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
alfalfa-star:perf/agc-staging-pool-heap-budget

Conversation

@alfalfa-star

@alfalfa-star alfalfa-star commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

The buffer pool's device tier (the detiler's scratch buffers and the staging shadows of written guest buffers) kept at most 512 MiB by default. When a title's working set of these buffers is larger, the driver allocates and frees device memory continually. PPSA14632 had 70761 misses and 69828 evictions per 10 s with 485 of 512 MiB retained, and each one is a vkAllocateMemory or vkFreeMemory.

  • The default budget is now an eighth of the largest device-local heap, and at least the old 512 MiB: up to 2 GiB on a 16 GiB card, 1 GiB on an 8 GiB card, 512 MiB below that.
  • APS5_STAGING_POOL_MIB still sets it explicitly, and 0 still disables the tier.
  • BufferPool::DeviceBudget takes the memory properties and is public, so the rule can be tested.

Tested

Linux 7.2.9 (Arch), Mesa 26.2.4 RADV, RX 9070 XT (16 GiB).

  • PPSA14632 (LEGO Horizon Adventures) with a 2 GiB budget: 1312 misses and 99 evictions per 10 s (1628 MiB retained), and presents rose from about 34 to about 39 per 10 s (with feat(agc): import shared direct memory through udmabuf on Linux #943 and the draw snapshot budget PR in place).
  • agc_driver_graphics: new stagingPoolBudgetTests (an eighth of a 16 GiB device-local heap is 2 GiB, a 2 GiB heap gives the 512 MiB floor, a host-only heap does not count).
  • ctest: 463 of 463 pass on current main.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

@alfalfa-star

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (be04d4a). The new conventions check flagged the edited BufferPool.hpp comment about the APS5_STAGING_POOL_MIB default. I deleted that comment instead of updating it; the default is described above. ctest: 463/463.

@alfalfa-star

Copy link
Copy Markdown
Contributor Author

The linux failure on be04d4a is a timeout in agc_driver_flat_store. The previous head passed linux, and the only change since then removes a comment, so it looks flaky. Could a maintainer re-run the failed job? I don't have the rights to.

@alfalfa-star

alfalfa-star commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Follow-up on the agc_driver_flat_store timeout, with numbers instead of "looks flaky". On linux every PR runs it three times (--repeat until-fail:3), and the first run is always much slower than the other two, presumably lavapipe compiling its shaders with a cold cache:

run ctest flags 1st 2nd 3rd
#1147 2625e61 serial 78.6 s 7.7 s 7.9 s
#1152 be2b6d3 serial 62.2 s 8.4 s 8.5 s
#1077 f402eab --parallel 4 85.0 s 5.2 s 5.0 s
#1147 be04d4a --parallel 4 timeout (120 s)

main's workflow added --parallel 4 between the two #1147 runs, and this test already used half to two thirds of its 120 s timeout when run serially. So whether it fails depends on what else runs beside it, not on this PR. A re-run should pass most of the time. The root cause, the size of this test's shaders, is already being addressed in #1265, which reports the cold run going from 84.4 s to 27.0 s (measured on Intel ANV, not lavapipe). #1280 and #1282 change the same code. Edit: #1715 (merged) already runs this test with RUN_SERIAL, so a rebase onto current main picks up that fix.

@alfalfa-star

Copy link
Copy Markdown
Contributor Author

Rebased onto main 6b56175 (6d1cf27), which includes #1715. agc_driver_flat_store now passes when run alone, but agc_driver_buffer_format timed out instead. Since #639 that test takes 93-108 s on lavapipe for every PR, not only this one (details in #1762). Could a maintainer re-run the linux job? Locally, 479/479 pass.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 9, 2026

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

This conflicts with main since commit 28345b0 (device size classes), and it isn't a mechanical rebase:

  • BufferPool::DeviceTiered and Capacity(bytes, properties) are static and call DeviceBudget() to know whether the device tier is enabled. With DeviceBudget(memory) there is no Context there, so they can't compute the heap-derived value. Keep the environment check (APS5_STAGING_POOL_MIB=0 disables the tier) in a static helper that doesn't need the heap, and use the heap-derived value only for deviceTier.budget in the constructor, so Capacity and tierFor stay consistent.
  • After the rebase, keep stagingPoolBudgetTests and make sure it still covers the 16 GiB / 2 GiB / host-only-heap cases.
  • The linux failure on the current head is the agc_driver_buffer_format timeout on lavapipe you already described (#1762), not this change; it should clear on a re-run after the rebase.

@alfalfa-star
alfalfa-star force-pushed the perf/agc-staging-pool-heap-budget branch from 6d1cf27 to 77fd100 Compare October 10, 2026 08:09
@alfalfa-star

alfalfa-star commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (after #1004's size classes) in 3bb4494, as you suggested:

  • DeviceTierEnabled() (static, read once) holds the environment check: the tier is in use unless APS5_STAGING_POOL_MIB is set to 0. DeviceTiered, and through it Capacity, Usage and tierFor, use only that, so the size classes and the tier choice can't disagree.
  • DeviceBudget(memory) is used only in the constructor, for deviceTier.budget: APS5_STAGING_POOL_MIB when set, else max(512 MiB, largest device-local heap / 8).
  • stagingPoolBudgetTests is unchanged and still covers 16 GiB → 2 GiB, 2 GiB → 512 MiB and a host-only heap → 512 MiB.

ctest: 638/638 on RADV (RX 9070 XT).

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#374 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp L8
#425 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp L8
#454 core/libs/prx/libSceAgcDriver/tests/Graphics.cpp L8

The buffer pool's device tier (detiler scratch buffers and the staging
shadows of written guest buffers) retained at most 512 MiB by default.
A title whose working set of these buffers is larger allocates and
frees device memory continually: PPSA14632 (LEGO Horizon Adventures)
had 70761 misses and 69828 evictions per 10 s with 485 of 512 MiB
retained, each a vkAllocateMemory or vkFreeMemory.

The default budget is now an eighth of the largest device-local heap,
and at least the old 512 MiB, so a 16 GiB card retains up to 2 GiB and
an 8 GiB card 1 GiB. APS5_STAGING_POOL_MIB still sets it.

Only the constructor knows the device, so only the budget comes from the
heap. Whether the tier is in use at all (APS5_STAGING_POOL_MIB unset or
not 0) stays a static check, DeviceTierEnabled, which Capacity, Usage and
tierFor share, so the size classes and the tier stay consistent.

On PPSA14632 with 2 GiB the device tier has 1312 misses and 99
evictions per 10 s (1628 MiB retained), and presents rise from about 34
to about 39 per 10 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alfalfa-star
alfalfa-star force-pushed the perf/agc-staging-pool-heap-budget branch from 77fd100 to 3bb4494 Compare October 10, 2026 08:21

@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 the rework against the size classes: DeviceTiered/Capacity only depend on the static DeviceTierEnabled(), the heap-derived budget is used only in the constructor. Built and ran agc_driver_graphics on RADV; the new stagingPoolBudgetTests don't compile without the change. Merging.

@adriadam10
adriadam10 merged commit aad13ac into boykopovar:main Oct 10, 2026
14 checks passed
@alfalfa-star
alfalfa-star deleted the perf/agc-staging-pool-heap-budget branch October 10, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants