[L0 v2] remove addCommandList methods for cmd list cache

Instead, rely on raii to return the list back to the cache

Author: igchor

source/adapters/level_zero/v2/command_list_cache.cpp

@@ -76,7 +76,8 @@ command_list_cache_t::createCommandList(const command_list_descriptor_t &desc) {
   }
 }
 
-raii::ze_command_list_t command_list_cache_t::getImmediateCommandList(
+raii::cache_borrowed_command_list_t
+command_list_cache_t::getImmediateCommandList(
     ze_device_handle_t ZeDevice, bool IsInOrder, uint32_t Ordinal,
     ze_command_queue_mode_t Mode, ze_command_queue_priority_t Priority,
     std::optional<uint32_t> Index) {
@@ -87,41 +88,29 @@ raii::ze_command_list_t command_list_cache_t::getImmediateCommandList(
   Desc.Mode = Mode;
   Desc.Priority = Priority;
   Desc.Index = Index;
-  return getCommandList(Desc);
+
+  auto CommandList = getCommandList(Desc).release();
+  return raii::cache_borrowed_command_list_t(
+      CommandList, [Cache = this, Desc](ze_command_list_handle_t CmdList) {
+        Cache->addCommandList(
+            Desc, raii::ze_command_list_t(CmdList, &zeCommandListDestroy));
+      });
 }
 
-raii::ze_command_list_t
+raii::cache_borrowed_command_list_t
 command_list_cache_t::getRegularCommandList(ze_device_handle_t ZeDevice,
                                             bool IsInOrder, uint32_t Ordinal) {
   regular_command_list_descriptor_t Desc;
   Desc.ZeDevice = ZeDevice;
   Desc.IsInOrder = IsInOrder;
   Desc.Ordinal = Ordinal;
-  return getCommandList(Desc);
-}
-
-void command_list_cache_t::addImmediateCommandList(
-    raii::ze_command_list_t cmdList, ze_device_handle_t ZeDevice,
-    bool IsInOrder, uint32_t Ordinal, ze_command_queue_mode_t Mode,
-    ze_command_queue_priority_t Priority, std::optional<uint32_t> Index) {
-  immediate_command_list_descriptor_t Desc;
-  Desc.ZeDevice = ZeDevice;
-  Desc.Ordinal = Ordinal;
-  Desc.IsInOrder = IsInOrder;
-  Desc.Mode = Mode;
-  Desc.Priority = Priority;
-  Desc.Index = Index;
-  addCommandList(Desc, std::move(cmdList));
-}
 
-void command_list_cache_t::addRegularCommandList(
-    raii::ze_command_list_t cmdList, ze_device_handle_t ZeDevice,
-    bool IsInOrder, uint32_t Ordinal) {
-  regular_command_list_descriptor_t Desc;
-  Desc.ZeDevice = ZeDevice;
-  Desc.IsInOrder = IsInOrder;
-  Desc.Ordinal = Ordinal;
-  addCommandList(Desc, std::move(cmdList));
+  auto CommandList = getCommandList(Desc).release();
+  return raii::cache_borrowed_command_list_t(
+      CommandList, [Cache = this, Desc](ze_command_list_handle_t CmdList) {
+        Cache->addCommandList(
+            Desc, raii::ze_command_list_t(CmdList, &zeCommandListDestroy));
+      });
 }
 
 raii::ze_command_list_t

source/adapters/level_zero/v2/command_list_cache.hpp

@@ -9,6 +9,7 @@
 //===----------------------------------------------------------------------===//
 #pragma once
 
+#include <functional>
 #include <stack>
 
 #include <ur/ur.hpp>
@@ -21,7 +22,10 @@ namespace v2 {
 namespace raii {
 using ze_command_list_t = std::unique_ptr<::_ze_command_list_handle_t,
                                           decltype(&zeCommandListDestroy)>;
-}
+using cache_borrowed_command_list_t =
+    std::unique_ptr<::_ze_command_list_handle_t,
+                    std::function<void(ze_command_list_handle_t)>>;
+} // namespace raii
 
 struct immediate_command_list_descriptor_t {
   ze_device_handle_t ZeDevice;
@@ -51,23 +55,14 @@ struct command_list_descriptor_hash_t {
 struct command_list_cache_t {
   command_list_cache_t(ze_context_handle_t ZeContext);
 
-  raii::ze_command_list_t
+  raii::cache_borrowed_command_list_t
   getImmediateCommandList(ze_device_handle_t ZeDevice, bool IsInOrder,
                           uint32_t Ordinal, ze_command_queue_mode_t Mode,
                           ze_command_queue_priority_t Priority,
                           std::optional<uint32_t> Index = std::nullopt);
-  raii::ze_command_list_t getRegularCommandList(ze_device_handle_t ZeDevice,
-                                                bool IsInOrder,
-                                                uint32_t Ordinal);
-
-  void addImmediateCommandList(raii::ze_command_list_t cmdList,
-                               ze_device_handle_t ZeDevice, bool IsInOrder,
-                               uint32_t Ordinal, ze_command_queue_mode_t Mode,
-                               ze_command_queue_priority_t Priority,
-                               std::optional<uint32_t> Index = std::nullopt);
-  void addRegularCommandList(raii::ze_command_list_t cmdList,
-                             ze_device_handle_t ZeDevice, bool IsInOrder,
-                             uint32_t Ordinal);
+  raii::cache_borrowed_command_list_t
+  getRegularCommandList(ze_device_handle_t ZeDevice, bool IsInOrder,
+                        uint32_t Ordinal);
 
 private:
   ze_context_handle_t ZeContext;

test/adapters/level_zero/v2/command_list_cache_test.cpp

@@ -15,6 +15,7 @@
 #include <map>
 #include <string>
 #include <unordered_set>
+#include <vector>
 
 struct CommandListCacheTest : public uur::urContextTest {};
 
@@ -30,35 +31,28 @@ TEST_P(CommandListCacheTest, CanStoreAndRetriveImmediateAndRegularCmdLists) {
     ze_command_queue_priority_t Priority = ZE_COMMAND_QUEUE_PRIORITY_NORMAL;
 
     static constexpr int numListsPerType = 3;
+    std::vector<v2::raii::cache_borrowed_command_list_t> regCmdListOwners;
+    std::vector<v2::raii::cache_borrowed_command_list_t> immCmdListOwners;
+
     std::unordered_set<ze_command_list_handle_t> regCmdLists;
     std::unordered_set<ze_command_list_handle_t> immCmdLists;
 
     // get command lists from the cache
     for (int i = 0; i < numListsPerType; ++i) {
-        auto [it, _] = regCmdLists.emplace(
-            cache.getRegularCommandList(device->ZeDevice, IsInOrder, Ordinal)
-                .release());
+        regCmdListOwners.emplace_back(
+            cache.getRegularCommandList(device->ZeDevice, IsInOrder, Ordinal));
+        auto [it, _] = regCmdLists.emplace(regCmdListOwners.back().get());
         ASSERT_TRUE(*it != nullptr);
 
-        std::tie(it, _) = immCmdLists.emplace(
-            cache
-                .getImmediateCommandList(device->ZeDevice, IsInOrder, Ordinal,
-                                         Mode, Priority)
-                .release());
+        immCmdListOwners.emplace_back(cache.getImmediateCommandList(
+            device->ZeDevice, IsInOrder, Ordinal, Mode, Priority));
+        std::tie(it, _) = immCmdLists.emplace(immCmdListOwners.back().get());
         ASSERT_TRUE(*it != nullptr);
     }
 
     // store them back into the cache
-    for (auto cmdList : regCmdLists) {
-        cache.addRegularCommandList(
-            v2::raii::ze_command_list_t(cmdList, &zeCommandListDestroy),
-            device->ZeDevice, IsInOrder, Ordinal);
-    }
-    for (auto cmdList : immCmdLists) {
-        cache.addImmediateCommandList(
-            v2::raii::ze_command_list_t(cmdList, &zeCommandListDestroy),
-            device->ZeDevice, IsInOrder, Ordinal, Mode, Priority);
-    }
+    regCmdListOwners.clear();
+    immCmdListOwners.clear();
 
     // verify we get back the same command lists
     for (int i = 0; i < numListsPerType; ++i) {
@@ -72,6 +66,10 @@ TEST_P(CommandListCacheTest, CanStoreAndRetriveImmediateAndRegularCmdLists) {
 
         ASSERT_EQ(regCmdLists.erase(regCmdList.get()), 1);
         ASSERT_EQ(immCmdLists.erase(immCmdList.get()), 1);
+
+        // release the command list manually so they are not added back to the cache
+        zeCommandListDestroy(regCmdList.release());
+        zeCommandListDestroy(immCmdList.release());
     }
 }
 
@@ -131,11 +129,6 @@ TEST_P(CommandListCacheTest, ImmediateCommandListsHaveProperAttributes) {
             } else {
                 ASSERT_EQ(Ret, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE);
             }
-
-            // store the list back to the cache
-            cache.addImmediateCommandList(std::move(CommandList),
-                                          device->ZeDevice, IsInOrder, Ordinal,
-                                          Mode, Priority, Index);
         }
 
         // verify list creation without an index