intel / intel/llvm

Optimization potential of synchronization on buffer destruction?

Open
#2,036 4 comments 0 reactions 0 assignees View on GitHub
confirmed performance
Dominant language
LLVM
Stars
1.5k
Forks
854
Avg merge
3d 17h
Merged PRs (30d)
137

Description

When a buffer is destroyed that requires device memory to be made available on host, after sumitting a command group, I am observing at least 3 PI event waits. I wonder if they are necessary or if there is potential to reduce PI API call overhead?

For example, when running the LIT test for [`sycl/test/basic_tests/buffer/buffer.cpp`](https://github.com/intel/llvm/blob/sycl/sycl/test/basic_tests/buffer/buffer.cpp) as follows:
```sh
cd

./bin/llvm-lit -a -v ./tools/sycl/test/ --param SYCL_BE=PI_OPENCL --filter=.*buffer/buffer.cpp
```

Then take the second last printed run line to repeat it by hand:
```sh
$ ":" "RUN: at line 6"
$ "env" "SYCL_DEVICE_TYPE=GPU" "SYCL_BE=PI_OPENCL" "/$( pwd )/tools/sycl/test/basic_tests/buffer/Output/buffer.cpp.tmp2.out"
# command output:
move constructor
move assignment operator
copy constructor
copy assignment operator
```
and run it as follows:
```sh
"env" LD_LIBRARY_PATH=$( pwd )/lib SYCL_PI_TRACE=2 "SYCL_DEVICE_TYPE=GPU" "SYCL_BE=PI_OPENCL" "/$( pwd )/tools/sycl/test/basic_tests/buffer/Output/buffer.cpp.tmp2.out" > trace.txt
```

For the [first test case](https://github.com/intel/llvm/blob/sycl/sycl/test/basic_tests/buffer/buffer.cpp#L27):
```cpp
{
int data1[10] = {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1};
{
buffer b(data1, range<1>(10), {property::buffer::use_host_ptr()});
queue myQueue;
myQueue.submit([&](handler &cgh) {
auto B = b.get_access(cgh);
cgh.parallel_for(range<1>{10},
[=](id<1> index) { B[index] = 0; });
});

} // Data is copied back because there is a user side shared_ptr
for (int i = 0; i < 10; i++)
assert(data1[i] == 0);
}
```

results in this trace just before the buffer `b` is destroyed:
```txt
...
---> piEnqueueKernelLaunch(
: 0x26b3520
: 0x65fcfd0
: 1
: 0x3167420
: 0x31673f0
: 0
: 0
: 0
: 0x27b0310
) ---> pi_result : PI_SUCCESS
---> piEnqueueMemBufferRead(
: 0x26b3520
: 0x27af510
: 0
: 0
: 40
: 6
: 1
: 0x2967810
: 0x2827030
) ---> pi_result : PI_SUCCESS
---> piEventsWait(
: 1
: 0x2827030
) ---> pi_result : PI_SUCCESS
---> piEventsWait(
: 1
: 0x2827030
) ---> pi_result : PI_SUCCESS
---> piEventsWait(
: 1
: 0x27b0310
) ---> pi_result : PI_SUCCESS
---> piMemRelease(
: 0x27af510
) ---> pi_result : PI_SUCCESS
...
```

The buffer destruction triggers the `piEnqueueMemBufferRead` call. However, why are there then:
* 2 `piEventsWait`s on the event returned by `piEnqueueMemBufferRead` (`0x2827030`), and
* 1 extra wait on the event returned by `piEnqueueKernelLaunch` (`0x27b0310`)?

I probably miss a requirement from the SYCL specs though my expectation would be that on the host side, only 1 `piEventsWait` on the event representing `piEnqueueMemBufferRead` would have been required?

Especially for small kernels or operations this could reduce the overhead of extra API calls.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.