-
Notifications
You must be signed in to change notification settings - Fork 115
[Dynamic Selection] Customization of Backends and Policies #2508
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
Conversation
…t after, also removed submit from the sycl_backend. Now uses the base implementation
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
documentation/library_guide/dynamic_selection_api/functions.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
| and ``submit_and_wait`` functions select a resource and then pass the chosen resource | ||
| to a developer-provided function. | ||
| Policy objects are used as arguments to the dynamic selection functions. The | ||
| ``try_submit`` function attempts to select a resource and returns null if none are |
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.
Does this return a std::optional now?
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.
good catch, fixed.
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
| Lazy Reporting | ||
| -------------- |
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.
I wonder if we should name it "on-demand reporting" or similarly - at least in the documentation (while the API naming can be discussed / addressed later).
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.
@vossmjp Do you have an opinion here? This is not something new in the design, but just something we've documented. I have no strong opinions here, but there are a few places we would have to change it if we choose to.
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.
To be clear, my comment is primarily about the documentation. And it's also minor and can be postponed.
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.
I don't have a strong objection to the change itself, but I'd prefer to defer all but essential changes at this point so we can get things merged.
documentation/library_guide/dynamic_selection_api/custom_policies.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
akukanov
left a comment
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.
This approval is for the public documentation update in the PR. I have not reviewed anything else.
vossmjp
left a comment
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.
LGTM
SergeyKopienko
left a comment
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.
LGTM
Dynamic Selection API: Backend and Policy Customization & Removal of Selection API
This PR refactors the Dynamic Selection API to introduce a flexible backend architecture and simplify the user-facing API by removing the
select()function. It provides better tools for customization of backend and policies to allow for easier customization and more flexibility for users.Implements RFCs #2220.
Key Changes
Backend Architecture
policy_base.handdefault_backend.hto provide common base classes for policies and backends respectivelyResourceAdapterfunction to support different flavors of resource with the same backend (e.g.sycl::queuevssycl::queue*)API Simplification
select()function - selections are now internal implementation detailstry_submit- always returns astd::optionalquickly, returns empty if unable to obtain a resourcetry_submit,submit()andsubmit_and_wait()functionstry_select_impl()(returnsstd::optional) instead of publicselect()Execution Info & Reporting
Summary of changes to look out for from individual components: