[OV][ITT][GPU Plugin] Enable default ITT markers for inference and op submission#33313
[OV][ITT][GPU Plugin] Enable default ITT markers for inference and op submission#33313praasz merged 10 commits intoopenvinotoolkit:masterfrom
Conversation
- Enables default ITT markers for higher level operations such as inference pass, op preparation and submission - Follows the same guidelines to standardize the conventions for namespaces: ov::phases::gpu::inference ov::op::gpu - Supports both synchronous and asynchronous operations Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
+ Adding base markers for node execution instead of a prepare() and execute() Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
| void SyncInferRequest::infer() { | ||
| OV_ITT_SCOPED_TASK(itt::domains::intel_gpu_plugin, "SyncInferRequest::infer"); | ||
| // String can be constructed once in the constructor | ||
| OV_ITT_SCOPED_TASK_BASE(itt::domains::intel_gpu_inference, "SyncInferRequestGPU::infer" + m_graph->get_runtime_model()->get_name()); |
There was a problem hiding this comment.
out of curiosity, is it OK to change the name here? you commented not to change the name in other place.. [Warning] The strings in ITT_SCOPED_TASK_BASE should NOT be deleted or edited!
There was a problem hiding this comment.
@isanghao Please suggest what changes you need. I tried to make the strings in TASK_BASE to be similar for all plugins.
There was a problem hiding this comment.
I mean, you changed the name from SyncInferRequest to SyncInferRequestGPU. If it is OK to change the name, why do you leave comment that we should not change the name?
There was a problem hiding this comment.
@isanghao Previous task definition was incomplete. Also, it is not enabled by default. I enable the task by default using OV_ITT_SCOPED_TASK_BASE macro and renamed the string to be indicative of being executed by the GPU plugin. This new string cannot be changed as it acts as a SW contract and all tools that use this data will rely on the strings being invariant.
|
build_jenkins |
- This update fixes the GPU runtime performance regressions due to increased submission times. By caching the required string for each inference, the overheads are eliminated. Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
|
build_jenkins |
| #include "intel_gpu/runtime/debug_configuration.hpp" | ||
|
|
||
| #include <algorithm> | ||
| #include <functional> |
There was a problem hiding this comment.
Please do not include style fix in the PR. It makes review difficult.
There was a problem hiding this comment.
You don't need to reformat the whole file. You can just change the problematic line to pass the test..
There was a problem hiding this comment.
It should consistent with current style in GPU, @isanghao has greater experience in this component I would follow his suggestions
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
src/plugins/intel_gpu/include/intel_gpu/plugin/compiled_model.hpp
Outdated
Show resolved
Hide resolved
- Moving string constructor to where it is used Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
|
@praasz @aobolensk @isanghao Any progress or next steps? |
|
build_jenkins |
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
- Also update the strings to be consistent across all plugins Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
|
@isanghao The style changes have been removed as requested. Should be easily reviewable now. |
Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
|
build_jenkins |
| OV_ITT_DOMAIN(intel_gpu_inference, "ov::phases::gpu::inference"); | ||
| // Domain namespace for all of the operators | ||
| OV_ITT_DOMAIN(intel_gpu_op, "ov::op::gpu"); | ||
| OV_ITT_DOMAIN(intel_gpu_plugin); |
There was a problem hiding this comment.
It would be beneficial to define this domain name as ov::intel_gpu_plugin to maintain consistency with other domains. This ensures the ov:: prefix is present, making it easier for profiling tools to filter and capture all ov events
| OV_ITT_DOMAIN(intel_gpu_plugin); | |
| OV_ITT_DOMAIN(intel_gpu_plugin, "ov::intel_gpu_plugin"); |
… submission (#33313) - Enables default ITT markers for higher level operations such as inference pass, op preparation and submission - Follows the same guidelines to standardize the conventions for namespaces: ov::phases::gpu::inference ov::op::gpu - Supports both synchronous and asynchronous operations Enabling default GPU ITT markers using standard convention - Part 3 This PR is the **third** of a series of PRs to standardize the ITT markers in OpenVINO that will be enabled by default through host-side instrumentation. 1. The first PR addresses the enhancements required in ITT and the framework to support the creation and propagation of IDs when asynchronous execution is in play [PR#33639](#33639). 2. The second PR will standardize ITT markers in the CPU and enhance support to include asynchronous execution [PR#33312](#33312). 3. This **third** PR will enable default markers for GPU plugin to allow visibility into inference pass begin/end and operator preparation and submission within each inference. Follow standardized conventions as described in 1 and 2 4. The final PR will extend the same host side markers for NPU execution, which capturing the inference span and pipeline activity. Summary of the current PR (PR#3) Use the same convention standardized in [PR#33639](#33639) Ensures the namespace for GPU Plugin activity falls under: ov::phases::gpu::inference ov::op::gpu Details: GPU support is enabled with default ITT markers that support synchronous an asynchronous execution. This PR ensures a standardized convention is followed in namespaces used. Tickets: [CVS-179230](https://jira.devtools.intel.com/browse/CVS-179230) @isanghao Please review this as you are generally aware of what was discussed --------- Signed-off-by: Vasanth Tovinkere <vasanth.tovinkere@intel.com>
Enabling default GPU ITT markers using standard convention - Part 3
This PR is the third of a series of PRs to standardize the ITT markers in OpenVINO that will be enabled by default through host-side instrumentation.
Summary of the current PR (PR#3)
Use the same convention standardized in PR#33639
Ensures the namespace for GPU Plugin activity falls under:
ov::phases::gpu::inference
ov::op::gpu
Details:
GPU support is enabled with default ITT markers that support synchronous an asynchronous execution. This PR ensures a standardized convention is followed in namespaces used.
Tickets:
CVS-179230
@isanghao Please review this as you are generally aware of what was discussed