Skip to content

Metal: Residency sets and barriers - #46

Merged
DarioSamo merged 6 commits into
renderbag:mainfrom
squidbus:residency-sets
Aug 5, 2025
Merged

Metal: Residency sets and barriers#46
DarioSamo merged 6 commits into
renderbag:mainfrom
squidbus:residency-sets

Conversation

@squidbus

@squidbus squidbus commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

Based on #25, plus:

  • Rebased on the latest changes.
  • Residency sets:
    • Fixed location of useResidencySet and commit calls on residency set.
    • Added locking on residency set write and commit since residency sets are not thread-safe according to documentation and validation.
    • Fixed residency issues with GPU-addressable buffers.
  • Barriers:
    • Moved fences to be per-command list and allocated as needed without a cap.
    • Reduced unneeded encoded fence waits/updates.
    • Fixed some bugs/typos.

I've tested this with UnleashedRecomp and MarathonRecomp and seem to be working okay with my changes added.

EDIT: Fixes #10 (not directly but makes it not necessary).

@squidbus
squidbus requested a review from dcvz August 3, 2025 09:43
@IsaacMarovitz IsaacMarovitz mentioned this pull request Aug 3, 2025
1 task
Comment thread plume_render_interface_types.h Outdated
Comment thread plume_metal.cpp Outdated
Comment thread plume_metal.cpp Outdated
Comment thread plume_metal.cpp Outdated
Comment thread plume_metal.cpp Outdated
Comment thread plume_metal.cpp Outdated
Comment thread plume_metal.cpp Outdated
@squidbus squidbus changed the title Metal: Residency sets and barriers Metal: Residency sets Aug 3, 2025
@squidbus
squidbus force-pushed the residency-sets branch 2 times, most recently from a2e0d7d to 8bbcc86 Compare August 3, 2025 22:43
@squidbus squidbus changed the title Metal: Residency sets Metal: Residency sets and barriers Aug 3, 2025
squidbus and others added 2 commits August 4, 2025 05:34
Co-authored-by: Isaac Marovitz <isaacryu@icloud.com>
Co-authored-by: Isaac Marovitz <isaacryu@icloud.com>
@dcvz

dcvz commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

Getting this when using it via RT64 and Zelda:

IOGPUMetalCommandBufferStorageAddResidencySets:1201: failed assertion `IOGPUMetalCommandBufferStorageAddResidencySets: command buffer residency set limit of 32 exceeded'
Program ended with exit code: 6

Dealing with 32 set limit per command buffer
@squidbus

squidbus commented Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

Getting this when using it via RT64 and Zelda:

IOGPUMetalCommandBufferStorageAddResidencySets:1201: failed assertion `IOGPUMetalCommandBufferStorageAddResidencySets: command buffer residency set limit of 32 exceeded'
Program ended with exit code: 6

Updated to only use residency sets on large descriptor sets, to avoid hitting limit.

Comment thread plume_metal.cpp Outdated

@dcvz dcvz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 works on Zelda and left one small comment

Comment thread plume_metal.cpp
Comment thread plume_metal.cpp

@DarioSamo DarioSamo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted some race conditions that should be addressed.

Comment thread plume_metal.cpp
Comment thread plume_metal.cpp
@squidbus

squidbus commented Aug 5, 2025

Copy link
Copy Markdown
Contributor Author

Addressed the locking concerns.

Comment thread plume_metal.h Outdated
@DarioSamo
DarioSamo merged commit 77a6e10 into renderbag:main Aug 5, 2025
10 checks passed
@squidbus
squidbus deleted the residency-sets branch August 7, 2025 22:19
jessetbh pushed a commit to jessetbh/plume that referenced this pull request Jul 6, 2026
* Metal: Residency sets

Co-authored-by: Isaac Marovitz <isaacryu@icloud.com>

* Metal: Manual barriers

Co-authored-by: Isaac Marovitz <isaacryu@icloud.com>

* Metal: Use residency set only for large descriptor sets

Dealing with 32 set limit per command buffer

* Metal: Cache supportsResidencySets and fix up bufferDeviceAddress capability.

* Metal: Lock accesses to gpuAddressableResources

* Metal: Add missing nullptr init

---------

Co-authored-by: Isaac Marovitz <isaacryu@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement descriptor heaps on Metal.

3 participants