Skip to content

Commit c07e59a

Browse files
Fixed 2 bugs in defragmentation.
1st was when defragmenting mapped allocations. 2nd was a nasty one, when defragmentation moved allocation earlier in the same block. Also fixed some nullptr -> VMA_NULL, fixed compilation when VMA_HEAVY_ASSERT is enabled.
1 parent 1299c9a commit c07e59a

File tree

1 file changed

+104
-44
lines changed

1 file changed

+104
-44
lines changed

src/vk_mem_alloc.h

Lines changed: 104 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2916,7 +2916,7 @@ class VmaList
29162916
}
29172917
else
29182918
{
2919-
VMA_HEAVY_ASSERT(!m_pList.IsEmpty());
2919+
VMA_HEAVY_ASSERT(!m_pList->IsEmpty());
29202920
m_pItem = m_pList->Back();
29212921
}
29222922
return *this;
@@ -3243,14 +3243,9 @@ struct VmaAllocation_T
32433243
}
32443244

32453245
void ChangeBlockAllocation(
3246+
VmaAllocator hAllocator,
32463247
VmaDeviceMemoryBlock* block,
3247-
VkDeviceSize offset)
3248-
{
3249-
VMA_ASSERT(block != VMA_NULL);
3250-
VMA_ASSERT(m_Type == ALLOCATION_TYPE_BLOCK);
3251-
m_BlockAllocation.m_Block = block;
3252-
m_BlockAllocation.m_Offset = offset;
3253-
}
3248+
VkDeviceSize offset);
32543249

32553250
// pMappedData not null means allocation is created with MAPPED flag.
32563251
void InitDedicatedAllocation(
@@ -3337,7 +3332,7 @@ struct VmaAllocation_T
33373332
uint8_t m_Type; // ALLOCATION_TYPE
33383333
uint8_t m_SuballocationType; // VmaSuballocationType
33393334
// Bit 0x80 is set when allocation was created with VMA_ALLOCATION_CREATE_MAPPED_BIT.
3340-
// Bits with mask 0x7F, used only when ALLOCATION_TYPE_DEDICATED, are reference counter for vmaMapMemory()/vmaUnmapMemory().
3335+
// Bits with mask 0x7F are reference counter for vmaMapMemory()/vmaUnmapMemory().
33413336
uint8_t m_MapCount;
33423337
uint8_t m_Flags; // enum FLAGS
33433338

@@ -3472,6 +3467,7 @@ class VmaBlockMetadata
34723467

34733468
// Frees suballocation assigned to given memory region.
34743469
void Free(const VmaAllocation allocation);
3470+
void FreeAtOffset(VkDeviceSize offset);
34753471

34763472
private:
34773473
VkDeviceSize m_Size;
@@ -3523,8 +3519,8 @@ class VmaDeviceMemoryMapping
35233519
void* GetMappedData() const { return m_pMappedData; }
35243520

35253521
// ppData can be null.
3526-
VkResult Map(VmaAllocator hAllocator, VkDeviceMemory hMemory, void **ppData);
3527-
void Unmap(VmaAllocator hAllocator, VkDeviceMemory hMemory);
3522+
VkResult Map(VmaAllocator hAllocator, VkDeviceMemory hMemory, uint32_t count, void **ppData);
3523+
void Unmap(VmaAllocator hAllocator, VkDeviceMemory hMemory, uint32_t count);
35283524

35293525
private:
35303526
VMA_MUTEX m_Mutex;
@@ -3565,8 +3561,8 @@ class VmaDeviceMemoryBlock
35653561
bool Validate() const;
35663562

35673563
// ppData can be null.
3568-
VkResult Map(VmaAllocator hAllocator, void** ppData);
3569-
void Unmap(VmaAllocator hAllocator);
3564+
VkResult Map(VmaAllocator hAllocator, uint32_t count, void** ppData);
3565+
void Unmap(VmaAllocator hAllocator, uint32_t count);
35703566
};
35713567

35723568
struct VmaPointerLess
@@ -4388,6 +4384,28 @@ void VmaAllocation_T::SetUserData(VmaAllocator hAllocator, void* pUserData)
43884384
}
43894385
}
43904386

4387+
void VmaAllocation_T::ChangeBlockAllocation(
4388+
VmaAllocator hAllocator,
4389+
VmaDeviceMemoryBlock* block,
4390+
VkDeviceSize offset)
4391+
{
4392+
VMA_ASSERT(block != VMA_NULL);
4393+
VMA_ASSERT(m_Type == ALLOCATION_TYPE_BLOCK);
4394+
4395+
// Move mapping reference counter from old block to new block.
4396+
if(block != m_BlockAllocation.m_Block)
4397+
{
4398+
uint32_t mapRefCount = m_MapCount & ~MAP_COUNT_FLAG_PERSISTENT_MAP;
4399+
if(IsPersistentMap())
4400+
++mapRefCount;
4401+
m_BlockAllocation.m_Block->Unmap(hAllocator, mapRefCount);
4402+
block->Map(hAllocator, mapRefCount, VMA_NULL);
4403+
}
4404+
4405+
m_BlockAllocation.m_Block = block;
4406+
m_BlockAllocation.m_Offset = offset;
4407+
}
4408+
43914409
VkDeviceSize VmaAllocation_T::GetOffset() const
43924410
{
43934411
switch(m_Type)
@@ -4754,7 +4772,6 @@ bool VmaBlockMetadata::Validate() const
47544772
{
47554773
return false;
47564774
}
4757-
prevFree = currFree;
47584775

47594776
if(currFree != (subAlloc.hAllocation == VK_NULL_HANDLE))
47604777
{
@@ -4770,8 +4787,20 @@ bool VmaBlockMetadata::Validate() const
47704787
++freeSuballocationsToRegister;
47714788
}
47724789
}
4790+
else
4791+
{
4792+
if(subAlloc.hAllocation->GetOffset() != subAlloc.offset)
4793+
{
4794+
return false;
4795+
}
4796+
if(subAlloc.hAllocation->GetSize() != subAlloc.size)
4797+
{
4798+
return false;
4799+
}
4800+
}
47734801

47744802
calculatedOffset += subAlloc.size;
4803+
prevFree = currFree;
47754804
}
47764805

47774806
// Number of free suballocations registered in m_FreeSuballocationsBySize doesn't
@@ -4801,11 +4830,15 @@ bool VmaBlockMetadata::Validate() const
48014830
}
48024831

48034832
// Check if totals match calculacted values.
4804-
return
4805-
ValidateFreeSuballocationList() &&
4806-
(calculatedOffset == m_Size) &&
4807-
(calculatedSumFreeSize == m_SumFreeSize) &&
4808-
(calculatedFreeCount == m_FreeCount);
4833+
if(!ValidateFreeSuballocationList() ||
4834+
(calculatedOffset != m_Size) ||
4835+
(calculatedSumFreeSize != m_SumFreeSize) ||
4836+
(calculatedFreeCount != m_FreeCount))
4837+
{
4838+
return false;
4839+
}
4840+
4841+
return true;
48094842
}
48104843

48114844
VkDeviceSize VmaBlockMetadata::GetUnusedRangeSizeMax() const
@@ -5214,6 +5247,22 @@ void VmaBlockMetadata::Free(const VmaAllocation allocation)
52145247
VMA_ASSERT(0 && "Not found!");
52155248
}
52165249

5250+
void VmaBlockMetadata::FreeAtOffset(VkDeviceSize offset)
5251+
{
5252+
for(VmaSuballocationList::iterator suballocItem = m_Suballocations.begin();
5253+
suballocItem != m_Suballocations.end();
5254+
++suballocItem)
5255+
{
5256+
VmaSuballocation& suballoc = *suballocItem;
5257+
if(suballoc.offset == offset)
5258+
{
5259+
FreeSuballocation(suballocItem);
5260+
return;
5261+
}
5262+
}
5263+
VMA_ASSERT(0 && "Not found!");
5264+
}
5265+
52175266
bool VmaBlockMetadata::ValidateFreeSuballocationList() const
52185267
{
52195268
VkDeviceSize lastSize = 0;
@@ -5663,12 +5712,17 @@ VmaDeviceMemoryMapping::~VmaDeviceMemoryMapping()
56635712
VMA_ASSERT(m_MapCount == 0 && "VkDeviceMemory block is being destroyed while it is still mapped.");
56645713
}
56655714

5666-
VkResult VmaDeviceMemoryMapping::Map(VmaAllocator hAllocator, VkDeviceMemory hMemory, void **ppData)
5715+
VkResult VmaDeviceMemoryMapping::Map(VmaAllocator hAllocator, VkDeviceMemory hMemory, uint32_t count, void **ppData)
56675716
{
5717+
if(count == 0)
5718+
{
5719+
return VK_SUCCESS;
5720+
}
5721+
56685722
VmaMutexLock lock(m_Mutex, hAllocator->m_UseMutex);
56695723
if(m_MapCount != 0)
56705724
{
5671-
++m_MapCount;
5725+
m_MapCount += count;
56725726
VMA_ASSERT(m_pMappedData != VMA_NULL);
56735727
if(ppData != VMA_NULL)
56745728
{
@@ -5691,18 +5745,24 @@ VkResult VmaDeviceMemoryMapping::Map(VmaAllocator hAllocator, VkDeviceMemory hMe
56915745
{
56925746
*ppData = m_pMappedData;
56935747
}
5694-
m_MapCount = 1;
5748+
m_MapCount = count;
56955749
}
56965750
return result;
56975751
}
56985752
}
56995753

5700-
void VmaDeviceMemoryMapping::Unmap(VmaAllocator hAllocator, VkDeviceMemory hMemory)
5754+
void VmaDeviceMemoryMapping::Unmap(VmaAllocator hAllocator, VkDeviceMemory hMemory, uint32_t count)
57015755
{
5756+
if(count == 0)
5757+
{
5758+
return;
5759+
}
5760+
57025761
VmaMutexLock lock(m_Mutex, hAllocator->m_UseMutex);
5703-
if(m_MapCount != 0)
5762+
if(m_MapCount >= count)
57045763
{
5705-
if(--m_MapCount == 0)
5764+
m_MapCount -= count;
5765+
if(m_MapCount == 0)
57065766
{
57075767
m_pMappedData = VMA_NULL;
57085768
(*hAllocator->GetVulkanFunctions().vkUnmapMemory)(hAllocator->m_hDevice, hMemory);
@@ -5759,14 +5819,14 @@ bool VmaDeviceMemoryBlock::Validate() const
57595819
return m_Metadata.Validate();
57605820
}
57615821

5762-
VkResult VmaDeviceMemoryBlock::Map(VmaAllocator hAllocator, void** ppData)
5822+
VkResult VmaDeviceMemoryBlock::Map(VmaAllocator hAllocator, uint32_t count, void** ppData)
57635823
{
5764-
return m_Mapping.Map(hAllocator, m_hMemory, ppData);
5824+
return m_Mapping.Map(hAllocator, m_hMemory, count, ppData);
57655825
}
57665826

5767-
void VmaDeviceMemoryBlock::Unmap(VmaAllocator hAllocator)
5827+
void VmaDeviceMemoryBlock::Unmap(VmaAllocator hAllocator, uint32_t count)
57685828
{
5769-
m_Mapping.Unmap(hAllocator, m_hMemory);
5829+
m_Mapping.Unmap(hAllocator, m_hMemory, count);
57705830
}
57715831

57725832
static void InitStatInfo(VmaStatInfo& outInfo)
@@ -5924,7 +5984,7 @@ VkResult VmaBlockVector::Allocate(
59245984

59255985
if(mapped)
59265986
{
5927-
VkResult res = pCurrBlock->Map(m_hAllocator, nullptr);
5987+
VkResult res = pCurrBlock->Map(m_hAllocator, 1, VMA_NULL);
59285988
if(res != VK_SUCCESS)
59295989
{
59305990
return res;
@@ -6016,7 +6076,7 @@ VkResult VmaBlockVector::Allocate(
60166076

60176077
if(mapped)
60186078
{
6019-
res = pBlock->Map(m_hAllocator, nullptr);
6079+
res = pBlock->Map(m_hAllocator, 1, VMA_NULL);
60206080
if(res != VK_SUCCESS)
60216081
{
60226082
return res;
@@ -6093,7 +6153,7 @@ VkResult VmaBlockVector::Allocate(
60936153
{
60946154
if(mapped)
60956155
{
6096-
VkResult res = pBestRequestBlock->Map(m_hAllocator, nullptr);
6156+
VkResult res = pBestRequestBlock->Map(m_hAllocator, 1, VMA_NULL);
60976157
if(res != VK_SUCCESS)
60986158
{
60996159
return res;
@@ -6122,7 +6182,7 @@ VkResult VmaBlockVector::Allocate(
61226182
suballocType,
61236183
mapped,
61246184
(createInfo.flags & VMA_ALLOCATION_CREATE_CAN_BECOME_LOST_BIT) != 0);
6125-
VMA_HEAVY_ASSERT(pBlock->Validate());
6185+
VMA_HEAVY_ASSERT(pBestRequestBlock->Validate());
61266186
VMA_DEBUG_LOG(" Returned from existing allocation #%u", (uint32_t)blockIndex);
61276187
(*pAllocation)->SetUserData(m_hAllocator, createInfo.pUserData);
61286188
return VK_SUCCESS;
@@ -6160,7 +6220,7 @@ void VmaBlockVector::Free(
61606220

61616221
if(hAllocation->IsPersistentMap())
61626222
{
6163-
pBlock->m_Mapping.Unmap(m_hAllocator, pBlock->m_hMemory);
6223+
pBlock->m_Mapping.Unmap(m_hAllocator, pBlock->m_hMemory, 1);
61646224
}
61656225

61666226
pBlock->m_Metadata.Free(hAllocation);
@@ -6505,7 +6565,7 @@ VkResult VmaDefragmentator::BlockInfo::EnsureMapping(VmaAllocator hAllocator, vo
65056565
}
65066566

65076567
// Map on first usage.
6508-
VkResult res = m_pBlock->Map(hAllocator, &m_pMappedDataForDefragmentation);
6568+
VkResult res = m_pBlock->Map(hAllocator, 1, &m_pMappedDataForDefragmentation);
65096569
*ppMappedData = m_pMappedDataForDefragmentation;
65106570
return res;
65116571
}
@@ -6514,7 +6574,7 @@ void VmaDefragmentator::BlockInfo::Unmap(VmaAllocator hAllocator)
65146574
{
65156575
if(m_pMappedDataForDefragmentation != VMA_NULL)
65166576
{
6517-
m_pBlock->Unmap(hAllocator);
6577+
m_pBlock->Unmap(hAllocator, 1);
65186578
}
65196579
}
65206580

@@ -6610,10 +6670,10 @@ VkResult VmaDefragmentator::DefragmentRound(
66106670
static_cast<size_t>(size));
66116671

66126672
pDstBlockInfo->m_pBlock->m_Metadata.Alloc(dstAllocRequest, suballocType, size, allocInfo.m_hAllocation);
6613-
pSrcBlockInfo->m_pBlock->m_Metadata.Free(allocInfo.m_hAllocation);
6614-
6615-
allocInfo.m_hAllocation->ChangeBlockAllocation(pDstBlockInfo->m_pBlock, dstAllocRequest.offset);
6673+
pSrcBlockInfo->m_pBlock->m_Metadata.FreeAtOffset(srcOffset);
66166674

6675+
allocInfo.m_hAllocation->ChangeBlockAllocation(m_hAllocator, pDstBlockInfo->m_pBlock, dstAllocRequest.offset);
6676+
66176677
if(allocInfo.m_pChanged != VMA_NULL)
66186678
{
66196679
*allocInfo.m_pChanged = VK_TRUE;
@@ -7055,7 +7115,7 @@ VkResult VmaAllocator_T::AllocateDedicatedMemory(
70557115
return res;
70567116
}
70577117

7058-
void* pMappedData = nullptr;
7118+
void* pMappedData = VMA_NULL;
70597119
if(map)
70607120
{
70617121
res = (*m_VulkanFunctions.vkMapMemory)(
@@ -7391,7 +7451,7 @@ VkResult VmaAllocator_T::Defragment(
73917451
// Lost allocation cannot be defragmented.
73927452
(hAlloc->GetLastUseFrameIndex() != VMA_FRAME_INDEX_LOST))
73937453
{
7394-
VmaBlockVector* pAllocBlockVector = nullptr;
7454+
VmaBlockVector* pAllocBlockVector = VMA_NULL;
73957455

73967456
const VmaPool hAllocPool = hAlloc->GetPool();
73977457
// This allocation belongs to custom pool.
@@ -7655,8 +7715,8 @@ VkResult VmaAllocator_T::Map(VmaAllocation hAllocation, void** ppData)
76557715
case VmaAllocation_T::ALLOCATION_TYPE_BLOCK:
76567716
{
76577717
VmaDeviceMemoryBlock* const pBlock = hAllocation->GetBlock();
7658-
char *pBytes = nullptr;
7659-
VkResult res = pBlock->Map(this, (void**)&pBytes);
7718+
char *pBytes = VMA_NULL;
7719+
VkResult res = pBlock->Map(this, 1, (void**)&pBytes);
76607720
if(res == VK_SUCCESS)
76617721
{
76627722
*ppData = pBytes + (ptrdiff_t)hAllocation->GetOffset();
@@ -7680,7 +7740,7 @@ void VmaAllocator_T::Unmap(VmaAllocation hAllocation)
76807740
{
76817741
VmaDeviceMemoryBlock* const pBlock = hAllocation->GetBlock();
76827742
hAllocation->BlockAllocUnmap();
7683-
pBlock->Unmap(this);
7743+
pBlock->Unmap(this, 1);
76847744
}
76857745
break;
76867746
case VmaAllocation_T::ALLOCATION_TYPE_DEDICATED:

0 commit comments

Comments
 (0)