Skip to content

Fix pool DeviceMemory allocations and rek - #1732

Merged
robertosfield merged 4 commits into
vsg-dev:masterfrom
rolandhill:devmem
Aug 18, 2026
Merged

Fix pool DeviceMemory allocations and rek#1732
robertosfield merged 4 commits into
vsg-dev:masterfrom
rolandhill:devmem

Conversation

@rolandhill

Copy link
Copy Markdown
Contributor

Pool DeviceMemory allocations in MemoryBufferPools

Problem

MemoryBufferPools::reserveMemory() computes a pool-sized deviceSize (floored at
minimumDeviceMemorySize, 16 MB) but never uses it — DeviceMemory::create() receives the
original memRequirements, so every image gets an exact-fit dedicated vkAllocateMemory.
In applications that page resources continuously (paged terrain databases), memoryPools
accumulates one entry per image and is never pruned. In our application (Spatial Integration)
a scene reached 23,000+ DeviceMemory entries averaging ~57 KB:

  • The linear memoryPools scan under the per-device mutex became the dominant cost of resource
    compilation: per-tile compile time grew from 156 ms at ~400 resident tiles to 2.5 s at ~1600,
    while data preparation stayed flat.
  • The allocation count far exceeds maxMemoryAllocationCount (spec minimum 4096).
  • Freed memory was never returned to the driver — footprint pinned at high-water mark.

Changes

Commit 1 — use deviceSize as intended (src/vsg/vk/MemoryBufferPools.cpp):

  • New DeviceMemory blocks are allocated at deviceSize, so subsequent resources suballocate
    from shared pools. Explicit dedicated allocations (pNextAllocInfo) keep their exact size.
  • Since buffers and optimal-tiling images can now share a pool, every slot is aligned to
    bufferImageGranularity.
  • Pool matching uses equality of memoryTypeBits and property flags — the previous superset
    test could select a pool whose chosen memory type index is invalid for the new resource.
  • reserveBuffer() skips pool buffers whose maximumAvailableSpace() cannot fit the request,
    avoiding a futile walk of each fragmented buffer's slot map.

Commit 2 — MemoryBufferPools::releaseUnusedPools(): releases pool entries with no
reserved slots back to the driver and returns the bytes freed. Opt-in for applications to call
under memory pressure; no behavioral change otherwise.

Results

Same scripted camera path, same resident tile count (~500), 6 DatabasePager threads:

before after
DeviceMemory allocations 23,285 96
per-tile compile median / p90 / max 27 / 49 / 205 ms 2.7 / 5.4 / 6.0 ms
tiles compiled in same test time 12,966 17,996 (+39%)

Compile cost is now flat in residency instead of growing linearly.

targetMaxNumPagedLODWithHighResSubgraphs can now be set much higher without degradation (I'm defaulting to 4000).

Tested on Kubuntu 26.04 and Windows 11.

@rolandhill

Copy link
Copy Markdown
Contributor Author

Hold off on this. There is a possible regression on one hardware/driver combination.

@robertosfield

Copy link
Copy Markdown
Collaborator

What hardware/driver combination is causing a regression? What are the symptoms?

FYI, I've done a lot of work over the last few months refactoring DatabasePager/MemoryBufferPools/CompileManager etc. to handle very large paged databases that quickly blow the device memory limit without these changes. Part of this work is to dynamic change targetMaxNumPagedLODWithHighResSubgraphs to make it possible to claw back memory/resources. This work has been a bit of case of 2 steps forwards, one step back. I addressed one of the steps back in commits in July so I'm no looking at making a dev release to wrap these fixes up.

From your description, your issues are different, but touch upon the same code base. For my own testing, what's the best way to spot the issue?

@rolandhill

rolandhill commented Aug 14, 2026 via email

Copy link
Copy Markdown
Contributor Author

@rolandhill

Copy link
Copy Markdown
Contributor Author

It was a false alarm. The possible regression was in my application and had nothing to do with this PR. Please go ahead and assess this.

{
reservedSlot = deviceMemory->reserve(totalSize, alignment);
// if (!deviceMemory->full())

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 in essence just commenting back in the original if (!device->full()) line. This original usage is clearer in intent than the new if (..) check.

I commented out this check to make sure the MemoryBufferPools full keeps track of all DeviceMemory objects, as I found it useful to take memory usage etc. I think if we want to re-enable check before adding it to track memory then having an option in MemoryBufferPools to control this would be appropriate rather than having dualling commits.

@robertosfield
robertosfield merged commit ddd5d1c into vsg-dev:master Aug 18, 2026
5 checks passed
@robertosfield

Copy link
Copy Markdown
Collaborator

I merge now merged the rolandhill-devmem branch associated with this PR with VSG master, minus the change to // if (!deviceMemory->full()) discussed above. We can discuss that separately.

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.

2 participants