Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fallback to dedicated allocation can skip budget/maxMemoryAllocationCount checks? #278

Open
taril42 opened this issue Jul 27, 2022 · 1 comment
Labels
bug Something isn't working investigating Still to be determined whether we work on this

Comments

@taril42
Copy link

taril42 commented Jul 27, 2022

We love MEMORY_USAGE_AUTO* and CREATE_WITHIN_BUDGET. It has allowed network forks to boot 4+ instances of the game for testing, and made the game more stable for artists working with editor + game + DCC tools on the side. (A few small points of friction on switching over that I'd be happy to elaborate on elsewhere, if desired.)

But the primary point of this ticket -- I think we have run into a place where fallback to dedicated memory can ignore CREATE_WITHIN_BUDGET and also maxMemoryAllocationCount tests. We're on 51c8b56 from July 22, 2022 (just last week).

An in-development game triggered maxMemoryAllocationCount failure in Vulkan validation layers. We had a ridiculous number of vfx creating small uniform buffers, and a congested DEVICE_LOCAL|HOST_VISIBLE|... default pool. I think I would have expected it to fall back to a different memory type in this case.

From what I can ascertain (snippets of dump below -- I can scrub the full thing for NDA'd material and upload if desired -- but this is a NV RTX 2060 SUPER on 516.59 drivers), we exceeded budget early with a combination of one ~28MB block alloc + a number of big 16MiB and 4MiB dedicated allocs. The 28MB block filled up.

    "Heap 2": {
      "Flags": ["DEVICE_LOCAL"], 
      "Size": 224395264, 
      "Budget": {
        "BudgetBytes": 14536704, 
        "UsageBytes": 209865088
      }, 
      "Stats": {
        "BlockCount": 4042, 
        "BlockBytes": 122520000, 
        "AllocationCount": 12036, 
        "AllocationBytes": 122506240, 
        "UnusedRangeCount": 162, 
        "AllocationSizeMin": 64, 
        "AllocationSizeMax": 16777216, 
        "UnusedRangeSizeMin": 64, 
        "UnusedRangeSizeMax": 832
      }, 
      "MemoryPools": {
        "Type 4": {
          "Flags": ["DEVICE_LOCAL", "HOST_VISIBLE", "HOST_COHERENT"], 
          "Stats": {
            "BlockCount": 4042, 
            "BlockBytes": 122520000, 
            "AllocationCount": 12036, 
            "AllocationBytes": 122506240, 
            "UnusedRangeCount": 162, 
            "AllocationSizeMin": 64, 
            "AllocationSizeMax": 16777216, 
            "UnusedRangeSizeMin": 64, 
            "UnusedRangeSizeMax": 832
          }
        }
      }
    }
  }, 
[...]
  "DefaultPools": {
    [...]
    "Type 4": {
      "PreferredBlockSize": 28049408, 
      "Blocks": {
        "0": {
          "MapRefCount": 7995, 
          "TotalBytes": 3506176, 
          "UnusedBytes": 13760, 
          "Allocations": 7995, 
          "UnusedRanges": 162, 
          "Suballocations": [...many entries...],
          ]
        }
      }, 
      "DedicatedAllocations": [...many entries...]

Reading code and assuming some things (I may be arbitrarily incorrect in my understanding in places), seems like what may have happened was:

AllocateMemory -> AllocateMemoryOfType hit this just fine:

            // Protection against creating each allocation as dedicated when we reach or exceed heap size/budget,
            // which can quickly deplete maxMemoryAllocationCount: Don't prefer dedicated allocations when above
            // 3/4 of the maximum allocation count.
            if(m_DeviceMemoryCount.load() > m_PhysicalDeviceProperties.limits.maxMemoryAllocationCount * 3 / 4)
            {
                dedicatedPreferred = false;
            }

...but then proceeded to fail res = blockVector.Allocate (guessing Allocate -> AllocatePage skipped CreateBlock due to the freeMemory test), and subsequently did:

        // Try dedicated memory.
        if(canAllocateDedicated && !dedicatedPreferred)
        {
            res = AllocateDedicatedMemory( /*...*/

It does not appear that it re-checks maxMemoryAllocationCount on this path.

It also looks like the check for "within budget" for dedicated allocs is at the top of AllocateMemoryOfType inside CalcMemTypeParams, but only happens with VMA_ALLOCATION_CREATE_DEDICATED_MEMORY_BIT, so we probably missed that budget check on this fallback path? (Had that check occurred, it might also have saved us. I might just be missing something here, though!)

Could probably artificially recreate this situation and step through, if needed.

We have a local workaround, I think: we kicked the 16M and 4M allocations out, here -- tagged them especially as "avoid HOST_ACCESS and create a staging buffer if needed". (We'd done something similar prior to VMA_MEMORY_USAGE_AUTO being available, so it was mostly just a matter of restoring that.) So we might be in the clear.

But it seems possible we'll run into this situation again when VK_EXT_memory_budget extensions report low numbers, or similar.

We have a few things we need to do:

  • scale back our VFX draw count =)
  • dedupe some of these buffers based on contents/usage
  • possibly have them share a single large buffer

...but it felt like something that might be interesting to report upstream.

We toyed with calling once with CREATE_NEVER_ALLOCATE and then again without, but this has the side effect of skipping the "Heuristics: Allocate dedicated memory if requested size if greater than half of preferred block size." stuff, which is maybe undesirable.

I thought of a few other ways to potentially address this within the vma code, but ... I'm not sure I fully grok the implications of any of these thoughts. Happy to describe or bodge together a PR or two for eyes, if helpful.

@adam-sawicki-a adam-sawicki-a added bug Something isn't working investigating Still to be determined whether we work on this labels Aug 22, 2022
@adam-sawicki-a
Copy link
Contributor

Thank you for reporting this issue and for the detailed description. We will investigate it.

A few small points of friction on switching over that I'd be happy to elaborate on elsewhere, if desired.

Any additional feedback is welcome. Please feel free to file a separate issue ticket on GitHub or contact me any way (e.g. via Twitter, LinkedIn) to discuss it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigating Still to be determined whether we work on this
Projects
None yet
Development

No branches or pull requests

2 participants