From 8dd8a8bed72622471a89fe409dcbbb1da7b58af2 Mon Sep 17 00:00:00 2001 From: AWoloszyn Date: Wed, 12 Sep 2018 14:24:11 -0400 Subject: [PATCH] Update how we handle descriptor sets again. This provides some performance benefit. --- gapis/api/vulkan/api/coherent_memory.api | 60 +++++++++++++++++-- gapis/api/vulkan/api/descriptor.api | 40 ++++++++++++- gapis/api/vulkan/api/queue.api | 4 +- .../vulkan/api/queued_command_tracking.api | 5 +- gapis/api/vulkan/api/synchronization.api | 2 +- gapis/api/vulkan/vulkan.api | 11 ++++ 6 files changed, 110 insertions(+), 12 deletions(-) diff --git a/gapis/api/vulkan/api/coherent_memory.api b/gapis/api/vulkan/api/coherent_memory.api index 30fe36a53e..45139ff20b 100644 --- a/gapis/api/vulkan/api/coherent_memory.api +++ b/gapis/api/vulkan/api/coherent_memory.api @@ -107,7 +107,7 @@ sub void readMemoryInImageBindings(map!(u32, ref!VkDescriptorImageInfo) imageBin } } -sub void readMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map!(u32, map!(u32, VkDeviceSize)) bufferBindingOffsets) { +sub void readMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map!(u32, map!(u32, VkDeviceSize)) bufferBindingOffsets, bool processImages) { if descriptor_set != null { for i in (0 .. len(descriptor_set.Bindings)) { binding := descriptor_set.Bindings[as!u32(i)] @@ -131,7 +131,9 @@ sub void readMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map!( VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: + if (processImages) { readMemoryInImageBindings(binding.ImageBinding) + } default: {} } } @@ -196,7 +198,7 @@ sub void writeMemoryInImageBindings(map!(u32, ref!VkDescriptorImageInfo) imageBi } } -sub void writeMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map!(u32, map!(u32, VkDeviceSize)) bufferBindingOffsets) { +sub void writeMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map!(u32, map!(u32, VkDeviceSize)) bufferBindingOffsets, bool processImages) { if descriptor_set != null { for i in (0 .. len(descriptor_set.Bindings)) { binding := descriptor_set.Bindings[as!u32(i)] @@ -212,22 +214,68 @@ sub void writeMemoryInDescriptorSet(ref!DescriptorSetObject descriptor_set, map! case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: writeMemoryInBufferViewBindings(binding.BufferViewBindings) case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: - writeMemoryInImageBindings(binding.ImageBinding) + if (processImages) { + writeMemoryInImageBindings(binding.ImageBinding) + } default: {} } } } } - sub void readWriteMemoryInBoundDescriptorSets( ref!PipelineLayoutObject pipelineLayout, map!(u32, ref!DescriptorSetObject) descriptorSets, map!(u32, map!(u32, map!(u32, VkDeviceSize))) bufferBindingOffsets) { layerCount := len(pipelineLayout.SetLayouts) for i in (0 .. layerCount) { - readMemoryInDescriptorSet(descriptorSets[as!u32(i)], bufferBindingOffsets[as!u32(i)]) - writeMemoryInDescriptorSet(descriptorSets[as!u32(i)], bufferBindingOffsets[as!u32(i)]) + descriptorSet := descriptorSets[as!u32(i)] + if descriptorSet != null { + if !(descriptorSet.VulkanHandle in ProcessedDescriptorSets.val) { + offsets := bufferBindingOffsets[as!u32(i)] + readMemoryInDescriptorSet(descriptorSet, offsets, true) + writeMemoryInDescriptorSet(descriptorSet, offsets, true) + + descriptors := ProcessedDescriptorSet() + descriptors.shouldReadBuffers = false + newOffsets := descriptors.bufferOffsets + for _, j, jj in offsets { + for _, k, v in jj { + if (!newOffsets[j][k][v]) { + x := newOffsets[j] + y := x[k] + y[v] = true + x[k] = y + newOffsets[j] = x + } + + } + } + descriptors.bufferOffsets = newOffsets + ProcessedDescriptorSets.val[descriptorSet.VulkanHandle] = descriptors + } else if ProcessedDescriptorSets.val[descriptorSet.VulkanHandle].shouldReadBuffers { + offsets := bufferBindingOffsets[as!u32(i)] + readMemoryInDescriptorSet(descriptorSet, offsets, false) + writeMemoryInDescriptorSet(descriptorSet, offsets, false) + + descriptors := ProcessedDescriptorSets.val[descriptorSet.VulkanHandle] + descriptors.shouldReadBuffers = false + newOffsets := descriptors.bufferOffsets + for _, j, jj in offsets { + for _, k, v in jj { + if (!newOffsets[j][k][v]) { + x := newOffsets[j] + y := x[k] + y[v] = true + x[k] = y + newOffsets[j] = x + } + } + } + descriptors.bufferOffsets = newOffsets + ProcessedDescriptorSets.val[descriptorSet.VulkanHandle] = descriptors + } + } } } diff --git a/gapis/api/vulkan/api/descriptor.api b/gapis/api/vulkan/api/descriptor.api index 0e68829d63..014c685db7 100644 --- a/gapis/api/vulkan/api/descriptor.api +++ b/gapis/api/vulkan/api/descriptor.api @@ -671,6 +671,11 @@ vkCmdBindDescriptorSetsArgs { map!(u32, u32) DynamicOffsets } +@internal class +emptyBufferOffsets { + map!(u32, map!(u32, map!(VkDeviceSize, bool))) BufferBindingOffsets +} + sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { _ = PipelineLayouts[args.Layout] dynamic_offset_index := MutableU32(0) @@ -691,14 +696,29 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { case VK_PIPELINE_BIND_POINT_GRAPHICS: drawInfo.BufferBindingOffsets } - + for i in (0 .. len(args.DescriptorSets)) { if args.DescriptorSets[as!u32(i)] in DescriptorSets { + + set := args.DescriptorSets[as!u32(i)] setObj := DescriptorSets[set] currentDescriptorSets[args.FirstSet + as!u32(i)] = DescriptorSets[set] desc_set_buf_offsets := bufferBindingOffsets[as!u32(i)+args.FirstSet] + hasBeenRead := switch (set in ProcessedDescriptorSets.val) { + case true: + MutableBool(true) + default: + MutableBool(false) + } + existingOffsets := switch(hasBeenRead.b) { + case true: + ProcessedDescriptorSets.val[set].bufferOffsets + case false: + emptyBufferOffsets().BufferBindingOffsets + } + // Since the pDynamicOffsets point into the bindings in order of // binding index, and then array index, we have to loop over all // of the Bindings in order of the binding number. @@ -722,6 +742,24 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { dynamic_offset_index.Val } desc_binding_buf_offsets[as!u32(k)] = binding_offset + + // If the offset has changed, then we may have to reprocess this + // descriptor set. + if hasBeenRead.b { + if !(as!u32(j) in existingOffsets[as!u32(i)]) { + hasBeenRead.b = false + } else if !(as!u32(k) in existingOffsets[as!u32(j)]) { + hasBeenRead.b = false + } else if !(binding_offset in existingOffsets[as!u32(j)][as!u32(k)]) { + hasBeenRead.b = false + } + + if !hasBeenRead.b { + oldSets := ProcessedDescriptorSets.val[set].bufferOffsets + ProcessedDescriptorSets.val[set] = + ProcessedDescriptorSet(oldSets, true) + } + } } desc_set_buf_offsets[as!u32(j)] = desc_binding_buf_offsets } diff --git a/gapis/api/vulkan/api/queue.api b/gapis/api/vulkan/api/queue.api index f12f19f585..18507e5e56 100644 --- a/gapis/api/vulkan/api/queue.api +++ b/gapis/api/vulkan/api/queue.api @@ -181,7 +181,7 @@ cmd VkResult vkQueueSubmit( as!VkSemaphore(0), null, fence) } - execPendingCommands(queue) + execPendingCommands(queue, true) fence // 'fence' keyword, marking the point where observed memory writes become visible return ? @@ -338,7 +338,7 @@ cmd VkResult vkQueueBindSparse( as!VkSemaphore(0), null, fence) } - execPendingCommands(queue) + execPendingCommands(queue, true) fence return ? } diff --git a/gapis/api/vulkan/api/queued_command_tracking.api b/gapis/api/vulkan/api/queued_command_tracking.api index b88a425c79..8aaecbce8f 100644 --- a/gapis/api/vulkan/api/queued_command_tracking.api +++ b/gapis/api/vulkan/api/queued_command_tracking.api @@ -40,12 +40,13 @@ dense_map!(u32, VkQueue) m } -sub void execPendingCommands(VkQueue queue) { +sub void execPendingCommands(VkQueue queue, bool isRoot) { q := Queues[queue] newCmds := emptyMap().m signaledQueues := queueMap().m processSubcommand := MutableBool() processSubcommand.b = true + ProcessedDescriptorSets = UsedDescriptorSets() for i in (0 .. len(q.PendingCommands)) { cmd := q.PendingCommands[as!u32(i)] @@ -210,7 +211,7 @@ sub void execPendingCommands(VkQueue queue) { resetSubcontext() for i in (0 .. len(signaledQueues)) { - execPendingCommands(signaledQueues[as!u32(i)]) + execPendingCommands(signaledQueues[as!u32(i)], false) } } diff --git a/gapis/api/vulkan/api/synchronization.api b/gapis/api/vulkan/api/synchronization.api index 29e6c8a354..ae3bc46758 100644 --- a/gapis/api/vulkan/api/synchronization.api +++ b/gapis/api/vulkan/api/synchronization.api @@ -245,7 +245,7 @@ cmd VkResult vkSetEvent( // event list in the queue object, we should roll out the pending commands if len(q.PendingEvents) == 0 { LastBoundQueue = Queues[queue] - execPendingCommands(queue) + execPendingCommands(queue, true) } } fence diff --git a/gapis/api/vulkan/vulkan.api b/gapis/api/vulkan/vulkan.api index ffac31c529..f6c86c1369 100644 --- a/gapis/api/vulkan/vulkan.api +++ b/gapis/api/vulkan/vulkan.api @@ -313,6 +313,17 @@ enum LastSubmissionType { @serialize map!(VkQueue, ref!ComputeInfo) LastComputeInfos @serialize PresentInfo LastPresentInfo @serialize LastSubmissionType LastSubmission +@hidden @serialize UsedDescriptorSets ProcessedDescriptorSets + + +@internal class ProcessedDescriptorSet { + map!(u32, map!(u32, map!(VkDeviceSize, bool))) bufferOffsets + bool shouldReadBuffers +} + +@internal class UsedDescriptorSets { + map!(VkDescriptorSet, ProcessedDescriptorSet) val +} @hidden bool IsRebuilding