Skip to content

Vulkan: Isolate oversized memory pages from shared allocations - #826

Open
alken1t15 wants to merge 6 commits into
DiligentGraphics:masterfrom
alken1t15:fix/vulkan-oversized-memory-pages
Open

alken1t15 wants to merge 6 commits into
DiligentGraphics:masterfrom
alken1t15:fix/vulkan-oversized-memory-pages

Conversation

@alken1t15

@alken1t15 alken1t15 commented Nov 9, 2026 •

Copy link
Copy Markdown

The allocation policy is under discussion following review. A separate opt-in threshold is proposed in the review discussion.

Fixes #820.

The Vulkan memory manager grows pages by powers of two for large allocations, then makes the remaining space available to unrelated resources. A small, long-lived allocation can keep the entire page alive after the large resource has been released.

Changes

  • Use the configured device-local and host-visible page sizes as the shared-allocation limits. Requests above the corresponding page size get a separate page sized to the request, without power-of-two rounding.
  • Exclude oversized pages from subsequent allocations and reclaim empty oversized pages during memory collection, even when total allocated memory is below the reserve.
  • Keep the existing deferred-release path: a page remains occupied until its allocation is released through the GPU completion queues.
  • Document the behavior of DeviceLocalMemoryPageSize and HostVisibleMemoryPageSize. No new public settings are introduced.
  • Add VulkanMemoryManagerTest.OversizedPages, covering both device-local and host-visible memory, small-allocation sharing, the exact page-size boundary, live-page preservation, and oversized-page reclamation with a nonzero reserve. The reproduction uses small pages rather than requiring gigabytes of GPU memory.

Validation

Ubuntu 22.04 under WSL, GCC Debug build, Vulkan 1.3.255 on Mesa llvmpipe (software driver).

  • With the original MemoryManager.cpp, the new test fails: the small allocation shares the oversized page and the oversized pages remain allocated under the reserve.
  • With this change restored, all 17 selected tests pass: VulkanMemoryManagerTest.*, BufferCreationTest.*, BufferAccessTest.*, and CopyTexture.*.
  • Source formatting checked with the repository clang-format version; git diff --check passes.

Windows x64, MSVC 2022 (19.44), Debug build. After installing the missing ATL component, the native Windows build succeeds.

On both Intel Iris Xe (32.0.101.7092) and NVIDIA RTX 3060 Laptop (621.01), the new regression test fails with the original MemoryManager.cpp; all 17 selected tests pass with the fix restored.

The baseline and restored builds both recompiled MemoryManager.cpp. The Windows Vulkan runs use the physical GPUs; VK_LAYER_KHRONOS_validation is not installed.

Both test configurations disable HLSL and glslang; the selected tests do not require shader compilation.

Review follow-up: fixed new-page sizing to include the suballocator’s alignment rounding. Added VulkanMemoryManagerTest.AllocationAlignment for non-aligned sizes and alignment larger than the base page size. It fails on the previous PR revision and passes after the fix. All 18 selected tests pass on both Windows GPUs after this update.

@MikhailGorobets

MikhailGorobets commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

It seems like this commit doesn't make sense at all. We can't use HostVisibleMemoryPageSize and DeviceLocalMemoryPageSize to limit allocation sizes. This would increase memory fragmentation, since these values are intentionally set to be small to allow allocations to grow as memory fills up. So what about alignment?

@alken1t15
alken1t15 marked this pull request as draft November 9, 2026 15:29
@alken1t15

alken1t15 commented Nov 9, 2026 •

Copy link
Copy Markdown
Author

You are right: I conflated the base page size with a limit for shared allocations. This changes the allocation policy for existing configurations, and the tests I added do not establish that the fragmentation tradeoff is acceptable.

There was also an alignment bug. VariableSizeAllocationsManager::Allocate() rounds the requested size up to the alignment, so sizing a new page to the unrounded request can make the first allocation fail. I fixed that and added a regression test for a non-aligned size and for alignment larger than the base page size. The new test fails on the previous PR revision; with the fix, all 18 selected tests pass on both Intel Iris Xe and RTX 3060 Laptop under Windows.

For the allocation policy, would you accept separate, opt-in device-local and host-visible size thresholds, with zero preserving the current sharing and page-growth behavior? Only requests above an enabled threshold would use isolated pages, reclaimed outside the shared-page reserve. The existing page-size settings would retain their original meaning.

I have not added the new settings yet, pending agreement on that direction.

@alken1t15
alken1t15 marked this pull request as ready for review November 9, 2026 15:30
@alken1t15

Copy link
Copy Markdown
Author

Hi @TheMostDiligent, @MikhailGorobets — could you please let me know who would be the right person to review this PR, or when you might have a chance to take a look? I would also appreciate feedback on the opt-in allocation thresholds proposed in my previous comment before proceeding with that part. Thanks!

@TheMostDiligent

Copy link
Copy Markdown
Contributor

This is a high-risk change with potentially a very large blast radius as it alters memory management.

Initial analysis shows that it introduces at least two regressions:

  1. An empty oversized page can force a second native allocation. Allocate() skips every oversized page, including an empty page of exactly the requested size. Both command submission and ReleaseStaleResources() call ShrinkMemory() before purging completed releases. A purge can therefore leave an empty page resident, and the next allocation can fail for lack of memory while that page is still available. In a same-size replacement test, the original allocator used one native page and the PR used two.
  2. Oversized pages can drain the shared-page reserve. The reserve comparison counts both shared and isolated pages. With 64 KiB pages, a 128 KiB reserve, and a live 96 KiB isolated allocation, collection discarded the only empty shared page. Releasing the isolated allocation then left zero pages reserved. The original allocator retained one.

Making changes in this area requires extra attention.

@alken1t15

Copy link
Copy Markdown
Author

Thanks for the concrete repro cases, @TheMostDiligent. I reproduced both regressions and pushed fixes:

  • Empty oversized pages can now be reused for compatible oversized requests, choosing the smallest suitable page. The same-size replacement test verifies that no second native page is created, including when collection runs before the previous allocation is released.
  • The shared-page reserve is now calculated from shared pages only. The regression test covers the 64 KiB page / 128 KiB reserve / 96 KiB isolated allocation scenario, both creation orders, and collection after releasing the isolated allocation. It also checks that excess empty shared pages are still collected.

I also reviewed the surrounding allocation and collection paths. Small requests no longer pin empty oversized pages; unusable empty oversized pages in the matching page group are reclaimed before requesting a new native allocation. Resident and peak allocation counters are updated only after successful page construction and insertion. I added checks for invalid alignment and size overflow, plus regression tests for page selection, growth, and recovery after an injected host-allocation failure. Three of these additional tests fail on the previous PR revision and pass with the fixes.

I merged the current master and resolved the conflicts, preserving the upstream synchronized IsEmpty() implementation and the restriction that only MemoryManager can allocate from a page.

Validation on Windows with Vulkan: all 26 selected tests pass on both Intel Iris Xe and RTX 3060 Laptop, covering the memory manager, buffer creation/access, and texture copies. The concurrent release/collection test also passed 20 repetitions on each GPU before the master merge, and passed again as part of the post-merge test run. This is targeted coverage; I have not run the full API suite or forced a real driver OOM/mapping failure. AppVeyor is still pending.

These fixes address the correctness issues, but do not establish that the allocation-policy tradeoff is acceptable. The separate opt-in thresholds discussed above are still unimplemented pending agreement on that direction.

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.

Vulkan: Small resources can prevent oversized memory pages from being released

3 participants