-
Notifications
You must be signed in to change notification settings - Fork 752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify execution graph via analysis of dependencies #15724
base: sycl
Are you sure you want to change the base?
Simplify execution graph via analysis of dependencies #15724
Conversation
15c2e75
to
f74ad1a
Compare
Cmd->MLeafCounter -= Record->MReadLeaves.remove(Cmd); | ||
Cmd->MLeafCounter -= Record->MWriteLeaves.remove(Cmd); | ||
if (WasLeaf && Cmd->readyForCleanup()) { | ||
ToCleanUp.push_back(Cmd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to capture what we discussed here. Instead of adding a new condition where we send commands to cleanup, I think a better solution would be excluding such dependencies from leaves (where both the newest node and its dependency are leaves of the same memory object) and let the usual cleanup path handle it. That can be done by adding a redundant dependency to the graph, the same way we're keeping graph width under control by imposing a leaf limit on each memory object.
That approach would also cover more cases: it's not needed that every dependency of the old command is covered by the new one for us to be able to clean it up. We should only care about how many memory objects the old command is a leaf for, other dependencies shouldn't be relevant.
dbdf3af
to
2e32532
Compare
If a dependent command has all dependencies that has newly added command, the dependent command can be excluded from the execution graph. For in-order queue, all dependent command from current queue can be excluded from an execution graph.
Adding host task to cleanupCommand() leads to its releasing, that may lead to crash if the host task is in ThreadPool.
2e32532
to
ee03c0b
Compare
If a dependent command has all dependencies that has newly added command, the dependent command can be excluded from the execution graph.
For in-order queue, all dependent command from current queue can be excluded from an execution graph.
In current implementation, case with single accessor is supported, but for
auto e = queue.submit([&](auto &h) {
sycl::accessor acc(DataBuf, h);
sycl::accessor acc1(DataBuf1, h, sycl::read_only);
h.parallel_for(range, ...);
});
called in loop a size of dependency graph is limited by LeafLimit.