Add config option to reuse descriptor_set

On Mali devices, using the ARM DDK, some application quickly get to a
point where the driver consider the memory as fragemented and it not
capable of allocated new descriptor_set anymore.

To avoid that, do not free descriptor_set, reuse ones that have been
release from the application point of view.

Author: rjodinchr

src/config.def

@@ -71,6 +71,8 @@ OPTION(bool, keep_memory_allocations_mapped, false)
 OPTION(std::string, device_extensions, "")
 OPTION(std::string, device_extensions_masked, "")
 
+OPTION(bool, reuse_descriptor_set, false)
+
 //
 // Logging
 //

src/device.hpp

@@ -703,6 +703,10 @@ struct cvk_device : public _cl_device_id,
         return m_clvk_properties->keep_memory_allocations_mapped();
     }
 
+    bool reuse_descriptor_set() const {
+        return m_clvk_properties->reuse_descriptor_set();
+    }
+
 private:
     std::string version_desc() const {
         std::string ret = "CLVK on Vulkan v";

src/device_properties.cpp

@@ -26,8 +26,9 @@ struct cvk_device_properties_mali : public cvk_device_properties {
     cl_uint get_max_first_cmd_batch_size() const override final { return 10; }
     cl_uint get_max_cmd_group_size() const override final { return 1; }
 
-    cvk_device_properties_mali(const uint32_t deviceID)
-        : m_deviceID(deviceID) {}
+    cvk_device_properties_mali(const uint32_t deviceID,
+                               const VkDriverId driverID)
+        : m_deviceID(deviceID), m_driverID(driverID) {}
 
     bool is_non_uniform_decoration_broken() const override final {
 #define GPU_ID2_ARCH_MAJOR_SHIFT 28
@@ -37,24 +38,31 @@ struct cvk_device_properties_mali : public cvk_device_properties {
         return (m_deviceID & GPU_ID2_ARCH_MAJOR) <= bifrost_arch_major;
     }
 
+    bool reuse_descriptor_set() const override final {
+        return m_driverID == VK_DRIVER_ID_ARM_PROPRIETARY;
+    }
+
 private:
     const uint32_t m_deviceID;
+    const VkDriverId m_driverID;
 };
 
 struct cvk_device_properties_mali_exynos9820
     : public cvk_device_properties_mali {
     cl_ulong get_global_mem_cache_size() const override final { return 262144; }
     cl_ulong get_num_compute_units() const override final { return 12; }
-    cvk_device_properties_mali_exynos9820(const uint32_t deviceID)
-        : cvk_device_properties_mali(deviceID) {}
+    cvk_device_properties_mali_exynos9820(const uint32_t deviceID,
+                                          const VkDriverId driverID)
+        : cvk_device_properties_mali(deviceID, driverID) {}
 };
 
 struct cvk_device_properties_mali_exynos990
     : public cvk_device_properties_mali {
     cl_ulong get_global_mem_cache_size() const override final { return 262144; }
     cl_ulong get_num_compute_units() const override final { return 11; }
-    cvk_device_properties_mali_exynos990(const uint32_t deviceID)
-        : cvk_device_properties_mali(deviceID) {}
+    cvk_device_properties_mali_exynos990(const uint32_t deviceID,
+                                         const VkDriverId driverID)
+        : cvk_device_properties_mali(deviceID, driverID) {}
 };
 
 static bool isMaliDevice(const char* name, const uint32_t vendorID) {
@@ -264,9 +272,9 @@ std::unique_ptr<cvk_device_properties> create_cvk_device_properties(
             cvk_warn("Unable to query 'ro.hardware' system property, some "
                      "device properties will be incorrect.");
         } else if (strcmp(soc, "exynos9820") == 0) {
-            RETURN(cvk_device_properties_mali_exynos9820, deviceID);
+            RETURN(cvk_device_properties_mali_exynos9820, deviceID, driverID);
         } else if (strcmp(soc, "exynos990") == 0) {
-            RETURN(cvk_device_properties_mali_exynos990, deviceID);
+            RETURN(cvk_device_properties_mali_exynos990, deviceID, driverID);
         } else {
             cvk_warn("Unrecognized 'ro.hardware' value '%s', some device "
                      "properties will be incorrect.",
@@ -276,7 +284,7 @@ std::unique_ptr<cvk_device_properties> create_cvk_device_properties(
         cvk_warn("Unrecognized Mali device, some device properties will be "
                  "incorrect.");
 #endif
-        RETURN(cvk_device_properties_mali, deviceID);
+        RETURN(cvk_device_properties_mali, deviceID, driverID);
     } else if (strcmp(name, "Adreno (TM) 615") == 0) {
         RETURN(cvk_device_properties_adreno_615);
     } else if (strcmp(name, "Adreno (TM) 620") == 0) {

src/device_properties.hpp

@@ -75,6 +75,10 @@ struct cvk_device_properties {
         return config.keep_memory_allocations_mapped();
     }
 
+    virtual bool reuse_descriptor_set() const {
+        return config.reuse_descriptor_set();
+    }
+
     virtual ~cvk_device_properties() {}
 };
 

src/kernel.hpp

@@ -194,11 +194,7 @@ struct cvk_kernel_argument_values {
           m_descriptor_sets_refcount(0) {}
 
     ~cvk_kernel_argument_values() {
-        for (auto ds : m_descriptor_sets) {
-            if (ds != VK_NULL_HANDLE) {
-                m_entry_point->free_descriptor_set(ds);
-            }
-        }
+        m_entry_point->free_descriptor_sets(m_descriptor_sets);
     }
 
     static std::shared_ptr<cvk_kernel_argument_values>
@@ -385,12 +381,7 @@ struct cvk_kernel_argument_values {
         std::lock_guard<std::mutex> lock(m_lock);
         if (--m_descriptor_sets_refcount == 0) {
             m_is_enqueued = false;
-            for (auto& ds : m_descriptor_sets) {
-                if (ds != VK_NULL_HANDLE) {
-                    m_entry_point->free_descriptor_set(ds);
-                    ds = VK_NULL_HANDLE;
-                }
-            }
+            m_entry_point->free_descriptor_sets(m_descriptor_sets);
         }
     }
 
@@ -437,7 +428,6 @@ struct cvk_kernel_argument_values {
     std::vector<bool> m_args_set;
 
     std::unique_ptr<cvk_buffer> m_pod_buffer;
-    std::array<VkDescriptorSet, spir_binary::MAX_DESCRIPTOR_SETS>
-        m_descriptor_sets;
+    cvk_descriptor_set_array m_descriptor_sets;
     uint32_t m_descriptor_sets_refcount;
 };

src/program.cpp

@@ -2269,6 +2269,16 @@ bool cvk_entry_point::allocate_descriptor_sets(VkDescriptorSet* ds) {
 
     std::lock_guard<std::mutex> lock(m_descriptor_pool_lock);
 
+    if (m_device->reuse_descriptor_set() && !m_descriptor_sets_array.empty()) {
+        cvk_descriptor_set_array descriptor_sets =
+            m_descriptor_sets_array.back();
+        m_descriptor_sets_array.pop_back();
+        for (unsigned i = 0; i < descriptor_sets.size(); i++) {
+            ds[i] = descriptor_sets[i];
+        }
+        return true;
+    }
+
 #if CLVK_UNIT_TESTING_ENABLED
     if (config.force_descriptor_set_allocation_failure() &&
         m_nb_descriptor_set_allocated + m_descriptor_set_layouts.size() >

src/program.hpp

@@ -428,6 +428,9 @@ using cvk_spec_constant_map = std::map<uint32_t, uint32_t>;
 
 struct cvk_program;
 
+using cvk_descriptor_set_array =
+    std::array<VkDescriptorSet, spir_binary::MAX_DESCRIPTOR_SETS>;
+
 class cvk_entry_point {
 public:
     cvk_entry_point(cvk_device* dev, cvk_program* program,
@@ -440,6 +443,11 @@ class cvk_entry_point {
                      m_name.c_str());
             vkDestroyPipeline(vkdev, pipeline.second, nullptr);
         }
+        for (auto& descriptor_sets : m_descriptor_sets_array) {
+            vkFreeDescriptorSets(m_device->vulkan_device(), m_descriptor_pool,
+                                 descriptor_sets.size(),
+                                 descriptor_sets.data());
+        }
         if (m_descriptor_pool != VK_NULL_HANDLE) {
             vkDestroyDescriptorPool(vkdev, m_descriptor_pool, nullptr);
         }
@@ -458,14 +466,23 @@ class cvk_entry_point {
 
     CHECK_RETURN bool allocate_descriptor_sets(VkDescriptorSet* ds);
 
-    void free_descriptor_set(VkDescriptorSet ds) {
+    void free_descriptor_sets(cvk_descriptor_set_array& descriptor_sets) {
         TRACE_FUNCTION();
+        if (descriptor_sets == cvk_descriptor_set_array{VK_NULL_HANDLE}) {
+            return;
+        }
         std::lock_guard<std::mutex> lock(m_descriptor_pool_lock);
-        vkFreeDescriptorSets(m_device->vulkan_device(), m_descriptor_pool, 1,
-                             &ds);
-        m_nb_descriptor_set_allocated--;
-        TRACE_CNT(descriptor_set_allocated_counter,
-                  m_nb_descriptor_set_allocated);
+        if (m_device->reuse_descriptor_set()) {
+            m_descriptor_sets_array.push_back(descriptor_sets);
+        } else {
+            vkFreeDescriptorSets(m_device->vulkan_device(), m_descriptor_pool,
+                                 descriptor_sets.size(),
+                                 descriptor_sets.data());
+            m_nb_descriptor_set_allocated -= m_descriptor_set_layouts.size();
+            TRACE_CNT(descriptor_set_allocated_counter,
+                      m_nb_descriptor_set_allocated);
+        }
+        descriptor_sets = cvk_descriptor_set_array{VK_NULL_HANDLE};
     }
 
     uint32_t num_set_layouts() const { return m_descriptor_set_layouts.size(); }
@@ -572,6 +589,8 @@ class cvk_entry_point {
     TRACE_CNT_VAR(descriptor_set_allocated_counter);
 
     bool m_first_allocation_failure;
+
+    std::vector<cvk_descriptor_set_array> m_descriptor_sets_array;
 };
 
 struct cvk_program : public _cl_program, api_object<object_magic::program> {

tests/perfetto/api_tests.EnqueueTooManyCommandWithRetry-expectation.txt

@@ -30,7 +30,7 @@
 "execute_cmds"
 "extract_cmds_required_by"
 "flush_no_lock"
-"free_descriptor_set"
+"free_descriptor_sets"
 "name"
 "set_event_status"
 "vkQueueSubmit"

tests/perfetto/simple_test-expectation.txt

@@ -29,7 +29,7 @@
 "execute_cmds"
 "extract_cmds_required_by"
 "flush_no_lock"
-"free_descriptor_set"
+"free_descriptor_sets"
 "name"
 "set_event_status"
 "vkQueueSubmit"