From 2decca5f27f74a449b2ac6de0e151b3410833a64 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 12 Nov 2015 16:33:15 +0100 Subject: [PATCH] Reorganise VkResourceRecord to reduce wasted space * This reduces the total size from 448 bytes -> 232 bytes, but does mean some extra pointer chasing. I'm not sure what the best balance is so this will do for now and I'll wait and see if profiling turns up any issues. --- renderdoc/core/resource_manager.h | 16 +-- renderdoc/driver/vulkan/vk_initstate.cpp | 10 +- renderdoc/driver/vulkan/vk_resources.cpp | 37 ++++-- renderdoc/driver/vulkan/vk_resources.h | 109 ++++++++++-------- .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 4 +- .../vulkan/wrappers/vk_descriptor_funcs.cpp | 19 +-- .../driver/vulkan/wrappers/vk_misc_funcs.cpp | 5 +- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 4 +- .../vulkan/wrappers/vk_resource_funcs.cpp | 5 - 9 files changed, 118 insertions(+), 91 deletions(-) diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h index 29b14de31..9a2f9ad41 100644 --- a/renderdoc/core/resource_manager.h +++ b/renderdoc/core/resource_manager.h @@ -261,17 +261,17 @@ struct ResourceRecord int32_t Length; - int UpdateCount : 29; - bool DataInSerialiser : 1; - bool SpecialResource : 1; // like the swap chain back buffers - bool DataWritten : 1; + int UpdateCount; + bool DataInSerialiser; + bool SpecialResource; // like the swap chain back buffers + bool DataWritten; protected: + int RefCount; + byte *DataPtr; uint64_t DataOffset; - int RefCount; - ResourceId ResID; std::set Parents; @@ -282,11 +282,11 @@ protected: return Atomic::Inc32(&globalIDCounter); } - - map m_FrameRefs; std::map m_Chunks; Threading::CriticalSection *m_ChunkLock; + + map m_FrameRefs; }; // the resource manager is a utility class that's not required but is likely wanted by any API implementation. diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp index 9b7661fa2..0ded4c588 100644 --- a/renderdoc/driver/vulkan/vk_initstate.cpp +++ b/renderdoc/driver/vulkan/vk_initstate.cpp @@ -936,8 +936,8 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res) if(type == eResDescriptorSet) { VkResourceRecord *record = GetResourceManager()->GetResourceRecord(id); - RDCASSERT(record->layout); - const DescSetLayout &layout = *record->layout; + RDCASSERT(record->descInfo && record->descInfo->layout); + const DescSetLayout &layout = *record->descInfo->layout; uint32_t numElems = 0; for(size_t i=0; i < layout.bindings.size(); i++) @@ -949,7 +949,7 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res) uint32_t e=0; for(size_t i=0; i < layout.bindings.size(); i++) for(uint32_t b=0; b < layout.bindings[i].arraySize; b++) - info[e++] = record->descBindings[i][b]; + info[e++] = record->descInfo->descBindings[i][b]; GetResourceManager()->SetInitialContents(id, VulkanResourceManager::InitialContentData(NULL, 0, (byte *)info)); return true; @@ -1229,8 +1229,8 @@ bool WrappedVulkan::Serialise_InitialState(WrappedVkRes *res) if(type == eResDescriptorSet) { VkResourceRecord *record = GetResourceManager()->GetResourceRecord(id); - RDCASSERT(record->layout); - const DescSetLayout &layout = *record->layout; + RDCASSERT(record->descInfo && record->descInfo->layout); + const DescSetLayout &layout = *record->descInfo->layout; VkDescriptorInfo *info = (VkDescriptorInfo *)initContents.blob; diff --git a/renderdoc/driver/vulkan/vk_resources.cpp b/renderdoc/driver/vulkan/vk_resources.cpp index 20cbfde5a..870076bd0 100644 --- a/renderdoc/driver/vulkan/vk_resources.cpp +++ b/renderdoc/driver/vulkan/vk_resources.cpp @@ -488,25 +488,38 @@ uint32_t GetByteSize(uint32_t Width, uint32_t Height, uint32_t Depth, VkFormat F VkResourceRecord::~VkResourceRecord() { - for(size_t i=0; i < descBindings.size(); i++) - delete[] descBindings[i]; - descBindings.clear(); + VkResourceType resType = IdentifyTypeByPtr(Resource); + + if(resType == eResPhysicalDevice) + SAFE_DELETE(memProps); - SAFE_DELETE(memProps); + // bufferviews and imageviews have non-owning pointers to the sparseinfo struct + if(resType == eResBuffer || resType == eResImage) + SAFE_DELETE(sparseInfo); - SAFE_DELETE(layout); - SAFE_DELETE(swapInfo); - SAFE_DELETE(cmdInfo); - if(memMapState) + if(resType == eResSwapchain) + SAFE_DELETE(swapInfo); + + if(resType == eResDeviceMemory && memMapState) { Serialiser::FreeAlignedBuffer(memMapState->refData); SAFE_DELETE(memMapState); } - if(sparseOwner) - { - SAFE_DELETE(sparseInfo); - } + + if(resType == eResCmdBuffer) + SAFE_DELETE(cmdInfo); + + if(resType == eResFramebuffer) + SAFE_DELETE(imageAttachments); + + // only the descriptor set layout actually owns this pointer, descriptor sets + // have a pointer to it but don't own it + if(resType == eResDescriptorSetLayout) + SAFE_DELETE(descInfo->layout); + + if(resType == eResDescriptorSetLayout || resType == eResDescriptorSet) + SAFE_DELETE(descInfo); } void SparseMapping::Update(uint32_t numBindings, const VkSparseImageMemoryBindInfo *pBindings) diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index e8207fdc4..e786e023c 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -623,6 +623,34 @@ struct CmdBufferRecordingInfo vector subcmds; }; +struct DescSetLayout; + +struct DescriptorSetData +{ + DescriptorSetData() : layout(NULL) {} + + ~DescriptorSetData() + { + for(size_t i=0; i < descBindings.size(); i++) + delete[] descBindings[i]; + descBindings.clear(); + } + + DescSetLayout *layout; + + // descriptor set bindings for this descriptor set. Filled out on + // create from the layout. + vector descBindings; + + // contains the framerefs (ref counted) for the bound resources + // in the binding slots. Updated when updating descriptor sets + // and then applied in a block on descriptor set bind. + // the refcount has the high-bit set if this resource has sparse + // mapping information + static const uint32_t SPARSE_REF_BIT = 0x80000000; + map > bindFrameRefs; +}; + struct MemMapState { MemMapState() @@ -636,8 +664,6 @@ struct MemMapState byte *refData; }; -struct DescSetLayout; - struct VkResourceRecord : public ResourceRecord { public: @@ -649,16 +675,8 @@ struct VkResourceRecord : public ResourceRecord ResourceRecord(id, true), bakedCommands(NULL), pool(NULL), - mem(VK_NULL_HANDLE), - memOffset(0), - memProps(NULL), memIdxMap(NULL), - sparseOwner(false), - sparseInfo(NULL), - layout(NULL), - swapInfo(NULL), - cmdInfo(NULL), - memMapState(NULL) + ptrunion(NULL) { } @@ -683,16 +701,16 @@ struct VkResourceRecord : public ResourceRecord return; } - if((bindFrameRefs[id].first&~SPARSE_REF_BIT) == 0) + if((descInfo->bindFrameRefs[id].first & ~DescriptorSetData::SPARSE_REF_BIT) == 0) { - bindFrameRefs[id] = std::make_pair(1 | (hasSparse ? SPARSE_REF_BIT : 0), ref); + descInfo->bindFrameRefs[id] = std::make_pair(1 | (hasSparse ? DescriptorSetData::SPARSE_REF_BIT : 0), ref); } else { // be conservative - mark refs as read before write if we see a write and a read ref on it - if(ref == eFrameRef_Write && bindFrameRefs[id].second == eFrameRef_Read) - bindFrameRefs[id].second = eFrameRef_ReadBeforeWrite; - bindFrameRefs[id].first++; + if(ref == eFrameRef_Write && descInfo->bindFrameRefs[id].second == eFrameRef_Read) + descInfo->bindFrameRefs[id].second = eFrameRef_ReadBeforeWrite; + descInfo->bindFrameRefs[id].first++; } } @@ -702,27 +720,30 @@ struct VkResourceRecord : public ResourceRecord // deleted since it was bound. if(id == ResourceId()) return; - auto it = bindFrameRefs.find(id); + auto it = descInfo->bindFrameRefs.find(id); // in the case of re-used handles bound to descriptor sets, // it's possible to try and remove a frameref on something we // don't have (which means we'll have a corresponding stale ref) // but this is harmless so we can ignore it. - if(it == bindFrameRefs.end()) return; + if(it == descInfo->bindFrameRefs.end()) return; it->second.first--; - if((it->second.first & ~SPARSE_REF_BIT) == 0) - bindFrameRefs.erase(it); + if((it->second.first & ~DescriptorSetData::SPARSE_REF_BIT) == 0) + descInfo->bindFrameRefs.erase(it); } + // we have a lot of 'cold' data in the resource record, as it can be accessed + // through the wrapped objects without locking any lookup structures. + // To save on object size, the data is union'd as much as possible where only + // one type of object's record will contain some data, disjoint with another. + // Some of these are pointers to resource-specific data (often STL structures), + // which means a lot of pointer chasing - need to determine if this is a + // performance issue + WrappedVkRes *Resource; - VkDeviceMemory mem; - VkDeviceSize memOffset; - - VkPhysicalDeviceMemoryProperties *memProps; - // externally allocated/freed, a mapping from memory idx // in our modified properties that were passed to the app // to the memory indices that actually exist @@ -732,38 +753,32 @@ struct VkResourceRecord : public ResourceRecord // ie. the resource that can be modified or changes (or can become dirty) // since typical memory bindings are immutable and must happen before // creation or use, this can always be determined - bool sparseOwner; ResourceId baseResource; ResourceId baseResourceMem; // for image views, we need to point to both the image and mem - SparseMapping *sparseInfo; - // framebuffers are the only object that can point to multiple resources - // (as each attachment has an image). - VkResourceRecord *imageAttachments[8]; - + // these are all disjoint, so only a record of the right type will have each + // Note some of these need to be deleted in the constructor, so we check the + // allocation type of the Resource + union + { + void *ptrunion; // for initialisation to NULL + VkPhysicalDeviceMemoryProperties *memProps; // only for physical devices + SparseMapping *sparseInfo; // only for buffers, images, and views of them + SwapchainInfo *swapInfo; // only for swapchains + MemMapState *memMapState; // only for device memory + CmdBufferRecordingInfo *cmdInfo; // only for command buffers + VkResourceRecord **imageAttachments; // only for framebuffers + DescriptorSetData *descInfo; // only for descriptor sets and descriptor set layouts + }; + VkResourceRecord *bakedCommands; - SwapchainInfo *swapInfo; - CmdBufferRecordingInfo *cmdInfo; - MemMapState *memMapState; + static const int MaxImageAttachments = 8; // pointer to either the pool this item is allocated from, or the children allocated // from this pool. Protected by the chunk lock VkResourceRecord *pool; vector pooledChildren; - - // descriptor set bindings for this descriptor set. Filled out on - // create from the layout. - DescSetLayout *layout; - vector descBindings; - - // contains the framerefs (ref counted) for the bound resources - // in the binding slots. Updated when updating descriptor sets - // and then applied in a block on descriptor set bind. - // the refcount has the high-bit set if this resource has sparse - // mapping information - static const uint32_t SPARSE_REF_BIT = 0x80000000; - map > bindFrameRefs; }; struct ImageLayouts diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index 0661e5658..fca8e954e 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -289,6 +289,7 @@ VkResult WrappedVulkan::vkBeginCommandBuffer( record->bakedCommands->Delete(GetResourceManager()); record->bakedCommands = GetResourceManager()->AddResourceRecord(ResourceIDGen::GetNewUniqueID()); + record->bakedCommands->Resource = (WrappedVkRes *)cmdBuffer; record->bakedCommands->cmdInfo = new CmdBufferRecordingInfo(); record->bakedCommands->cmdInfo->device = record->cmdInfo->device; @@ -601,9 +602,10 @@ void WrappedVulkan::vkCmdBeginRenderPass( VkResourceRecord *fb = GetRecord(pRenderPassBegin->framebuffer); record->MarkResourceFrameReferenced(fb->GetResourceID(), eFrameRef_Read); - for(size_t i=0; i < ARRAY_COUNT(fb->imageAttachments); i++) + for(size_t i=0; i < VkResourceRecord::MaxImageAttachments; i++) { if(fb->imageAttachments[i] == NULL) break; + record->MarkResourceFrameReferenced(fb->imageAttachments[i]->baseResource, eFrameRef_Write); if(fb->imageAttachments[i]->baseResourceMem != ResourceId()) record->MarkResourceFrameReferenced(fb->imageAttachments[i]->baseResourceMem, eFrameRef_Read); diff --git a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp index 659730b17..b756586c5 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp @@ -179,8 +179,9 @@ VkResult WrappedVulkan::vkCreateDescriptorSetLayout( VkResourceRecord *record = GetResourceManager()->AddResourceRecord(*pSetLayout); record->AddChunk(chunk); - record->layout = new DescSetLayout(); - record->layout->Init(GetResourceManager(), pCreateInfo); + record->descInfo = new DescriptorSetData(); + record->descInfo->layout = new DescSetLayout(); + record->descInfo->layout->Init(GetResourceManager(), pCreateInfo); } else { @@ -295,9 +296,9 @@ VkResult WrappedVulkan::vkAllocDescriptorSets( GetResourceManager()->MarkPendingDirty(id); } - record->layout = new DescSetLayout(); - *record->layout = *layoutRecord->layout; - record->layout->CreateBindingsArray(record->descBindings); + record->descInfo = new DescriptorSetData(); + record->descInfo->layout = layoutRecord->descInfo->layout; + record->descInfo->layout->CreateBindingsArray(record->descInfo->descBindings); } else { @@ -571,12 +572,12 @@ void WrappedVulkan::vkUpdateDescriptorSets( for(uint32_t i=0; i < writeCount; i++) { VkResourceRecord *record = GetRecord(pDescriptorWrites[i].destSet); - RDCASSERT(record->layout); - const DescSetLayout &layout = *record->layout; + RDCASSERT(record->descInfo && record->descInfo->layout); + const DescSetLayout &layout = *record->descInfo->layout; - RDCASSERT(pDescriptorWrites[i].destBinding < record->descBindings.size()); + RDCASSERT(pDescriptorWrites[i].destBinding < record->descInfo->descBindings.size()); - VkDescriptorInfo *binding = record->descBindings[pDescriptorWrites[i].destBinding]; + VkDescriptorInfo *binding = record->descInfo->descBindings[pDescriptorWrites[i].destBinding]; FrameRefType ref = eFrameRef_Write; diff --git a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp index c0bd030f4..4fd99eb24 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp @@ -407,9 +407,10 @@ VkResult WrappedVulkan::vkCreateFramebuffer( VkResourceRecord *record = GetResourceManager()->AddResourceRecord(*pFramebuffer); record->AddChunk(chunk); - RDCASSERT(pCreateInfo->attachmentCount < (uint32_t)ARRAY_COUNT(record->imageAttachments)); + record->imageAttachments = new VkResourceRecord*[VkResourceRecord::MaxImageAttachments]; + RDCASSERT(pCreateInfo->attachmentCount < VkResourceRecord::MaxImageAttachments); - RDCEraseEl(record->imageAttachments); + RDCEraseMem(record->imageAttachments, sizeof(ResourceId)*VkResourceRecord::MaxImageAttachments); if(pCreateInfo->renderPass != VK_NULL_HANDLE) record->AddParent(GetRecord(pCreateInfo->renderPass)); diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index d581b3a56..8be1b8740 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -395,12 +395,12 @@ VkResult WrappedVulkan::vkQueueSubmit( VkResourceRecord *setrecord = GetRecord(*it); - for(auto refit = setrecord->bindFrameRefs.begin(); refit != setrecord->bindFrameRefs.end(); ++refit) + for(auto refit = setrecord->descInfo->bindFrameRefs.begin(); refit != setrecord->descInfo->bindFrameRefs.end(); ++refit) { refdIDs.insert(refit->first); GetResourceManager()->MarkResourceFrameReferenced(refit->first, refit->second.second); - if(refit->second.first & VkResourceRecord::SPARSE_REF_BIT) + if(refit->second.first & DescriptorSetData::SPARSE_REF_BIT) { VkResourceRecord *record = GetResourceManager()->GetResourceRecord(refit->first); diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 91c4f973b..a9cdb1d37 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -645,9 +645,6 @@ VkResult WrappedVulkan::vkBindImageMemory( // Anything that looks up a baseResource for an image knows not to chase further // than the image. record->baseResource = GetResID(mem); - - record->mem = mem; - record->memOffset = memOffset; } return ObjDisp(device)->BindImageMemory(Unwrap(device), Unwrap(image), Unwrap(mem), memOffset); @@ -885,7 +882,6 @@ VkResult WrappedVulkan::vkCreateBuffer( if(pCreateInfo->flags & (VK_BUFFER_CREATE_SPARSE_BINDING_BIT|VK_BUFFER_CREATE_SPARSE_RESIDENCY_BIT)) { record->sparseInfo = new SparseMapping(); - record->sparseOwner = true; // buffers are always bound opaquely and in arbitrary divisions, sparse residency // only means not all the buffer needs to be bound, which is not that interesting for @@ -1086,7 +1082,6 @@ VkResult WrappedVulkan::vkCreateImage( if(pCreateInfo->flags & (VK_IMAGE_CREATE_SPARSE_BINDING_BIT|VK_IMAGE_CREATE_SPARSE_RESIDENCY_BIT)) { record->sparseInfo = new SparseMapping(); - record->sparseOwner = true; { SCOPED_LOCK(m_CapTransitionLock);