Skip to content

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

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.

Sign up for GitHub

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

Open
ndgrigorian wants to merge 6 commits into
base:master
Choose a base branch
from
+63 −2
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5de914
make SequentialOrderManager no-op for in-order queues
ndgrigorian Apr 26, 2026
3759089
fix typing in _order_manager
ndgrigorian Apr 26, 2026
115b184
add _NoOpOrderManager tests
ndgrigorian Apr 26, 2026
30b9d1c
fix TypeError in SequentialOrderManager
ndgrigorian Apr 26, 2026
2fd5bc0
Merge branch 'master' into no-op-order-manager-for-in-order-queues
ndgrigorian Apr 27, 2026
6db4cd7
Merge branch 'master' into no-op-order-manager-for-in-order-queues
ndgrigorian May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions dpctl/tests/test_utils.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,3 +92,25 @@ def test_order_manager():
_passed = True
finally:
assert _passed


def test_no_op_order_manager():
try:
q = dpctl.SyclQueue(property=("in_order",))
except dpctl.SyclQueueCreationError:
pytest.skip("Queue could not be created for default-selected device")
assert q.is_in_order
_mngr = dpctl.utils.SequentialOrderManager[q]
assert _mngr.num_host_task_events == 0
assert _mngr.num_submitted_events == 0
assert isinstance(_mngr.submitted_events, list)
assert len(_mngr.submitted_events) == 0
assert isinstance(_mngr.host_task_events, list)
assert len(_mngr.host_task_events) == 0
_mngr.add_event_pair(dpctl.SyclEvent(), dpctl.SyclEvent())
_mngr.add_event_pair([dpctl.SyclEvent()], dpctl.SyclEvent())
_mngr.add_event_pair(dpctl.SyclEvent(), [dpctl.SyclEvent()])
# waits on queue
_mngr.wait()
cpy = _mngr.__copy__()
del cpy
43 changes: 41 additions & 2 deletions dpctl/utils/_order_manager.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import weakref
from collections import defaultdict
from contextvars import ContextVar
from typing import Union

from .._sycl_event import SyclEvent
from .._sycl_queue import SyclQueue
Expand DownExpand Up@@ -64,6 +65,39 @@ def __copy__(self):
return res


class _NoOpOrderManager:
"""Dummy order manager used when queue is in-order."""

def __init__(self, q: SyclQueue):
self._queue = q

def add_event_pair(self, _host_task_ev, _comp_ev):
pass

@property
def num_host_task_events(self):
return 0

@property
def num_submitted_events(self):
return 0

@property
def host_task_events(self):
return []

@property
def submitted_events(self):
return []

def wait(self):
# to imitate SequentialOrderManager, wait on the in-order queue
self._queue.wait()

def __copy__(self):
return _NoOpOrderManager(self._queue)


class SyclQueueToOrderManagerMap:
"""Utility class used to ensure sequential ordering of offloaded tasks
when passed to order manager."""
Expand All@@ -74,11 +108,16 @@ def __init__(self):
default=defaultdict(_SequentialOrderManager),
)

def __getitem__(self, q: SyclQueue) -> _SequentialOrderManager:
def __getitem__(
self, q: SyclQueue
) -> Union[_SequentialOrderManager, _NoOpOrderManager]:

@antonwolfyantonwolfyMay 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to implement a protocol instead of union to have a pure type-checking construct?

fromtypingimportProtocolclassOrderManagerProtocol(Protocol):
"""Protocol defining order manager interface"""defadd_event_pair(self, host_task_ev, comp_ev) ->None: ...
@propertydefnum_host_task_events(self) ->int: ...
# ... etcdef__getitem__(self, q: SyclQueue) ->OrderManagerProtocol:
# type checkers understand this is an interface

@ndgrigorianndgrigorianMay 6, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

we could, though also, we could just use | instead of the Union since we restrict to >=3.10 anyway.

I'll try the protocol approach and see if it saves some redundancy

"""Get order manager for given SyclQueue"""
_local = self._map.get()
if not isinstance(q, SyclQueue):
raise TypeError(f"Expected `dpctl.SyclQueue`, got {type(q)}")
if q.is_in_order:
# we don't need to cache the NoOpOrderManager since it's stateless
return _NoOpOrderManager(q)

@antonwolfyantonwolfyMay 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it expected there will be no wait() called for that queue on clear method below?

@antonwolfyantonwolfyMay 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wonder about finalization of tasks from in-order queue at exit

@ndgrigorianndgrigorianMay 6, 2026•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Is it expected there will be no wait() called for that queue on clear method below?

I think it makes sense in the sense that, no events were present in it to begin with. The original SequentialOrderManager calls the wait on its own events, but this order manager, owns no events. But I see your point about the finalization though, it's fair: unlike the normal order manager, this one wouldn't clear.

We will need to track some queues, I think.

_local = self._map.get()
if q in _local:
return _local[q]
else:
Expand Down
Loading