Skip to content

Commit

Permalink
Move duplication detection to GraphBuilder::updateLeaves().
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alexandr-Konovalov
Alexandr-Konovalov committed Oct 25, 2024
1 parent b097735 commit f74ad1a
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 47 deletions.
84 changes: 50 additions & 34 deletions sycl/source/detail/scheduler/graph_builder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,19 +243,29 @@ Scheduler::GraphBuilder::getOrInsertMemObjRecord(const QueueImplPtr &Queue,
void Scheduler::GraphBuilder::updateLeaves(const std::set<Command *> &Cmds,
MemObjRecord *Record,
access::mode AccessMode,
const MapOfDependentCmds &DependentCmdsOfNewCmd,
const QueueImplPtr &Queue,
std::vector<Command *> &ToCleanUp) {

const bool ReadOnlyReq = AccessMode == access::mode::read;
if (ReadOnlyReq)
return;

for (Command *Cmd : Cmds) {
bool WasLeaf = Cmd->MLeafCounter > 0;
Cmd->MLeafCounter -= Record->MReadLeaves.remove(Cmd);
Cmd->MLeafCounter -= Record->MWriteLeaves.remove(Cmd);
if (WasLeaf && Cmd->readyForCleanup()) {
ToCleanUp.push_back(Cmd);
if (! ReadOnlyReq) {
bool WasLeaf = Cmd->MLeafCounter > 0;
Cmd->MLeafCounter -= Record->MReadLeaves.remove(Cmd);
Cmd->MLeafCounter -= Record->MWriteLeaves.remove(Cmd);
if (WasLeaf && Cmd->readyForCleanup()) {
ToCleanUp.push_back(Cmd);
}
}

detectDuplicates(Cmd, DependentCmdsOfNewCmd, ToCleanUp);

// For in-order queue, we may cleanup all dependent command from our queue
if (Queue && Queue->isInOrder() && Cmd->getQueue() == Queue
&& Cmd->getType() == Command::RUN_CG
&& Cmd->MLeafCounter
&& Cmd->MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess)
commandToCleanup(Cmd, ToCleanUp);
}
}

Expand All @@ -270,18 +280,20 @@ void Scheduler::GraphBuilder::addNodeToLeaves(
}

void Scheduler::GraphBuilder::detectDuplicates(
Command *DepCommand, const std::pmr::unordered_set<Command *> &DependentCmdsOfNewCmd,
Command *DepCommand, const MapOfDependentCmds &DependentCmdsOfNewCmd,
std::vector<Command *> &ToCleanUp) {
if (!DepCommand->MLeafCounter || // already no leaves, can't be duplicate
DepCommand->MEnqueueStatus != EnqueueResultT::SyclEnqueueSuccess)
return;
// any dependence of DepCommand already covered by NewCmd
bool Duplicate = std::all_of(DepCommand->MDeps.begin(), DepCommand->MDeps.end(),
[&DependentCmdsOfNewCmd](const DepDesc &DepOfDep) {
return DependentCmdsOfNewCmd.count(DepOfDep.MDepCommand);
return DependentCmdsOfNewCmd.isMemObjExist(
std::make_pair(DepOfDep.MDepRequirement->MSYCLMemObj, DepOfDep.MDepRequirement->MAccessMode));
});
if (Duplicate)
if (Duplicate) {
commandToCleanup(DepCommand, ToCleanUp);
}
}

void Scheduler::GraphBuilder::commandToCleanup(
Expand Down Expand Up @@ -315,7 +327,9 @@ UpdateHostRequirementCommand *Scheduler::GraphBuilder::insertUpdateHostReqCmd(
if (ConnCmd)
ToEnqueue.push_back(ConnCmd);
}
updateLeaves(Deps, Record, Req->MAccessMode, ToCleanUp);
const MapOfDependentCmds DependentCmdsOfNewCmd(UpdateCommand->MDeps);
updateLeaves(Deps, Record, Req->MAccessMode,
DependentCmdsOfNewCmd, Queue, ToCleanUp);
addNodeToLeaves(Record, UpdateCommand, Req->MAccessMode, ToEnqueue);
for (Command *Cmd : ToCleanUp)
cleanupCommand(Cmd);
Expand Down Expand Up @@ -430,7 +444,8 @@ Command *Scheduler::GraphBuilder::insertMemoryMove(
if (ConnCmd)
ToEnqueue.push_back(ConnCmd);
}
updateLeaves(Deps, Record, access::mode::read_write, ToCleanUp);
const MapOfDependentCmds DependentCmdsOfNewCmd(NewCmd->MDeps);
updateLeaves(Deps, Record, access::mode::read_write, DependentCmdsOfNewCmd, Queue, ToCleanUp);
addNodeToLeaves(Record, NewCmd, access::mode::read_write, ToEnqueue);
for (Command *Cmd : ToCleanUp)
cleanupCommand(Cmd);
Expand Down Expand Up @@ -474,7 +489,9 @@ Command *Scheduler::GraphBuilder::remapMemoryObject(
if (ConnCmd)
ToEnqueue.push_back(ConnCmd);

updateLeaves(Deps, Record, access::mode::read_write, ToCleanUp);
const MapOfDependentCmds DependentCmdsOfNewCmd(UnMapCmd->MDeps);
updateLeaves(Deps, Record, access::mode::read_write,
DependentCmdsOfNewCmd, LinkedAllocaCmd->getQueue(), ToCleanUp);
addNodeToLeaves(Record, MapCmd, access::mode::read_write, ToEnqueue);
for (Command *Cmd : ToCleanUp)
cleanupCommand(Cmd);
Expand Down Expand Up @@ -518,7 +535,9 @@ Scheduler::GraphBuilder::addCopyBack(Requirement *Req,
ToEnqueue.push_back(ConnCmd);
}

updateLeaves(Deps, Record, Req->MAccessMode, ToCleanUp);
const MapOfDependentCmds DependentCmdsOfNewCmd(MemCpyCmd->MDeps);
updateLeaves(Deps, Record, Req->MAccessMode,
DependentCmdsOfNewCmd, SrcAllocaCmd->getQueue(), ToCleanUp);
addNodeToLeaves(Record, MemCpyCmd, Req->MAccessMode, ToEnqueue);
for (Command *Cmd : ToCleanUp)
cleanupCommand(Cmd);
Expand Down Expand Up @@ -843,7 +862,9 @@ AllocaCommandBase *Scheduler::GraphBuilder::getOrCreateAllocaForReq(
if (ConnCmd)
ToEnqueue.push_back(ConnCmd);
}
updateLeaves(Deps, Record, Req->MAccessMode, ToCleanUp);
const MapOfDependentCmds DependentCmdsOfNewCmd(AllocaCmd->MDeps);
updateLeaves(Deps, Record, Req->MAccessMode,
DependentCmdsOfNewCmd, Queue, ToCleanUp);
addNodeToLeaves(Record, AllocaCmd, Req->MAccessMode, ToEnqueue);
}
}
Expand Down Expand Up @@ -900,11 +921,12 @@ EmptyCommand *Scheduler::GraphBuilder::addEmptyCmd(

const std::vector<DepDesc> &Deps = Cmd->MDeps;
std::vector<Command *> ToCleanUp;
const MapOfDependentCmds DependentCmdsOfNewCmd(EmptyCmd->MDeps);
for (const DepDesc &Dep : Deps) {
const Requirement *Req = Dep.MDepRequirement;
MemObjRecord *Record = getMemObjRecord(Req->MSYCLMemObj);

updateLeaves({Cmd}, Record, Req->MAccessMode, ToCleanUp);
updateLeaves({Cmd}, Record, Req->MAccessMode, DependentCmdsOfNewCmd, nullptr, ToCleanUp);
addNodeToLeaves(Record, EmptyCmd, Req->MAccessMode, ToEnqueue);
}
for (Command *Cmd : ToCleanUp)
Expand Down Expand Up @@ -1042,28 +1064,15 @@ Command *Scheduler::GraphBuilder::addCG(
// iterate over their copy.
// FIXME employ a reference here to eliminate copying of a vector

std::array<std::byte, 16*1024> DependentCmdsOfNewCmdBuf;
std::pmr::monotonic_buffer_resource DependentCmdsOfNewCmdBufRes{DependentCmdsOfNewCmdBuf.data(),
DependentCmdsOfNewCmdBuf.size()};
std::pmr::unordered_set<Command *> DependentCmdsOfNewCmd{&DependentCmdsOfNewCmdBufRes};
for (const DepDesc &Dep : NewCmd->MDeps)
DependentCmdsOfNewCmd.insert(Dep.MAllocaCmd);
const MapOfDependentCmds DependentCmdsOfNewCmd(NewCmd->MDeps);

std::vector<DepDesc> Deps = NewCmd->MDeps;
for (DepDesc &Dep : Deps) {
const Requirement *Req = Dep.MDepRequirement;
MemObjRecord *Record = getMemObjRecord(Req->MSYCLMemObj);
updateLeaves({Dep.MDepCommand}, Record, Req->MAccessMode, ToCleanUp);
updateLeaves({Dep.MDepCommand}, Record, Req->MAccessMode,
DependentCmdsOfNewCmd, Queue, ToCleanUp);
addNodeToLeaves(Record, NewCmd.get(), Req->MAccessMode, ToEnqueue);
// All dependences of a dependent cmd already covered by NewCmd,
// can move the dependent cmd in ToCleanUp
detectDuplicates(Dep.MDepCommand, DependentCmdsOfNewCmd, ToCleanUp);
// For in-order queue, we may cleanup all dependent command from our queue
if (Queue && Queue->isInOrder() && Dep.MDepCommand->getQueue() == Queue
&& Dep.MDepCommand->getType() == Command::RUN_CG
&& Dep.MDepCommand->MLeafCounter
&& Dep.MDepCommand->MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess)
commandToCleanup(Dep.MDepCommand, ToCleanUp);
}

for (detail::EventImplPtr e : Events) {
Expand Down Expand Up @@ -1372,6 +1381,7 @@ Command *Scheduler::GraphBuilder::addCommandGraphUpdate(
}
}

const MapOfDependentCmds DependentCmdsOfNewCmd(NewCmd->MDeps);
// Set new command as user for dependencies and update leaves.
// Node dependencies can be modified further when adding the node to leaves,
// iterate over their copy.
Expand All @@ -1380,7 +1390,8 @@ Command *Scheduler::GraphBuilder::addCommandGraphUpdate(
for (DepDesc &Dep : Deps) {
const Requirement *Req = Dep.MDepRequirement;
MemObjRecord *Record = getMemObjRecord(Req->MSYCLMemObj);
updateLeaves({Dep.MDepCommand}, Record, Req->MAccessMode, ToCleanUp);
updateLeaves({Dep.MDepCommand}, Record, Req->MAccessMode,
DependentCmdsOfNewCmd, Queue, ToCleanUp);
addNodeToLeaves(Record, NewCmd.get(), Req->MAccessMode, ToEnqueue);
}

Expand All @@ -1392,6 +1403,11 @@ Command *Scheduler::GraphBuilder::addCommandGraphUpdate(
}
if (Command *ConnCmd = NewCmd->addDep(e, ToCleanUp))
ToEnqueue.push_back(ConnCmd);

// If NewCmd depends on another command, and all dependences of that command
// already covered by NewCmd, can move the cmd in ToCleanUp
if (auto *Cmd = static_cast<Command *>(e->getCommand()))
detectDuplicates(Cmd, DependentCmdsOfNewCmd, ToCleanUp);
}

if (MPrintOptionsArray[AfterAddCG])
Expand Down
52 changes: 46 additions & 6 deletions sycl/source/detail/scheduler/scheduler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ class queue_impl;
class event_impl;
class context_impl;
class DispatchHostTask;
class MapOfDependentCmds;

using ContextImplPtr = std::shared_ptr<detail::context_impl>;
using EventImplPtr = std::shared_ptr<detail::event_impl>;
Expand Down Expand Up @@ -622,14 +623,10 @@ class Scheduler {

/// Removes commands from leaves.
static void updateLeaves(const std::set<Command *> &Cmds, MemObjRecord *Record,
access::mode AccessMode,
access::mode AccessMode, const MapOfDependentCmds &DependentCmdsOfNewCmd,
const QueueImplPtr &Queue,
std::vector<Command *> &ToCleanUp);

/// If dependent cmd do same as NewCmd, move it to cleanup
static void detectDuplicates(Command *DepCommand,
const std::pmr::unordered_set<Command *> &DependentCmdsOfNewCmd,
std::vector<Command *> &ToCleanUp);

/// Prepare a command to cleanup
static void commandToCleanup(Command *DepCommand, std::vector<Command *> &ToCleanUp);

Expand Down Expand Up @@ -700,6 +697,12 @@ class Scheduler {
Command::BlockReason Reason,
std::vector<Command *> &ToEnqueue);

/// If all dependences of a dependent cmd already covered by NewCmd,
/// move the dependent cmd in ToCleanUp
static void detectDuplicates(Command *DepCommand,
const MapOfDependentCmds &DependentCmdsOfNewCmd,
std::vector<Command *> &ToCleanUp);

protected:
/// Finds a command dependency corresponding to the record.
DepDesc findDepForRecord(Command *Cmd, MemObjRecord *Record);
Expand Down Expand Up @@ -900,6 +903,43 @@ class Scheduler {
friend class ::MockScheduler;
};

class MapOfDependentCmds {
using CommandModePair = std::pair<SYCLMemObjI *, access::mode>;

struct CommandModePairHash {
std::size_t operator()(const CommandModePair& p) const noexcept {
return std::hash<SYCLMemObjI *>{}(p.first) ^ std::hash<access::mode>{}(p.second);
}
};

using CommandModePairSet = std::pmr::unordered_set<CommandModePair, CommandModePairHash>;

std::array<std::byte, 4*1024> MDependentCmdsOfNewCmdBuf;
std::pmr::monotonic_buffer_resource MDependentCmdsOfNewCmdBufRes{MDependentCmdsOfNewCmdBuf.data(),
MDependentCmdsOfNewCmdBuf.size()};
CommandModePairSet MDependentCmdsOfNewCmd{&MDependentCmdsOfNewCmdBufRes};

void addDep(const DepDesc &Dep) {
MDependentCmdsOfNewCmd.emplace(Dep.MDepRequirement->MSYCLMemObj, Dep.MDepRequirement->MAccessMode);
}
public:

MapOfDependentCmds(const std::vector<DepDesc> &Deps) {
for (const DepDesc &Dep : Deps)
addDep(Dep);
}

void addDeps(const std::vector<DepDesc> &Deps) {
MDependentCmdsOfNewCmd.clear();
for (const DepDesc &Dep : Deps)
addDep(Dep);
}

bool isMemObjExist(const std::pair<SYCLMemObjI *, access::mode> &Mo) const {
return MDependentCmdsOfNewCmd.count(Mo);
}
};

} // namespace detail
} // namespace _V1
} // namespace sycl
23 changes: 17 additions & 6 deletions sycl/unittests/scheduler/GraphCleanup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ static void checkCleanupOnEnqueue(MockScheduler &MS,
ToCleanUp);
EXPECT_TRUE(ToCleanUp.empty());
MS.addNodeToLeaves(Record, MockCmd, access::mode::read_write, ToEnqueue);
MS.updateLeaves({AllocaCmd}, Record, access::mode::read_write, ToCleanUp);
detail::MapOfDependentCmds DependentCmdsOfNewCmd(MockCmd->MDeps);
MS.updateLeaves({AllocaCmd}, Record, access::mode::read_write,
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);

EXPECT_TRUE(ToCleanUp.empty());
std::unique_ptr<detail::CG> CG{
Expand All @@ -107,7 +109,9 @@ static void checkCleanupOnEnqueue(MockScheduler &MS,
MockCmd = new MockCommandWithCallback(QueueImpl, MockReq, Callback);
addEdge(MockCmd, Cmd, AllocaCmd);
MS.addNodeToLeaves(Record, MockCmd, access::mode::read_write, ToEnqueue);
MS.updateLeaves({Cmd}, Record, access::mode::read_write, ToCleanUp);
DependentCmdsOfNewCmd.addDeps(MockCmd->MDeps);
MS.updateLeaves({Cmd}, Record, access::mode::read_write,
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);
MS.addHostAccessor(&MockReq);
verifyCleanup(Record, AllocaCmd, MockCmd, CommandDeleted);

Expand All @@ -132,7 +136,9 @@ static void checkCleanupOnEnqueue(MockScheduler &MS,
// Since this mock command has already been enqueued, it's expected to be
// cleaned up during removal from leaves.
ToCleanUp.clear();
MS.updateLeaves({MockCmd}, Record, access::mode::read_write, ToCleanUp);
detail::MapOfDependentCmds DependentCmdsOfNewCmd(LeafMockCmd->MDeps);
MS.updateLeaves({MockCmd}, Record, access::mode::read_write,
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);
EXPECT_EQ(ToCleanUp.size(), 1U);
EXPECT_EQ(ToCleanUp[0], MockCmd);
MS.cleanupCommands({MockCmd});
Expand All @@ -143,7 +149,9 @@ static void checkCleanupOnEnqueue(MockScheduler &MS,
addEdge(LeafMockCmd, MockCmd, AllocaCmd);
MS.addNodeToLeaves(Record, LeafMockCmd, access::mode::read_write,
ToEnqueue);
MS.updateLeaves({MockCmd}, Record, access::mode::read_write, ToCleanUp);
DependentCmdsOfNewCmd.addDeps(LeafMockCmd->MDeps);
MS.updateLeaves({MockCmd}, Record, access::mode::read_write,
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);
return MockCmd;
};

Expand Down Expand Up @@ -184,7 +192,9 @@ static void checkCleanupOnLeafUpdate(
ToCleanUp);
EXPECT_TRUE(ToCleanUp.empty());
MS.addNodeToLeaves(Record, MockCmd, access::mode::read_write, ToEnqueue);
MS.updateLeaves({AllocaCmd}, Record, access::mode::read_write, ToCleanUp);
const detail::MapOfDependentCmds DependentCmdsOfNewCmd(MockCmd->MDeps);
MS.updateLeaves({AllocaCmd}, Record, access::mode::read_write,
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);
detail::EnqueueResultT Res;
MockScheduler::enqueueCommand(MockCmd, Res, detail::BLOCKING);

Expand Down Expand Up @@ -260,9 +270,10 @@ TEST_F(SchedulerTest, PostEnqueueCleanup) {
MS.addNodeToLeaves(Record, MockCmd.get(), access::mode::read_write,
ToEnqueue);
}
const detail::MapOfDependentCmds DependentCmdsOfNewCmd(AllocaCmd->MDeps);
for (std::unique_ptr<MockCommand> &MockCmd : Leaves)
MS.updateLeaves({MockCmd.get()}, Record, access::mode::read_write,
ToCleanUp);
DependentCmdsOfNewCmd, QueueImpl, ToCleanUp);
EXPECT_TRUE(ToCleanUp.empty());
});
}
Expand Down
5 changes: 4 additions & 1 deletion sycl/unittests/scheduler/SchedulerTestUtils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,11 @@ class MockScheduler : public sycl::detail::Scheduler {
void updateLeaves(const std::set<sycl::detail::Command *> &Cmds,
sycl::detail::MemObjRecord *Record,
sycl::access::mode AccessMode,
const sycl::detail::MapOfDependentCmds &DependentCmdsOfNewCmd,
const sycl::detail::QueueImplPtr &Queue,
std::vector<sycl::detail::Command *> &ToCleanUp) {
return MGraphBuilder.updateLeaves(Cmds, Record, AccessMode, ToCleanUp);
return MGraphBuilder.updateLeaves(Cmds, Record, AccessMode, DependentCmdsOfNewCmd,
Queue, ToCleanUp);
}

static bool enqueueCommand(sycl::detail::Command *Cmd,
Expand Down

0 comments on commit f74ad1a

Please sign in to comment.