Skip to content

Commit ce3ba4d

Browse files
igchorigchor
committed
[L0 v2] remove addCommandList methods for cmd list cache
Instead, rely on raii to return the list back to the cache
1 parent 31b2940 commit ce3ba4d

File tree

3 files changed

+41
-64
lines changed

3 files changed

+41
-64
lines changed

‎source/adapters/level_zero/v2/command_list_cache.cpp‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ command_list_cache_t::createCommandList(const command_list_descriptor_t &desc) {
7676
}
7777
}
7878

79-
raii::ze_command_list_t command_list_cache_t::getImmediateCommandList(
79+
raii::cache_borrowed_command_list_t
80+
command_list_cache_t::getImmediateCommandList(
8081
ze_device_handle_t ZeDevice, bool IsInOrder, uint32_t Ordinal,
8182
ze_command_queue_mode_t Mode, ze_command_queue_priority_t Priority,
8283
std::optional<uint32_t> Index) {
@@ -87,41 +88,29 @@ raii::ze_command_list_t command_list_cache_t::getImmediateCommandList(
8788
Desc.Mode = Mode;
8889
Desc.Priority = Priority;
8990
Desc.Index = Index;
90-
return getCommandList(Desc);
91+
92+
auto CommandList = getCommandList(Desc).release();
93+
return raii::cache_borrowed_command_list_t(
94+
CommandList, [Cache = this, Desc](ze_command_list_handle_t CmdList) {
95+
Cache->addCommandList(
96+
Desc, raii::ze_command_list_t(CmdList, &zeCommandListDestroy));
97+
});
9198
}
9299

93-
raii::ze_command_list_t
100+
raii::cache_borrowed_command_list_t
94101
command_list_cache_t::getRegularCommandList(ze_device_handle_t ZeDevice,
95102
bool IsInOrder, uint32_t Ordinal) {
96103
regular_command_list_descriptor_t Desc;
97104
Desc.ZeDevice = ZeDevice;
98105
Desc.IsInOrder = IsInOrder;
99106
Desc.Ordinal = Ordinal;
100-
return getCommandList(Desc);
101-
}
102-
103-
void command_list_cache_t::addImmediateCommandList(
104-
raii::ze_command_list_t cmdList, ze_device_handle_t ZeDevice,
105-
bool IsInOrder, uint32_t Ordinal, ze_command_queue_mode_t Mode,
106-
ze_command_queue_priority_t Priority, std::optional<uint32_t> Index) {
107-
immediate_command_list_descriptor_t Desc;
108-
Desc.ZeDevice = ZeDevice;
109-
Desc.Ordinal = Ordinal;
110-
Desc.IsInOrder = IsInOrder;
111-
Desc.Mode = Mode;
112-
Desc.Priority = Priority;
113-
Desc.Index = Index;
114-
addCommandList(Desc, std::move(cmdList));
115-
}
116107

117-
void command_list_cache_t::addRegularCommandList(
118-
raii::ze_command_list_t cmdList, ze_device_handle_t ZeDevice,
119-
bool IsInOrder, uint32_t Ordinal) {
120-
regular_command_list_descriptor_t Desc;
121-
Desc.ZeDevice = ZeDevice;
122-
Desc.IsInOrder = IsInOrder;
123-
Desc.Ordinal = Ordinal;
124-
addCommandList(Desc, std::move(cmdList));
108+
auto CommandList = getCommandList(Desc).release();
109+
return raii::cache_borrowed_command_list_t(
110+
CommandList, [Cache = this, Desc](ze_command_list_handle_t CmdList) {
111+
Cache->addCommandList(
112+
Desc, raii::ze_command_list_t(CmdList, &zeCommandListDestroy));
113+
});
125114
}
126115

127116
raii::ze_command_list_t

‎source/adapters/level_zero/v2/command_list_cache.hpp‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//===----------------------------------------------------------------------===//
1010
#pragma once
1111

12+
#include <functional>
1213
#include <stack>
1314

1415
#include <ur/ur.hpp>
@@ -21,7 +22,10 @@ namespace v2 {
2122
namespace raii {
2223
using ze_command_list_t = std::unique_ptr<::_ze_command_list_handle_t,
2324
decltype(&zeCommandListDestroy)>;
24-
}
25+
using cache_borrowed_command_list_t =
26+
std::unique_ptr<::_ze_command_list_handle_t,
27+
std::function<void(ze_command_list_handle_t)>>;
28+
} // namespace raii
2529

2630
struct immediate_command_list_descriptor_t {
2731
ze_device_handle_t ZeDevice;
@@ -51,23 +55,14 @@ struct command_list_descriptor_hash_t {
5155
struct command_list_cache_t {
5256
command_list_cache_t(ze_context_handle_t ZeContext);
5357

54-
raii::ze_command_list_t
58+
raii::cache_borrowed_command_list_t
5559
getImmediateCommandList(ze_device_handle_t ZeDevice, bool IsInOrder,
5660
uint32_t Ordinal, ze_command_queue_mode_t Mode,
5761
ze_command_queue_priority_t Priority,
5862
std::optional<uint32_t> Index = std::nullopt);
59-
raii::ze_command_list_t getRegularCommandList(ze_device_handle_t ZeDevice,
60-
bool IsInOrder,
61-
uint32_t Ordinal);
62-
63-
void addImmediateCommandList(raii::ze_command_list_t cmdList,
64-
ze_device_handle_t ZeDevice, bool IsInOrder,
65-
uint32_t Ordinal, ze_command_queue_mode_t Mode,
66-
ze_command_queue_priority_t Priority,
67-
std::optional<uint32_t> Index = std::nullopt);
68-
void addRegularCommandList(raii::ze_command_list_t cmdList,
69-
ze_device_handle_t ZeDevice, bool IsInOrder,
70-
uint32_t Ordinal);
63+
raii::cache_borrowed_command_list_t
64+
getRegularCommandList(ze_device_handle_t ZeDevice, bool IsInOrder,
65+
uint32_t Ordinal);
7166

7267
private:
7368
ze_context_handle_t ZeContext;

‎test/adapters/level_zero/v2/command_list_cache_test.cpp‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <map>
1616
#include <string>
1717
#include <unordered_set>
18+
#include <vector>
1819

1920
struct CommandListCacheTest : public uur::urContextTest {};
2021

@@ -30,35 +31,28 @@ TEST_P(CommandListCacheTest, CanStoreAndRetriveImmediateAndRegularCmdLists) {
3031
ze_command_queue_priority_t Priority = ZE_COMMAND_QUEUE_PRIORITY_NORMAL;
3132

3233
static constexpr int numListsPerType = 3;
34+
std::vector<v2::raii::cache_borrowed_command_list_t> regCmdListOwners;
35+
std::vector<v2::raii::cache_borrowed_command_list_t> immCmdListOwners;
36+
3337
std::unordered_set<ze_command_list_handle_t> regCmdLists;
3438
std::unordered_set<ze_command_list_handle_t> immCmdLists;
3539

3640
// get command lists from the cache
3741
for (int i = 0; i < numListsPerType; ++i) {
38-
auto [it, _] = regCmdLists.emplace(
39-
cache.getRegularCommandList(device->ZeDevice, IsInOrder, Ordinal)
40-
.release());
42+
regCmdListOwners.emplace_back(
43+
cache.getRegularCommandList(device->ZeDevice, IsInOrder, Ordinal));
44+
auto [it, _] = regCmdLists.emplace(regCmdListOwners.back().get());
4145
ASSERT_TRUE(*it != nullptr);
4246

43-
std::tie(it, _) = immCmdLists.emplace(
44-
cache
45-
.getImmediateCommandList(device->ZeDevice, IsInOrder, Ordinal,
46-
Mode, Priority)
47-
.release());
47+
immCmdListOwners.emplace_back(cache.getImmediateCommandList(
48+
device->ZeDevice, IsInOrder, Ordinal, Mode, Priority));
49+
std::tie(it, _) = immCmdLists.emplace(immCmdListOwners.back().get());
4850
ASSERT_TRUE(*it != nullptr);
4951
}
5052

5153
// store them back into the cache
52-
for (auto cmdList : regCmdLists) {
53-
cache.addRegularCommandList(
54-
v2::raii::ze_command_list_t(cmdList, &zeCommandListDestroy),
55-
device->ZeDevice, IsInOrder, Ordinal);
56-
}
57-
for (auto cmdList : immCmdLists) {
58-
cache.addImmediateCommandList(
59-
v2::raii::ze_command_list_t(cmdList, &zeCommandListDestroy),
60-
device->ZeDevice, IsInOrder, Ordinal, Mode, Priority);
61-
}
54+
regCmdListOwners.clear();
55+
immCmdListOwners.clear();
6256

6357
// verify we get back the same command lists
6458
for (int i = 0; i < numListsPerType; ++i) {
@@ -72,6 +66,10 @@ TEST_P(CommandListCacheTest, CanStoreAndRetriveImmediateAndRegularCmdLists) {
7266

7367
ASSERT_EQ(regCmdLists.erase(regCmdList.get()), 1);
7468
ASSERT_EQ(immCmdLists.erase(immCmdList.get()), 1);
69+
70+
// release the command list manually so they are not added back to the cache
71+
zeCommandListDestroy(regCmdList.release());
72+
zeCommandListDestroy(immCmdList.release());
7573
}
7674
}
7775

@@ -131,11 +129,6 @@ TEST_P(CommandListCacheTest, ImmediateCommandListsHaveProperAttributes) {
131129
} else {
132130
ASSERT_EQ(Ret, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE);
133131
}
134-
135-
// store the list back to the cache
136-
cache.addImmediateCommandList(std::move(CommandList),
137-
device->ZeDevice, IsInOrder, Ordinal,
138-
Mode, Priority, Index);
139132
}
140133

141134
// verify list creation without an index

0 commit comments

Comments
 (0)