Skip to content

Use a no-op order manager for in-order queues - #2299

Open
ndgrigorian wants to merge 2 commits into
masterfrom
no-op-order-manager-for-in-order-queues
Open

ndgrigorian wants to merge 2 commits into
masterfrom
no-op-order-manager-for-in-order-queues

Conversation

@ndgrigorian

@ndgrigorian ndgrigorian commented Apr 26, 2026 •

Copy link
Copy Markdown
Collaborator

As pointed out in #2293, an in-order queue already orders the tasks submitted to it, including tasks submitted by extensions that do not use SequentialOrderManager. What the order manager did not do for such a queue is account for those tasks: SequentialOrderManager[q].wait() returned before them, and submitted_events did not include them, so work on another queue depending on submitted_events could race with them.

This PR:

  • adds SyclQueue.get_last_event(), wrapping ext_oneapi_get_last_event() from the DPC++ sycl_ext_oneapi_in_order_queue_events extension
  • makes SequentialOrderManager[q] return an _InOrderManager for in-order queues. It keeps only the most recently
    added events. submitted_events is the queue's last event when get_last_event is available, and the most recently added computation events otherwise. wait() waits on the queue.
  • caches _InOrderManager in the thread-local map, so clear() at exit and thread exit wait on in-order queues as they do on out-of-order ones.
  • types the return value of SequentialOrderManager[q] as OrderManagerProtocol.
  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?
  • If this PR is a work in progress, are you opening the PR as a draft?

@ndgrigorian
ndgrigorian force-pushed the no-op-order-manager-for-in-order-queues branch from 3784662 to 849b0af Compare April 26, 2026 23:00
@github-actions

Copy link
Copy Markdown

@ndgrigorian
ndgrigorian force-pushed the no-op-order-manager-for-in-order-queues branch from 849b0af to 115b184 Compare April 26, 2026 23:38
@coveralls

coveralls commented Apr 27, 2026 •

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 75.26% (+0.3%) from 74.989% — no-op-order-manager-for-in-order-queues into master

@ndgrigorian
ndgrigorian marked this pull request as ready for review April 29, 2026 16:02
@ndgrigorian

Copy link
Copy Markdown
Collaborator Author

@antonwolfy @vlad-perevezentsev
this is ready for review, since this impacts order manager, it will impact all of tensor and dpnp. On my tests so far, I have seen no issues, but I will keep trying.

Please experiment with it as well

Comment thread dpctl/utils/_order_manager.py Resolved
Comment thread dpctl/utils/_order_manager.py Resolved
Wraps ext_oneapi_get_last_event() from sycl_ext_oneapi_in_order_queue_events,
with DPCTLQueue_GetLastEvent and DPCTLQueue_GetLastEvent_Available in the
C-API. Builds without the extension report it as unavailable.
_InOrderManager keeps only the most recently added events, and
submitted_events is the queue's last event when get_last_event is
available. It is cached like _SequentialOrderManager, so clear() and
thread exit wait on the queue.
@ndgrigorian
ndgrigorian force-pushed the no-op-order-manager-for-in-order-queues branch from 6db4cd7 to 2d6d8ea Compare November 2, 2026 08:19

This branch has not been deployed

No deployments
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