From 0b061f4565edb70a6219d3a3ddb2cd1a3cebeb53 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 31 Jul 2020 13:45:01 +0100 Subject: [PATCH] Stop tracking dirty state at fine-grained detail on vulkan * Almost all dirty-able resources (memory and images) become dirty almost immediately, so spending time tracking dirty state is wasted. Instead we treat these resources as dirty at creation and rely on the postponing logic to avoid preparing initial states for newly created resources that are not used in the frame. * This may cause more 'last-minute' postponed prepares for newly created resources, which would previously. --- renderdoc/core/resource_manager.h | 2 +- renderdoc/driver/vulkan/vk_resources.cpp | 10 ----- renderdoc/driver/vulkan/vk_resources.h | 4 -- .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 2 - .../driver/vulkan/wrappers/vk_draw_funcs.cpp | 2 - .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 34 ---------------- .../vulkan/wrappers/vk_resource_funcs.cpp | 39 +++++++++++-------- 7 files changed, 24 insertions(+), 69 deletions(-) diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h index 3153694f7..ee1adfec3 100644 --- a/renderdoc/core/resource_manager.h +++ b/renderdoc/core/resource_manager.h @@ -1006,7 +1006,7 @@ void ResourceManager::Prepare_InitialStateIfPostponed(ResourceId return; WrappedResourceType res = GetCurrentResource(id); - RDCDEBUG("Preparing resource %s after it has been postponed.", ToStr(id).c_str()); + RDCLOG("Preparing resource %s after it has been postponed.", ToStr(id).c_str()); Prepare_InitialState(res); m_PostponedResourceIDs.erase(id); diff --git a/renderdoc/driver/vulkan/vk_resources.cpp b/renderdoc/driver/vulkan/vk_resources.cpp index 32be72aa1..d4a3d0d26 100644 --- a/renderdoc/driver/vulkan/vk_resources.cpp +++ b/renderdoc/driver/vulkan/vk_resources.cpp @@ -3771,8 +3771,6 @@ void VkResourceRecord::MarkImageFrameReferenced(VkResourceRecord *img, const Ima MarkResourceFrameReferenced(img->baseResource, refType); ResourceId id = img->GetResourceID(); - if(refType != eFrameRef_Read && refType != eFrameRef_None) - cmdInfo->dirtied.insert(id); if(img->resInfo && img->resInfo->IsSparse()) cmdInfo->sparse.insert(img->resInfo); @@ -3798,9 +3796,6 @@ void VkResourceRecord::MarkImageViewFrameReferenced(VkResourceRecord *view, cons // mark memory backing image MarkResourceFrameReferenced(mem, refType); - if(refType != eFrameRef_Read && refType != eFrameRef_None) - cmdInfo->dirtied.insert(img); - ImageSubresourceRange imgRange; imgRange.aspectMask = view->viewRange.aspectMask; @@ -3837,8 +3832,6 @@ void VkResourceRecord::MarkImageViewFrameReferenced(VkResourceRecord *view, cons void VkResourceRecord::MarkMemoryFrameReferenced(ResourceId mem, VkDeviceSize offset, VkDeviceSize size, FrameRefType refType) { - if(refType != eFrameRef_Read && refType != eFrameRef_None) - cmdInfo->dirtied.insert(mem); FrameRefType maxRef = MarkMemoryReferenced(cmdInfo->memFrameRefs, mem, offset, size, refType); MarkResourceFrameReferenced(mem, maxRef, ComposeFrameRefsDisjoint); } @@ -3865,9 +3858,6 @@ void VkResourceRecord::MarkBufferImageCopyFrameReferenced(VkResourceRecord *buf, FrameRefType bufRefType, FrameRefType imgRefType) { - if(IsDirtyFrameRef(imgRefType)) - cmdInfo->dirtied.insert(img->GetResourceID()); - // mark buffer just as read MarkResourceFrameReferenced(buf->GetResourceID(), eFrameRef_Read); diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index e5a469140..744705ad6 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1013,9 +1013,6 @@ struct CmdBufferRecordingInfo // need to go through the sparse mapping and reference all memory) std::set sparse; - // a list of all resources dirtied by this command buffer - std::set dirtied; - // a list of descriptor sets that are bound at any point in this command buffer // used to look up all the frame refs per-desc set and apply them on queue // submit with latest binding refs. @@ -2111,7 +2108,6 @@ public: { RDCASSERT(cmdInfo); SwapChunks(bakedCommands); - cmdInfo->dirtied.swap(bakedCommands->cmdInfo->dirtied); cmdInfo->boundDescSets.swap(bakedCommands->cmdInfo->boundDescSets); cmdInfo->subcmds.swap(bakedCommands->cmdInfo->subcmds); cmdInfo->sparse.swap(bakedCommands->cmdInfo->sparse); diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index c0b0a15dc..3e66166a2 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -4144,8 +4144,6 @@ void WrappedVulkan::vkCmdExecuteCommands(VkCommandBuffer commandBuffer, uint32_t VkResourceRecord *execRecord = GetRecord(pCommandBuffers[i]); if(execRecord->bakedCommands) { - record->cmdInfo->dirtied.insert(execRecord->bakedCommands->cmdInfo->dirtied.begin(), - execRecord->bakedCommands->cmdInfo->dirtied.end()); record->cmdInfo->boundDescSets.insert( execRecord->bakedCommands->cmdInfo->boundDescSets.begin(), execRecord->bakedCommands->cmdInfo->boundDescSets.end()); diff --git a/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp index 1494f471a..f2904fe49 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp @@ -2275,7 +2275,6 @@ void WrappedVulkan::vkCmdClearColorImage(VkCommandBuffer commandBuffer, VkImage record->AddChunk(scope.Get()); record->MarkResourceFrameReferenced(GetRecord(image)->baseResource, eFrameRef_Read); - record->cmdInfo->dirtied.insert(GetResID(image)); VkResourceRecord *imageRecord = GetRecord(image); if(imageRecord->resInfo && imageRecord->resInfo->IsSparse()) record->cmdInfo->sparse.insert(imageRecord->resInfo); @@ -2394,7 +2393,6 @@ void WrappedVulkan::vkCmdClearDepthStencilImage(VkCommandBuffer commandBuffer, V record->AddChunk(scope.Get()); record->MarkResourceFrameReferenced(GetResID(image), eFrameRef_PartialWrite); record->MarkResourceFrameReferenced(GetRecord(image)->baseResource, eFrameRef_Read); - record->cmdInfo->dirtied.insert(GetResID(image)); VkResourceRecord *imageRecord = GetRecord(image); if(imageRecord->resInfo && imageRecord->resInfo->IsSparse()) record->cmdInfo->sparse.insert(imageRecord->resInfo); diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 912155a9b..65f765820 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -906,36 +906,6 @@ VkResult WrappedVulkan::vkQueueSubmit(VkQueue queue, uint32_t submitCount, UpdateImageStates(record->bakedCommands->cmdInfo->imageStates); - for(auto it = record->bakedCommands->cmdInfo->dirtied.begin(); - it != record->bakedCommands->cmdInfo->dirtied.end(); ++it) - { - if(GetResourceManager()->HasCurrentResource(*it)) - GetResourceManager()->MarkDirtyResource(*it); - } - - // with EXT_descriptor_indexing a binding might have been updated after - // vkCmdBindDescriptorSets, so we need to track dirtied here at the last second. - for(auto it = record->bakedCommands->cmdInfo->boundDescSets.begin(); - it != record->bakedCommands->cmdInfo->boundDescSets.end(); ++it) - { - VkResourceRecord *setrecord = GetRecord(*it); - - SCOPED_LOCK(setrecord->descInfo->refLock); - - const std::map> &frameRefs = - setrecord->descInfo->bindFrameRefs; - - for(auto refit = frameRefs.begin(); refit != frameRefs.end(); ++refit) - { - if(refit->second.second == eFrameRef_PartialWrite || - refit->second.second == eFrameRef_ReadBeforeWrite) - { - if(GetResourceManager()->HasCurrentResource(refit->first)) - GetResourceManager()->MarkDirtyResource(refit->first); - } - } - } - if(capframe) { // for each bound descriptor set, mark it referenced as well as all resources currently @@ -1005,8 +975,6 @@ VkResult WrappedVulkan::vkQueueSubmit(VkQueue queue, uint32_t submitCount, record->bakedCommands->AddRef(); } - - record->cmdInfo->dirtied.clear(); } } @@ -1147,8 +1115,6 @@ VkResult WrappedVulkan::vkQueueSubmit(VkQueue queue, uint32_t submitCount, vkFlushMappedMemoryRanges(dev, 1, &range); state.mapFlushed = false; } - - GetResourceManager()->MarkDirtyResource(record->GetResourceID()); } else { diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 3c39531d2..3c748c251 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -478,6 +478,13 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate } GetResourceManager()->AddDeviceMemory(id); + + // the memory is immediately dirty because we don't use dirty tracking, it's too expensive to + // follow all frame refs in the background and it's pointless because memory almost always + // immediately becomes dirty anyway. The one case we might care about non-dirty memory is + // memory that has been allocated but not used, but that will be skipped or postponed as + // appropriate. + GetResourceManager()->MarkDirtyResource(id); } else { @@ -706,7 +713,6 @@ void WrappedVulkan::vkUnmapMemory(VkDevice device, VkDeviceMemory mem) if(!capframe) { GetResourceManager()->MarkResourceFrameReferenced(id, eFrameRef_PartialWrite); - GetResourceManager()->MarkDirtyResource(id); } } @@ -910,7 +916,6 @@ VkResult WrappedVulkan::vkFlushMappedMemoryRanges(VkDevice device, uint32_t memR refType = eFrameRef_CompleteWrite; GetResourceManager()->MarkResourceFrameReferenced(memid, refType); - GetResourceManager()->MarkDirtyResource(memid); } } } @@ -1034,9 +1039,6 @@ VkResult WrappedVulkan::vkBindBufferMemory(VkDevice device, VkBuffer buffer, VkD eFrameRef_ReadBeforeWrite); GetResourceManager()->MarkMemoryFrameReferenced(id, memoryOffset, record->memSize, eFrameRef_ReadBeforeWrite); - - // the memory is immediately dirty because we have no way of tracking writes to it - GetResourceManager()->MarkDirtyResource(id); } } @@ -1810,18 +1812,26 @@ VkResult WrappedVulkan::vkCreateImage(VkDevice device, const VkImageCreateInfo * next = next->pNext; } - // sparse and external images are considered dirty from creation. For sparse images this is - // so that we can serialise the tracked page table, for external images this is so we can be - // sure to fetch their contents even if we don't see any writes. + // the image is immediately dirty because we don't use dirty tracking, it's too expensive to + // follow all frame refs in the background and it's pointless because memory almost always + // immediately becomes dirty anyway. The one case we might care about non-dirty memory is + // memory that has been allocated but not used, but that will be skipped or postponed as + // appropriate. + GetResourceManager()->MarkDirtyResource(id); + GetResourceManager()->MarkResourceFrameReferenced(id, eFrameRef_ReadBeforeWrite); + + // sparse and external images should be considered dirty from creation anyway. For sparse + // images this is so that we can serialise the tracked page table, for external images this is + // so we can be sure to fetch their contents even if we don't see any writes. // - // We also dirty linear images since we may not get another chance - if they are bound to - // host-visible memory they may only be updated via memory maps, and we want to be sure to - // correctly copy their initial contents out rather than relying on memory contents (which may - // not be valid to map from/into if the image isn't in GENERAL layout). + // We also should consider linear images dirty since we may not get another chance - if they + // are bound to host-visible memory they may only be updated via memory maps, and we want to + // be sure to correctly copy their initial contents out rather than relying on memory contents + // (which may not be valid to map from/into if the image isn't in GENERAL layout). if(isSparse || isExternal || isLinear) { - GetResourceManager()->MarkResourceFrameReferenced(id, eFrameRef_ReadBeforeWrite); GetResourceManager()->MarkDirtyResource(id); + GetResourceManager()->MarkResourceFrameReferenced(id, eFrameRef_ReadBeforeWrite); // for external images, try creating a non-external version and take the worst case of // memory requirements, in case the non-external one (as we will replay it) needs more @@ -2155,9 +2165,6 @@ VkResult WrappedVulkan::vkBindBufferMemory2(VkDevice device, uint32_t bindInfoCo GetResourceManager()->MarkMemoryFrameReferenced( GetResID(pBindInfos[i].memory), pBindInfos[i].memoryOffset, bufrecord->memSize, eFrameRef_ReadBeforeWrite); - - // the memory is immediately dirty because we have no way of tracking writes to it - GetResourceManager()->MarkDirtyResource(GetResID(pBindInfos[i].memory)); } } }