Skip to content

[Bug] Bypass VMA for gtmp/listgen buffers on Darwin - #834

Merged
hughperkins merged 2 commits into
mainfrom
hp/darwin-vma-bypass-gtmp
Aug 1, 2026
Merged

[Bug] Bypass VMA for gtmp/listgen buffers on Darwin#834
hughperkins merged 2 commits into
mainfrom
hp/darwin-vma-bypass-gtmp

Conversation

@hughperkins

Copy link
Copy Markdown
Collaborator

Summary

  • On Apple/MoltenVK, allocate the fixed gtmp (1MB) and listgen (32MB) runtime buffers with plain vkAllocateMemory instead of VMA (AllocParams::bypass_pooled_allocator).
  • Motivated by a standalone no-Quadrants repro: VMA allocation of both sizes under per-cycle VkDevice churn hangs with kIOGPUCommandBufferCallbackErrorHang / VK_ERROR_DEVICE_LOST; the same sizes via vkAllocateMemory do not.
  • Validated on macos-26: bare qd.init(vulkan) / qd.reset() reaches 5000 cycles (LEAK_DONE, rc=0) with this change; baseline aborted around ~2900. Other VMA allocations remain and were not sufficient alone to trigger the hang on that path.
  • Linux/Windows and non-gtmp/listgen Vulkan allocations are unchanged.

Test plan

Made with Cursor

MoltenVK on GitHub macos-26 runners eventually returns VK_ERROR_DEVICE_LOST
(kIOGPUCommandBufferCallbackErrorHang) after a few thousand qd.init/reset
cycles when the 1MB gtmp and 32MB listgen DEVICE_LOCAL buffers are allocated
through VMA. The same sizes via plain vkAllocateMemory do not hang. Route
those two allocations through a dedicated unpooled path on Apple only;
other backends and allocations keep using VMA.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a8e13e2d9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +1648 to +1649
alloc.buffer = vkapi::create_buffer_unpooled(device_, physical_device_, &buffer_info, alloc_info.requiredFlags,
alloc_info.preferredFlags, &alloc.alloc_info);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve exportability in the unpooled path

When a Vulkan caller sets the new bypass_pooled_allocator flag together with export_sharing, this call drops the export-sharing information before the raw allocation: the VMA path switches to allocator_export_, but create_buffer_unpooled receives only memory property flags, so its vkAllocateMemory cannot attach the external-memory handle type needed for CUDA/Vulkan interop. Either reject that combination or pass the handle type through to the unpooled allocation.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not part of public api from python pov.

We don't support calling things directly from c++. C++ things are not public API.

// When true, backends that use a pooled allocator (Vulkan/VMA) allocate a dedicated
// VkDeviceMemory via vkAllocateMemory instead. On Darwin this is set for gtmp/listgen to
// avoid a MoltenVK GPU hang from VMA's pooled path for those two DEVICE_LOCAL sizes.
bool bypass_pooled_allocator{false};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Document the new allocation parameter

Adding Device::AllocParams::bypass_pooled_allocator changes the public allocation API and its usage, but this commit has no matching docs/ update, so end users of the RHI have no guidance on when the new flag is supported or safe to set. Please add user-facing documentation or keep the MoltenVK workaround out of the public API.

AGENTS.md reference: AGENTS.md:L15-L22

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not part of public api from python pov.

We don't support calling things directly from c++. C++ things are not public API.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Satisfy the Check line wrapping CI job on the under-wrapped comment runs
added for bypass_pooled_allocator.
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

@hughperkins
hughperkins merged commit 306312e into main Aug 1, 2026
68 checks passed
@hughperkins
hughperkins deleted the hp/darwin-vma-bypass-gtmp branch August 1, 2026 20:25
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.

1 participant