Skip to content

Commit

Permalink
Use std::unique_ptr to hold command objects
Browse files Browse the repository at this point in the history
  • Loading branch information
@EwanC
EwanC committed Jan 22, 2025
1 parent 0d0ae88 commit 49fb7a9
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 82 deletions.
103 changes: 55 additions & 48 deletions source/adapters/cuda/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ commandBufferDestroy(ur_exp_command_buffer_handle_t CommandBuffer) try {
return Err;
}

ur_result_t
commandHandleDestroy(ur_exp_command_buffer_command_handle_t Command) try {
ur_result_t commandHandleDestroy(
std::unique_ptr<ur_exp_command_buffer_command_handle_t_> &Command) try {
// We create the ur_event_t returned to the user for a signal node using
// `makeWithNative` which sets `HasOwnership` to false. Therefore destruction
// of the `ur_event_t` object doesn't free the underlying CuEvent_t object and
Expand All @@ -49,7 +49,6 @@ commandHandleDestroy(ur_exp_command_buffer_command_handle_t Command) try {
UR_CHECK_ERROR(cuEventDestroy(SignalEvent));
}

delete Command;
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -336,13 +335,14 @@ static ur_result_t enqueueCommandBufferFillHelper(

std::vector<CUgraphNode> WaitNodes =
NumEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new T(CommandBuffer, GraphNode, SignalNode, WaitNodes,
std::move(DecomposedNodes));
CommandBuffer->CommandHandles.push_back(NewCommand);

auto NewCommand = std::make_unique<T>(CommandBuffer, GraphNode, SignalNode,
WaitNodes, std::move(DecomposedNodes));
if (RetCommand) {
*RetCommand = NewCommand;
*RetCommand = NewCommand.get();
}

CommandBuffer->CommandHandles.push_back(std::move(NewCommand));

return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -384,7 +384,7 @@ UR_APIEXPORT ur_result_t UR_APICALL
urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
if (hCommandBuffer->decrementReferenceCount() == 0) {
// Ref count has reached zero, release of created commands
for (auto Command : hCommandBuffer->CommandHandles) {
for (auto &Command : hCommandBuffer->CommandHandles) {
commandHandleDestroy(Command);
}

Expand Down Expand Up @@ -535,15 +535,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new kernel_command_handle(
auto NewCommand = std::make_unique<kernel_command_handle>(
hCommandBuffer, hKernel, GraphNode, NodeParams, workDim,
pGlobalWorkOffset, pGlobalWorkSize, pLocalWorkSize,
numKernelAlternatives, phKernelAlternatives, SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
} catch (ur_result_t Err) {
return Err;
}
Expand Down Expand Up @@ -591,13 +592,13 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendUSMMemcpyExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new usm_memcpy_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

auto NewCommand = std::make_unique<usm_memcpy_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -656,13 +657,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendMemBufferCopyExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_copy_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);
auto NewCommand = std::make_unique<buffer_copy_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -718,13 +720,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendMemBufferCopyRectExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_copy_rect_command_handle(
auto NewCommand = std::make_unique<buffer_copy_rect_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -776,13 +779,13 @@ ur_result_t UR_APICALL urCommandBufferAppendMemBufferWriteExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_write_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

auto NewCommand = std::make_unique<buffer_write_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -833,13 +836,13 @@ ur_result_t UR_APICALL urCommandBufferAppendMemBufferReadExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_read_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

auto NewCommand = std::make_unique<buffer_read_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -894,13 +897,14 @@ ur_result_t UR_APICALL urCommandBufferAppendMemBufferWriteRectExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_write_rect_command_handle(
auto NewCommand = std::make_unique<buffer_write_rect_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -955,13 +959,14 @@ ur_result_t UR_APICALL urCommandBufferAppendMemBufferReadRectExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new buffer_read_rect_command_handle(
auto NewCommand = std::make_unique<buffer_read_rect_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -1008,13 +1013,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendUSMPrefetchExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new usm_prefetch_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);
auto NewCommand = std::make_unique<usm_prefetch_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));
return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down Expand Up @@ -1061,14 +1067,15 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendUSMAdviseExp(

std::vector<CUgraphNode> WaitNodes =
numEventsInWaitList ? std::move(DepsList) : std::vector<CUgraphNode>();
auto NewCommand = new usm_advise_command_handle(hCommandBuffer, GraphNode,
SignalNode, WaitNodes);
hCommandBuffer->CommandHandles.push_back(NewCommand);
auto NewCommand = std::make_unique<usm_advise_command_handle>(
hCommandBuffer, GraphNode, SignalNode, WaitNodes);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));

return UR_RESULT_SUCCESS;
} catch (ur_result_t Err) {
return Err;
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/cuda/command_buffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,5 +347,6 @@ struct ur_exp_command_buffer_handle_t_ {
ur_exp_command_buffer_sync_point_t NextSyncPoint;

// Handles to individual commands in the command-buffer
std::vector<ur_exp_command_buffer_command_handle_t> CommandHandles;
std::vector<std::unique_ptr<ur_exp_command_buffer_command_handle_t_>>
CommandHandles;
};
14 changes: 4 additions & 10 deletions source/adapters/hip/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,6 @@ urCommandBufferRetainExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
UR_APIEXPORT ur_result_t UR_APICALL
urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
if (hCommandBuffer->decrementReferenceCount() == 0) {
// Ref count has reached zero, release of created commands
// commands.
for (auto Command : hCommandBuffer->CommandHandles) {
delete Command;
}

delete hCommandBuffer;
}
return UR_RESULT_SUCCESS;
Expand Down Expand Up @@ -374,18 +368,18 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
*pSyncPoint = SyncPoint;
}

auto NewCommand = new ur_exp_command_buffer_command_handle_t_{
auto NewCommand = std::make_unique<ur_exp_command_buffer_command_handle_t_>{
hCommandBuffer, hKernel, GraphNode,
NodeParams, workDim, pGlobalWorkOffset,
pGlobalWorkSize, pLocalWorkSize, numKernelAlternatives,
phKernelAlternatives};

hCommandBuffer->CommandHandles.push_back(NewCommand);

if (phCommand) {
*phCommand = NewCommand;
*phCommand = NewCommand.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(NewCommand));

} catch (ur_result_t Err) {
return Err;
}
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/hip/command_buffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,5 +151,6 @@ struct ur_exp_command_buffer_handle_t_ {
ur_exp_command_buffer_sync_point_t NextSyncPoint;

// Handles to individual commands in the command-buffer
std::vector<ur_exp_command_buffer_command_handle_t> CommandHandles;
std::vector<std::unique_ptr<ur_exp_command_buffer_command_handle_t_>>
CommandHandles;
};
16 changes: 7 additions & 9 deletions source/adapters/level_zero/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,6 @@ void ur_exp_command_buffer_handle_t_::cleanupCommandBufferResources() {
ReleaseIndirectMem(AssociatedKernel);
ur::level_zero::urKernelRelease(AssociatedKernel);
}

for (auto Command : CommandHandles) {
delete Command;
}
}

void ur_exp_command_buffer_handle_t_::registerSyncPoint(
Expand Down Expand Up @@ -924,7 +920,7 @@ createCommandHandle(ur_exp_command_buffer_handle_t CommandBuffer,
ur_kernel_handle_t Kernel, uint32_t WorkDim,
const size_t *LocalWorkSize, uint32_t NumKernelAlternatives,
ur_kernel_handle_t *KernelAlternatives,
ur_exp_command_buffer_command_handle_t &Command) {
ur_exp_command_buffer_command_handle_t *Command) {

assert(CommandBuffer->IsUpdatable);

Expand Down Expand Up @@ -992,17 +988,19 @@ createCommandHandle(ur_exp_command_buffer_handle_t CommandBuffer,
DEBUG_LOG(CommandId);

try {
Command = new kernel_command_handle(
auto NewCommand = std::make_unique<kernel_command_handle>(
CommandBuffer, Kernel, CommandId, WorkDim, LocalWorkSize != nullptr,
NumKernelAlternatives, KernelAlternatives);

*Command = NewCommand.get();

CommandBuffer->CommandHandles.push_back(std::move(NewCommand));
} catch (const std::bad_alloc &) {
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
} catch (...) {
return UR_RESULT_ERROR_UNKNOWN;
}

CommandBuffer->CommandHandles.push_back(Command);

return UR_RESULT_SUCCESS;
}

Expand Down Expand Up @@ -1070,7 +1068,7 @@ ur_result_t urCommandBufferAppendKernelLaunchExp(
if (Command) {
UR_CALL(createCommandHandle(CommandBuffer, Kernel, WorkDim, LocalWorkSize,
NumKernelAlternatives, KernelAlternatives,
*Command));
Command));
}
std::vector<ze_event_handle_t> ZeEventList;
ze_event_handle_t ZeLaunchEvent = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/level_zero/command_buffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ struct ur_exp_command_buffer_handle_t_ : public _ur_object {
// any fence or event synchronization to avoid repeated calls to synchronize.
bool NeedsUpdateSynchronization = false;
// Track handle objects to free when command-buffer is destroyed.
std::vector<ur_exp_command_buffer_command_handle_t> CommandHandles;
std::vector<std::unique_ptr<ur_exp_command_buffer_command_handle_t_>>
CommandHandles;
};

struct ur_exp_command_buffer_command_handle_t_ : public _ur_object {
Expand Down
17 changes: 6 additions & 11 deletions source/adapters/opencl/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ urCommandBufferRetainExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
UR_APIEXPORT ur_result_t UR_APICALL
urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
if (hCommandBuffer->decrementReferenceCount() == 0) {
for (auto Command : hCommandBuffer->CommandHandles) {
delete Command;
}

delete hCommandBuffer;
}

Expand Down Expand Up @@ -159,15 +155,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
pSyncPointWaitList, pSyncPoint, OutCommandHandle));

try {
auto URCommandHandle =
std::make_unique<ur_exp_command_buffer_command_handle_t_>(
hCommandBuffer, CommandHandle, hKernel, workDim,
pLocalWorkSize != nullptr);
ur_exp_command_buffer_command_handle_t Handle = URCommandHandle.release();
hCommandBuffer->CommandHandles.push_back(Handle);
auto Handle = std::make_unique<ur_exp_command_buffer_command_handle_t_>(
hCommandBuffer, CommandHandle, hKernel, workDim,
pLocalWorkSize != nullptr);
if (phCommandHandle) {
*phCommandHandle = Handle;
*phCommandHandle = Handle.get();
}

hCommandBuffer->CommandHandles.push_back(std::move(Handle));
} catch (...) {
return UR_RESULT_ERROR_OUT_OF_RESOURCES;
}
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/opencl/command_buffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ struct ur_exp_command_buffer_handle_t_ {
/// Set to true if the command-buffer has been finalized, false otherwise
bool IsFinalized;
/// List of commands in the command-buffer.
std::vector<ur_exp_command_buffer_command_handle_t> CommandHandles;
std::vector<std::unique_ptr<ur_exp_command_buffer_command_handle_t_>>
CommandHandles;
/// Object reference count
std::atomic_uint32_t RefCount;

Expand Down

0 comments on commit 49fb7a9

Please sign in to comment.