Skip to content

Conversation

@sergey-semenov
Copy link
Contributor

Current implementation of profiling info for NOP barriers is inconsistent
with other events from the same queue (e.g., if the previous event started
after the barrier was submitted). To make them consistent while keeping
the optimization, we would need to duplicate the event on our side and
make the duplicate check and potentially use profiling info of its
previous event.

Instead, as the first step, disable the NOP optimization during profiling
since profiling is known to incur a performance hit anyway. The proper
duplicate event approach can be implemented as a follow up if this
causes issues for users.

Partially reverts #12949

Current implementation of profiling info for NOP barriers is inconsistent
with other events from the same queue. To make them consistent while keeping the optimization, we would
need to duplicate the event on our side and make the duplicate also check
and potentially use profiling information of its previous event.

Instead, as the first step, disable the NOP optimization during profiling
since profiling is known to incur a performance hit anyway. The proper
duplicate event approach can be implemented as a follow up if this causes issues for
users.
@sergey-semenov sergey-semenov merged commit f2cd2a8 into intel:sycl Jun 13, 2024
ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
…ntel#14123)

Current implementation of profiling info for NOP barriers is
inconsistent
with other events from the same queue (e.g., if the previous event
started
after the barrier was submitted). To make them consistent while keeping
the optimization, we would need to duplicate the event on our side and
make the duplicate check and potentially use profiling info of its
previous event.

Instead, as the first step, disable the NOP optimization during
profiling
since profiling is known to incur a performance hit anyway. The proper
duplicate event approach can be implemented as a follow up if this
causes issues for users.

Partially reverts intel#12949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants