layers: Replace iterative DS Get.* with maps/sets

Profiling indicated hot spots in DescriptorSet Get functions which
iterated in binding space to lookup values.  Replaced iterative searches
with maps/sets.

Additionally simplified construction, optimized map/set creation and
Get.* for DescriptorSet and DescriptorSetLayout.

Change-Id: Ia2948e56333d3643d4377b39e75acf4c951d558b

diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index 2e64193..ed03840 100644 (file)
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -37,50 +37,53 @@ cvdescriptorset::DescriptorSetLayout::DescriptorSetLayout(const VkDescriptorSetL
       binding_count_(p_create_info->bindingCount),
       descriptor_count_(0),
       dynamic_descriptor_count_(0) {
-    // Dyn array indicies are ordered by binding # and array index of any array within the binding
-    //  so we store up bindings w/ count in ordered map in order to create dyn array mappings below
+    // Proactively reserve and resize as possible, as the reallocation was visible in profiling
+    std::set<uint32_t> sorted_bindings;
+    // Create the sorted set and unsorted map of bindings and indices
+    for (uint32_t i = 0; i < binding_count_; i++) {
+        sorted_bindings.insert(p_create_info->pBindings[i].binding);
+    }
+    uint32_t index = 0;
+    binding_to_index_map_.reserve(binding_count_);
+    for (const uint32_t binding_num : sorted_bindings) {
+        binding_to_index_map_[binding_num] = index++;
+    }
+
+    // Store the create info in the sorted order from above
     std::map<uint32_t, uint32_t> binding_to_dyn_count;
+    bindings_.resize(binding_count_);
     for (uint32_t i = 0; i < binding_count_; ++i) {
-        auto binding_num = p_create_info->pBindings[i].binding;
-        descriptor_count_ += p_create_info->pBindings[i].descriptorCount;
-        uint32_t insert_index = 0;  // Track vector index where we insert element
-        if (bindings_.empty() || binding_num > bindings_.back().binding) {
-            bindings_.push_back(safe_VkDescriptorSetLayoutBinding(&p_create_info->pBindings[i]));
-            insert_index = static_cast<uint32_t>(bindings_.size()) - 1;
-        } else {  // out-of-order binding number, need to insert into vector in-order
-            auto it = bindings_.begin();
-            // Find currently binding's spot in vector
-            while (binding_num > it->binding) {
-                assert(it != bindings_.end());
-                ++insert_index;
-                ++it;
-            }
-            bindings_.insert(it, safe_VkDescriptorSetLayoutBinding(&p_create_info->pBindings[i]));
-        }
-        // In cases where we should ignore pImmutableSamplers make sure it's NULL
-        if ((p_create_info->pBindings[i].pImmutableSamplers) &&
-            ((p_create_info->pBindings[i].descriptorType != VK_DESCRIPTOR_TYPE_SAMPLER) &&
-             (p_create_info->pBindings[i].descriptorType != VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER))) {
-            bindings_[insert_index].pImmutableSamplers = nullptr;
+        const auto binding_num = p_create_info->pBindings[i].binding;
+        auto &binding_info = bindings_[binding_to_index_map_[binding_num]];
+        binding_info = std::move(safe_VkDescriptorSetLayoutBinding(&p_create_info->pBindings[i]));
+        descriptor_count_ += binding_info.descriptorCount;
+        if (binding_info.descriptorCount > 0) {
+            non_empty_bindings_.insert(binding_num);
         }
-        if (p_create_info->pBindings[i].descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC ||
-            p_create_info->pBindings[i].descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC) {
-            binding_to_dyn_count[p_create_info->pBindings[i].binding] = p_create_info->pBindings[i].descriptorCount;
-            dynamic_descriptor_count_ += p_create_info->pBindings[i].descriptorCount;
+
+        if (binding_info.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC ||
+            binding_info.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC) {
+            binding_to_dyn_count[binding_num] = binding_info.descriptorCount;
+            dynamic_descriptor_count_ += binding_info.descriptorCount;
         }
     }
     assert(bindings_.size() == binding_count_);
     uint32_t global_index = 0;
-    // Vector order is finalized so create maps of bindings to indices
+    binding_to_global_index_range_map_.reserve(binding_count_);
+    // Vector order is finalized so create maps of bindings to descriptors and descriptors to indices
     for (uint32_t i = 0; i < binding_count_; ++i) {
         auto binding_num = bindings_[i].binding;
-        binding_to_index_map_[binding_num] = i;
         auto final_index = global_index + bindings_[i].descriptorCount;
         binding_to_global_index_range_map_[binding_num] = IndexRange(global_index, final_index);
+        if (final_index != global_index) {
+            global_start_to_index_map_[global_index] = i;
+        }
         global_index = final_index;
     }
+
     // Now create dyn offset array mapping for any dynamic descriptors
     uint32_t dyn_array_idx = 0;
+    binding_to_dynamic_array_idx_map_.reserve(binding_to_dyn_count.size());
     for (const auto &bc_pair : binding_to_dyn_count) {
         binding_to_dynamic_array_idx_map_[bc_pair.first] = dyn_array_idx;
         dyn_array_idx += bc_pair.second;
@@ -102,72 +105,54 @@ bool cvdescriptorset::DescriptorSetLayout::ValidateCreateInfo(debug_report_data
     return skip;
 }
 
-// put all bindings into the given set
-void cvdescriptorset::DescriptorSetLayout::FillBindingSet(std::unordered_set<uint32_t> *binding_set) const {
-    for (auto binding_index_pair : binding_to_index_map_) binding_set->insert(binding_index_pair.first);
-}
-
-VkDescriptorSetLayoutBinding const *cvdescriptorset::DescriptorSetLayout::GetDescriptorSetLayoutBindingPtrFromBinding(
-    const uint32_t binding) const {
+// Return valid index or "end" i.e. binding_count_;
+// The asserts in "Get" are reduced to the set where no valid answer(like null or 0) could be given
+// Common code for all binding lookups.
+uint32_t cvdescriptorset::DescriptorSetLayout::GetIndexFromBinding(uint32_t binding) const {
     const auto &bi_itr = binding_to_index_map_.find(binding);
-    if (bi_itr != binding_to_index_map_.end()) {
-        return bindings_[bi_itr->second].ptr();
-    }
-    return nullptr;
+    if (bi_itr != binding_to_index_map_.cend()) return bi_itr->second;
+    return GetBindingCount();
 }
 VkDescriptorSetLayoutBinding const *cvdescriptorset::DescriptorSetLayout::GetDescriptorSetLayoutBindingPtrFromIndex(
     const uint32_t index) const {
     if (index >= bindings_.size()) return nullptr;
     return bindings_[index].ptr();
 }
-// Return descriptorCount for given binding, 0 if index is unavailable
-uint32_t cvdescriptorset::DescriptorSetLayout::GetDescriptorCountFromBinding(const uint32_t binding) const {
-    const auto &bi_itr = binding_to_index_map_.find(binding);
-    if (bi_itr != binding_to_index_map_.end()) {
-        return bindings_[bi_itr->second].descriptorCount;
-    }
-    return 0;
-}
 // Return descriptorCount for given index, 0 if index is unavailable
 uint32_t cvdescriptorset::DescriptorSetLayout::GetDescriptorCountFromIndex(const uint32_t index) const {
     if (index >= bindings_.size()) return 0;
     return bindings_[index].descriptorCount;
 }
-// For the given binding, return descriptorType
-VkDescriptorType cvdescriptorset::DescriptorSetLayout::GetTypeFromBinding(const uint32_t binding) const {
-    assert(binding_to_index_map_.count(binding));
-    const auto &bi_itr = binding_to_index_map_.find(binding);
-    if (bi_itr != binding_to_index_map_.end()) {
-        return bindings_[bi_itr->second].descriptorType;
-    }
-    return VK_DESCRIPTOR_TYPE_MAX_ENUM;
-}
 // For the given index, return descriptorType
 VkDescriptorType cvdescriptorset::DescriptorSetLayout::GetTypeFromIndex(const uint32_t index) const {
     assert(index < bindings_.size());
-    return bindings_[index].descriptorType;
-}
-// For the given global index, return descriptorType
-//  Currently just counting up through bindings_, may improve this in future
-VkDescriptorType cvdescriptorset::DescriptorSetLayout::GetTypeFromGlobalIndex(const uint32_t index) const {
-    uint32_t global_offset = 0;
-    for (auto binding : bindings_) {
-        global_offset += binding.descriptorCount;
-        if (index < global_offset) return binding.descriptorType;
-    }
-    assert(0);  // requested global index is out of bounds
+    if (index < bindings_.size()) return bindings_[index].descriptorType;
     return VK_DESCRIPTOR_TYPE_MAX_ENUM;
 }
-// For the given binding, return stageFlags
-VkShaderStageFlags cvdescriptorset::DescriptorSetLayout::GetStageFlagsFromBinding(const uint32_t binding) const {
-    assert(binding_to_index_map_.count(binding));
-    const auto &bi_itr = binding_to_index_map_.find(binding);
-    if (bi_itr != binding_to_index_map_.end()) {
-        return bindings_[bi_itr->second].stageFlags;
-    }
+// For the given index, return stageFlags
+VkShaderStageFlags cvdescriptorset::DescriptorSetLayout::GetStageFlagsFromIndex(const uint32_t index) const {
+    assert(index < bindings_.size());
+    if (index < bindings_.size()) return bindings_[index].stageFlags;
     return VkShaderStageFlags(0);
 }
 
+// For the given global index, return index
+uint32_t cvdescriptorset::DescriptorSetLayout::GetIndexFromGlobalIndex(const uint32_t global_index) const {
+    auto start_it = global_start_to_index_map_.upper_bound(global_index);
+    uint32_t index = binding_count_;
+    assert(start_it != global_start_to_index_map_.cbegin());
+    if (start_it != global_start_to_index_map_.cbegin()) {
+        --start_it;
+        index = start_it->second;
+#ifndef NDEBUG
+        const auto &range = GetGlobalIndexRangeFromBinding(bindings_[index].binding);
+        assert(range.start <= global_index && global_index < range.end);
+#endif
+    }
+    return index;
+}
+
+// For the given binding, return the global index range
 // As start and end are often needed in pairs, get both with a single hash lookup.
 const cvdescriptorset::IndexRange &cvdescriptorset::DescriptorSetLayout::GetGlobalIndexRangeFromBinding(
     const uint32_t binding) const {
@@ -183,7 +168,6 @@ const cvdescriptorset::IndexRange &cvdescriptorset::DescriptorSetLayout::GetGlob
 
 // For given binding, return ptr to ImmutableSampler array
 VkSampler const *cvdescriptorset::DescriptorSetLayout::GetImmutableSamplerPtrFromBinding(const uint32_t binding) const {
-    assert(binding_to_index_map_.count(binding));
     const auto &bi_itr = binding_to_index_map_.find(binding);
     if (bi_itr != binding_to_index_map_.end()) {
         return bindings_[bi_itr->second].pImmutableSamplers;
@@ -192,16 +176,17 @@ VkSampler const *cvdescriptorset::DescriptorSetLayout::GetImmutableSamplerPtrFro
 }
 // Move to next valid binding having a non-zero binding count
 uint32_t cvdescriptorset::DescriptorSetLayout::GetNextValidBinding(const uint32_t binding) const {
-    uint32_t new_binding = binding;
-    do {
-        new_binding++;
-    } while (GetDescriptorCountFromBinding(new_binding) == 0);
-    return new_binding;
+    auto it = non_empty_bindings_.upper_bound(binding);
+    assert(it != non_empty_bindings_.cend());
+    if (it != non_empty_bindings_.cend()) return *it;
+    return GetMaxBinding() + 1;
 }
 // For given index, return ptr to ImmutableSampler array
 VkSampler const *cvdescriptorset::DescriptorSetLayout::GetImmutableSamplerPtrFromIndex(const uint32_t index) const {
-    assert(index < bindings_.size());
-    return bindings_[index].pImmutableSamplers;
+    if (index < bindings_.size()) {
+        return bindings_[index].pImmutableSamplers;
+    }
+    return nullptr;
 }
 // If our layout is compatible with rh_ds_layout, return true,
 //  else return false and fill in error_msg will description of what causes incompatibility
@@ -320,6 +305,7 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const V
       limits_(GetPhysDevProperties(dev_data)->properties.limits) {
     pool_state_ = GetDescriptorPoolState(dev_data, pool);
     // Foreach binding, create default descriptors of given type
+    descriptors_.reserve(p_layout_->GetTotalDescriptorCount());
     for (uint32_t i = 0; i < p_layout_->GetBindingCount(); ++i) {
         auto type = p_layout_->GetTypeFromIndex(i);
         switch (type) {
@@ -1186,6 +1172,7 @@ void cvdescriptorset::PerformUpdateDescriptorSetsWithTemplateKHR(layer_data *dev
         auto binding_being_updated = create_info.pDescriptorUpdateEntries[i].dstBinding;
         auto dst_array_element = create_info.pDescriptorUpdateEntries[i].dstArrayElement;
 
+        desc_writes.reserve(desc_writes.size() + create_info.pDescriptorUpdateEntries[i].descriptorCount);
         for (uint32_t j = 0; j < create_info.pDescriptorUpdateEntries[i].descriptorCount; j++) {
             desc_writes.emplace_back();
             auto &write_entry = desc_writes.back();

diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h
index 6a2ee6c..1ac2d0a 100644 (file)
--- a/layers/descriptor_sets.h
+++ b/layers/descriptor_sets.h
@@ -31,6 +31,7 @@
 #include "vk_object_types.h"
 #include <map>
 #include <memory>
+#include <set>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
@@ -91,8 +92,8 @@ class DescriptorSetLayout {
     VkDescriptorSetLayoutCreateFlags GetCreateFlags() const { return flags_; }
     // For a given binding, return the number of descriptors in that binding and all successive bindings
     uint32_t GetBindingCount() const { return binding_count_; };
-    // Fill passed-in set with bindings
-    void FillBindingSet(std::unordered_set<uint32_t> *) const;
+    // Non-empty binding numbers in order
+    const std::set<uint32_t> &GetSortedBindingSet() const { return non_empty_bindings_; }
     // Return true if given binding is present in this layout
     bool HasBinding(const uint32_t binding) const { return binding_to_index_map_.count(binding) > 0; };
     // Return true if this layout is compatible with passed in layout from a pipelineLayout,
@@ -100,16 +101,28 @@ class DescriptorSetLayout {
     bool IsCompatible(DescriptorSetLayout const *const, std::string *) const;
     // Return true if binding 1 beyond given exists and has same type, stageFlags & immutable sampler use
     bool IsNextBindingConsistent(const uint32_t) const;
+    uint32_t GetIndexFromBinding(uint32_t binding) const;
     // Various Get functions that can either be passed a binding#, which will
     //  be automatically translated into the appropriate index, or the index# can be passed in directly
-    VkDescriptorSetLayoutBinding const *GetDescriptorSetLayoutBindingPtrFromBinding(const uint32_t) const;
+    uint32_t GetMaxBinding() const { return bindings_[bindings_.size() - 1].binding; }
     VkDescriptorSetLayoutBinding const *GetDescriptorSetLayoutBindingPtrFromIndex(const uint32_t) const;
-    uint32_t GetDescriptorCountFromBinding(const uint32_t) const;
+    VkDescriptorSetLayoutBinding const *GetDescriptorSetLayoutBindingPtrFromBinding(uint32_t binding) const {
+        return GetDescriptorSetLayoutBindingPtrFromIndex(GetIndexFromBinding(binding));
+    }
     uint32_t GetDescriptorCountFromIndex(const uint32_t) const;
-    VkDescriptorType GetTypeFromBinding(const uint32_t) const;
+    uint32_t GetDescriptorCountFromBinding(const uint32_t binding) const {
+        return GetDescriptorCountFromIndex(GetIndexFromBinding(binding));
+    }
     VkDescriptorType GetTypeFromIndex(const uint32_t) const;
-    VkDescriptorType GetTypeFromGlobalIndex(const uint32_t) const;
-    VkShaderStageFlags GetStageFlagsFromBinding(const uint32_t) const;
+    VkDescriptorType GetTypeFromBinding(const uint32_t binding) const { return GetTypeFromIndex(GetIndexFromBinding(binding)); }
+    VkShaderStageFlags GetStageFlagsFromIndex(const uint32_t) const;
+    VkShaderStageFlags GetStageFlagsFromBinding(const uint32_t binding) const {
+        return GetStageFlagsFromIndex(GetIndexFromBinding(binding));
+    }
+    uint32_t GetIndexFromGlobalIndex(const uint32_t global_index) const;
+    VkDescriptorType GetTypeFromGlobalIndex(const uint32_t global_index) const {
+        return GetTypeFromIndex(GetIndexFromGlobalIndex(global_index));
+    }
     VkSampler const *GetImmutableSamplerPtrFromBinding(const uint32_t) const;
     VkSampler const *GetImmutableSamplerPtrFromIndex(const uint32_t) const;
     // For a given binding and array index, return the corresponding index into the dynamic offset array
@@ -134,7 +147,10 @@ class DescriptorSetLayout {
 
    private:
     VkDescriptorSetLayout layout_;
-    std::map<uint32_t, uint32_t> binding_to_index_map_;
+    std::set<uint32_t> non_empty_bindings_;  // Containing non-emtpy bindings in numerical order
+    std::unordered_map<uint32_t, uint32_t> binding_to_index_map_;
+    // The following map allows an non-iterative lookup of a binding from a global index...
+    std::map<uint32_t, uint32_t> global_start_to_index_map_;  // The index corresponding for a starting global (descriptor) index
     std::unordered_map<uint32_t, IndexRange> binding_to_global_index_range_map_;  // range is exclusive of .end
     // For a given binding map to associated index in the dynamic offset array
     std::unordered_map<uint32_t, uint32_t> binding_to_dynamic_array_idx_map_;