[XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL #1557
Conversation
+ Implementation of the specification in llvm/xpti + Documentation on the API and the architecture of the framework + Unit tests and additional semantic and performance tests + Sample collector (subscriber) to attach to an instrumented application and print out the trace data being received Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
+ Minor formatting changes and spelling fixes + Implementation of std container based framework that doesn't depend on TBB Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
+ The framework is fully enabled to use TBB or the standard library containers + The default build will use standard library containers in the implementation in order to remove the explicit dependency on TBB + Tests that use TBB for multi-threaded tests are disabled by default + TBB can be enabled with the soft option -DXPTI_ENABLE_TBB=ON Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
tovinkere
changed the title
|
LGTM, but I'd like @alexbatashev to approve before merging. |
+ Coding conventions mismatch Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
| plugin_data_t queryPlugin(xpti_plugin_handle_t Handle) { | ||
| plugin_data_t PData; | ||
| #ifdef XPTI_USE_TBB | ||
| tbb::spin_mutex::scoped_lock MyLock(MMutex); |
There was a problem hiding this comment.
Perhaps you could suggest the TBB team to provide a standard mutex API?
There was a problem hiding this comment.
@keryell For the critical section sizes here, the spin locks give better performance with increasing number of threads - as in the events/sec remain relatively constant whereas the std::mutex tends to drop off when threads are > 4ish. I am not sure if there are use cases that enqueue kernels from 10s of threads, but it is nice to have an option to handle that nicely, if required. This is the reason why std containers are the default and if required, users can enable TBB.
There was a problem hiding this comment.
Ah sure. I agree with that. I was really talking about the API, not the mutex implementation details or semantics.
For example it would be nice to use it with:
std::unique_lock<tbb::spin_mutex> MyLock(MMutex);There was a problem hiding this comment.
Ah, I see it now - I'll definitely suggest this to the team.
| printf("Notification statistics:\n"); | ||
| for (auto &s : MStats) { | ||
| printf("%19s: [%llu] \n", | ||
| stringify_trace_type((xpti_trace_point_type_t)s.first).c_str(), |
There was a problem hiding this comment.
At least with std::cout there is no need to go back to a C string :-)
There was a problem hiding this comment.
:-) Agreed, but formatting strings in iostream is too verbose - and printfs are used only when someone enables usage statistics
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Thank you Ronan. I appreciate the fact that you noticed it :-) Considering that we are instrumenting the SYCL runtime using this framework, it is only fair that the tests observe due diligence as well. It also gives us a good handle on how many events we can service when trying to keep the overheads low. |
@bader I believe @alexbatashev had approved it but my patch to address feedback from @keryell appears to have dismissed it. @alexbatashev and @keryell - Any objections? If not, I'd like this to be merged sooner than later. Thanks! |
…_docs * origin/sycl: [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557) [SYCL] Initial ABI checks implementation (intel#1528) [SYCL] Support connection with multiple plugins (intel#1490) [SYCL] Add a new header file with the reduction class definition (intel#1558) [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536)
…versioning * origin/sycl: [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557) [SYCL] Initial ABI checks implementation (intel#1528) [SYCL] Support connection with multiple plugins (intel#1490) [SYCL] Add a new header file with the reduction class definition (intel#1558) [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536) [SYCL][CUDA] Move interop tests (intel#1570) [Driver][SYCL] Remove COFF object format designator for Windows device compiles (intel#1574) [SYCL] Fix conflicting visibility attributes (intel#1571) [SYCL][DOC] Update the SYCL Runtime Interface document with design details (intel#680) [SYCL] Improve image accessors support on a host device (intel#1502) [SYCL] Make queue's non-USM event ownership temporary (intel#1561) [SYCL] Added support of rounding modes for non-host devices (intel#1463) [SYCL] SemaSYCL significant refactoring (intel#1517) [SYCL] Support 0-dim accessor in handler::copy(accessor, accessor) (intel#1551)
This pull-request is for the reference implementation of the XPTI framework so users can pull the instrumentation data enabled in the SYCL runtime.