Skip to content
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

[GPU] Improvement of event-related primitives code #27337

Merged

Conversation

vladimir-paramuzov
Copy link
Contributor

Details:

  • Removed _events map from network class. Now dependency and result events are stored in kernel_impl_params for each primitive
  • User events are not created for CPU impls with barrier based synchronization to avoid useless OCL API calls (clCreateUserEvent -> clSetUserEventStatus -> clReleaseEvent). Overall, methods can return nullptr instead of user event.
  • Update ocl_stream::wait_for_events impl to deal with C event handles (cl_event) instead of C++ wrapper to avoid redundant clRetainEvent call
  • Introduced ExecutionFlags structure which reflects an execution status of primitive. Now methods which prepares dynamic primitive for execution modify/check some flags instead of some primitive_inst attributes or explicit function arguments.

@vladimir-paramuzov vladimir-paramuzov added this to the 2025.0 milestone Oct 30, 2024
@vladimir-paramuzov vladimir-paramuzov requested review from a team as code owners October 30, 2024 13:14
@github-actions github-actions bot added the category: GPU OpenVINO GPU plugin label Oct 30, 2024
@vladimir-paramuzov vladimir-paramuzov force-pushed the ev_update branch 2 times, most recently from e5a5b26 to 78ab0ce Compare October 31, 2024 13:03
@vladimir-paramuzov vladimir-paramuzov force-pushed the ev_update branch 2 times, most recently from b6e9892 to b7a66e1 Compare November 12, 2024 07:38
Signed-off-by: Vladimir Paramuzov <[email protected]>
Signed-off-by: Vladimir Paramuzov <[email protected]>
Signed-off-by: Vladimir Paramuzov <[email protected]>
@@ -36,7 +36,7 @@ struct network_output {
// TODO: in_order queue doesn't create proper output event in some cases which leads to syncronization issues with user app
// So call finish for associated stream to enusre that the output data is ready.
if (do_sync) {
if (_stream->get_queue_type() == QueueTypes::in_order) {
if (_stream->get_queue_type() == QueueTypes::in_order || !_event) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When could event not be created for out-of-order queue?
If it's really possible, then this can add additional overhead in some cases, because currently some outputs can be processed asynchronously in SyncInferRequest::wait() function, however with this change it has to wait for all kernels to finish

Copy link
Contributor Author

@vladimir-paramuzov vladimir-paramuzov Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it happened when output primitive has CPU impl type which doesn't produce events anymore for barrier-based synchronization

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_output_event seems to handle this case as well?

@vladimir-paramuzov vladimir-paramuzov added this pull request to the merge queue Nov 18, 2024
Merged via the queue into openvinotoolkit:master with commit f72d958 Nov 18, 2024
150 checks passed
@vladimir-paramuzov vladimir-paramuzov deleted the ev_update branch November 18, 2024 12:52
ababushk pushed a commit to ababushk/openvino that referenced this pull request Nov 18, 2024
…7337)

### Details:
- Removed `_events` map from network class. Now dependency and result
events are stored in `kernel_impl_params` for each primitive
- User events are not created for CPU impls with barrier based
synchronization to avoid useless OCL API calls (clCreateUserEvent ->
clSetUserEventStatus -> clReleaseEvent). Overall, methods can return
nullptr instead of user event.
- Update ocl_stream::wait_for_events impl to deal with C event handles
(cl_event) instead of C++ wrapper to avoid redundant clRetainEvent call
- Introduced ExecutionFlags structure which reflects an execution status
of primitive. Now methods which prepares dynamic primitive for execution
modify/check some flags instead of some primitive_inst attributes or
explicit function arguments.

---------

Signed-off-by: Vladimir Paramuzov <[email protected]>
NishantPrabhuFujitsu pushed a commit to NishantPrabhuFujitsu/openvino that referenced this pull request Nov 26, 2024
…7337)

### Details:
- Removed `_events` map from network class. Now dependency and result
events are stored in `kernel_impl_params` for each primitive
- User events are not created for CPU impls with barrier based
synchronization to avoid useless OCL API calls (clCreateUserEvent ->
clSetUserEventStatus -> clReleaseEvent). Overall, methods can return
nullptr instead of user event.
- Update ocl_stream::wait_for_events impl to deal with C event handles
(cl_event) instead of C++ wrapper to avoid redundant clRetainEvent call
- Introduced ExecutionFlags structure which reflects an execution status
of primitive. Now methods which prepares dynamic primitive for execution
modify/check some flags instead of some primitive_inst attributes or
explicit function arguments.

---------

Signed-off-by: Vladimir Paramuzov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: GPU OpenVINO GPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants