-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
f5de914
3759089
115b184
30b9d1c
2fd5bc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff 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 | ||
|
|
@@ -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.""" | ||
|
|
@@ -74,11 +108,16 @@ def __init__(self): | |
| default=defaultdict(_SequentialOrderManager), | ||
| ) | ||
|
|
||
| def __getitem__(self, q: SyclQueue) -> _SequentialOrderManager: | ||
| def __getitem__( | ||
| self, q: SyclQueue | ||
| ) -> Union[_SequentialOrderManager, _NoOpOrderManager]: | ||
| """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) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it expected there will be no
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder about finalization of tasks from in-order queue at exit
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think it makes sense in the sense that, no events were present in it to begin with. The original We will need to track some queues, I think. |
||
| _local = self._map.get() | ||
| if q in _local: | ||
| return _local[q] | ||
| else: | ||
|
|
||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 theUnionsince we restrict to >=3.10 anyway.I'll try the protocol approach and see if it saves some redundancy