address third review of deferred releases

Keep DeferredReleases private behind a SyclQueue_DeferRelease C-API function,
drop the poll interval cap, restart the thread after a timed-out stop, keep
one named thread state, and drop the order manager's runtime deprecation
warnings.

Author: ndgrigorian

CHANGELOG.md

@@ -9,12 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 * Added `dpctl.SyclQueue.keep_args_alive` method, which keeps Python objects alive until the work on that queue using them completes [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 * Added `add_event` and `add_cleanup_event` methods and `cleanup_events` and `num_cleanup_events` properties to the order manager, for tracking the events that gate the release of objects used by offloaded tasks [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
+* Added `SyclQueue_DeferRelease` C-API function, which defers a release callback until work submitted to a queue completes, and through which `dpctl::utils::keep_args_alive` keeps its arguments alive [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 
 ### Changed
 * Object lifetime during offload is now managed by a list of releases deferred until the events that gate them have completed, rather than by a `host_task`. `dpctl` keeps one thread for looking over the list and running the releases that have come due, started by the first release deferred and again in the child of a `fork`, which takes the GIL to do so and lets a release call into the interpreter and free USM allocations [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 * The event returned by `dpctl.SyclQueue.keep_args_alive`, `dpctl::utils::keep_args_alive` and `dpctl.SyclQueue._submit_keep_args_alive` is now for an empty kernel that gates the deferred release rather than for a `host_task` that performs it, so its completion means that the objects are no longer in use rather than that they have been released [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 
 ### Deprecated
+* Deprecated `dpctl.SyclQueue._submit_keep_args_alive` in favor of `dpctl.SyclQueue.keep_args_alive`, which it now calls and which behaves identically [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
+* Deprecated the order manager's `add_event_pair`, `host_task_events` and `num_host_task_events`, as `host_task` is no longer used for managing object lifetimes, in favor of `add_event`, `add_cleanup_event`, `cleanup_events` and `num_cleanup_events`; they do not yet emit a `DeprecationWarning` [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 
 ### Removed
 * Removed the deprecated `dpctl.program` submodule, use `dpctl.compiler` instead [gh-2407](https://github.com/IntelPython/dpctl/pull/2407)
@@ -71,8 +74,6 @@ This release extends the SYCL interface exposed by `dpctl` in several directions
 * Deprecated `dpctl.program.create_program_from_source` and `dpctl.program.create_program_from_spirv` in favor of `dpctl.compiler.create_kernel_bundle_from_source` and `dpctl.compiler.create_kernel_bundle_from_spirv`; the old functions are kept as aliases that emit a `DeprecationWarning` [gh-2291](https://github.com/IntelPython/dpctl/pull/2291), [gh-2317](https://github.com/IntelPython/dpctl/pull/2317)
 * Deprecated `dpctl.program` submodule in favor of `dpctl.compiler`, which provides a better description of the purpose of the submodule in exposing DPC++ compilation-related functionality [gh-2317](https://github.com/IntelPython/dpctl/pull/2317)
 * Deprecated `DPCTL_ENABLE_L0_PROGRAM_CREATION` CMake option in favor of `DPCTL_ENABLE_L0_KERNEL_BUNDLE_CREATION` [gh-2317](https://github.com/IntelPython/dpctl/pull/2317)
-* Deprecated `dpctl.SyclQueue._submit_keep_args_alive` in favor of `dpctl.SyclQueue.keep_args_alive`, which it now calls and which behaves identically [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
-* Deprecated the order manager's `add_event_pair`, `host_task_events` and `num_host_task_events`, as `host_task` is no longer used for managing object lifetimes, in favor of `add_event`, `add_cleanup_event`, `cleanup_events` and `num_cleanup_events` [gh-2359](https://github.com/IntelPython/dpctl/pull/2359)
 
 ### Removed
 * Removed unused `platform_typeid_lookup` from `dpctl4pybind11.hpp` [gh-2286](https://github.com/IntelPython/dpctl/pull/2286)

docs/doc_sources/api_reference/dpctl/utils.rst

@@ -27,9 +27,11 @@
 
     Waiting with ``wait`` also drops the references that
     :meth:`dpctl.SyclQueue.keep_args_alive` took for tasks that have since
-    completed, which ``dpctl`` otherwise does on a thread of its own.
+    completed, which ``dpctl`` otherwise does on a thread of its own. This is
+    best-effort: references that thread is already dropping are left to it, so
+    they may still be held when ``wait`` returns.
 
-    .. deprecated:: 0.23.0
+    .. deprecated:: 0.24.0
         ``add_event_pair``, ``host_task_events`` and ``num_host_task_events``
         are deprecated. Tasks are no longer paired with a host task event, so
         ``add_event`` takes the computational event alone, and cleanup is

dpctl/CMakeLists.txt

@@ -198,12 +198,8 @@ endforeach()
 set(_cy_file ${CMAKE_CURRENT_SOURCE_DIR}/_sycl_queue.pyx)
 get_filename_component(_trgt ${_cy_file} NAME_WLE)
 build_dpctl_ext(${_trgt} ${_cy_file} "dpctl" SYCL)
-# _sycl_queue includes _async_dec_ref.hpp, which reaches
-# detail/deferred_releases.hpp in the public include directory
-target_include_directories(${_trgt} PRIVATE
-    ${CMAKE_CURRENT_SOURCE_DIR}
-    ${CMAKE_CURRENT_SOURCE_DIR}/apis/include
-)
+# _sycl_queue includes _async_dec_ref.hpp
+target_include_directories(${_trgt} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
 target_link_libraries(DpctlCAPI INTERFACE ${_trgt}_headers)
 
 add_subdirectory(compiler)

dpctl/_async_dec_ref.hpp

@@ -19,9 +19,10 @@
 //===----------------------------------------------------------------------===//
 ///
 /// \file
-/// This file implements a utility function to decrement reference counts for a
-/// given array of Python objects once a given array of sycl events has
-/// completed.
+/// This file implements the functions that defer releasing what offloaded work
+/// uses until that work has completed: decrementing the reference counts of an
+/// array of Python objects, or calling a release function another module
+/// passes in.
 ///
 /// N.B.: The reference counts are dropped by the thread `dpctl` keeps for
 /// running deferred releases, or by a thread that drains them.
@@ -42,6 +43,32 @@
 #include "syclinterface/dpctl_data_types.h"
 #include "syclinterface/dpctl_sycl_type_casters.hpp"
 
+namespace dpctl
+{
+namespace detail
+{
+
+/*!
+ * @brief Name of the kernel submitted by `submit_keep_alive_marker`.
+ */
+class keep_alive_marker;
+
+/*!
+ * @brief An event that gates the release of objects used by work on `q`.
+ *
+ * Submits an empty kernel after `deps`. On an in-order queue the submission is
+ * also ordered after everything already submitted there, so the event covers
+ * uses of the objects that the caller never tracked.
+ */
+inline sycl::event
+submit_keep_alive_marker(sycl::queue &q, const std::vector<sycl::event> &deps)
+{
+    return q.single_task<keep_alive_marker>(deps, []() {});
+}
+
+} // namespace detail
+} // namespace dpctl
+
 namespace
 {
 
@@ -62,6 +89,18 @@ std::vector<sycl::event> unwrap_events(DPCTLSyclEventRef *depERefs,
     return depends;
 }
 
+/*!
+ * @brief The queue behind `QRef`, which must not be null.
+ */
+sycl::queue &unwrap_queue(DPCTLSyclQueueRef QRef)
+{
+    sycl::queue *q = dpctl::syclinterface::unwrap<sycl::queue>(QRef);
+    if (!q) {
+        throw std::invalid_argument("A queue is required");
+    }
+    return *q;
+}
+
 /*!
  * @brief Schedule DECREFs of `obj_vec` for once `depends` have completed.
  */
@@ -98,22 +137,18 @@ inline DPCTLSyclEventRef async_dec_ref(DPCTLSyclQueueRef QRef,
                                        size_t nDepERefs,
                                        int *status)
 {
-    using dpctl::syclinterface::unwrap;
     using dpctl::syclinterface::wrap;
 
     try {
-        sycl::queue *q = unwrap<sycl::queue>(QRef);
-        if (!q) {
-            throw std::invalid_argument("A queue is required");
-        }
+        sycl::queue &q = unwrap_queue(QRef);
 
         std::vector<sycl::event> depends = unwrap_events(depERefs, nDepERefs);
         std::vector<PyObject *> obj_vec(obj_array, obj_array + obj_array_size);
 
         static constexpr int result_ok = 0;
 
         const sycl::event marker =
-            dpctl::detail::submit_keep_alive_marker(*q, depends);
+            dpctl::detail::submit_keep_alive_marker(q, depends);
 
         // allocated before scheduling, as failing afterwards could not be
         // reported: the caller would drop a reference the scheduled DECREFs own
@@ -131,3 +166,42 @@ inline DPCTLSyclEventRef async_dec_ref(DPCTLSyclQueueRef QRef,
         return nullptr;
     }
 }
+
+/*!
+ * @brief Call `release(data)` once the work using what it releases is done.
+ *
+ * Submits an empty kernel to `QRef` after `depERefs`, as `async_dec_ref` does,
+ * and assigns its event to the `sycl::event` behind `markerERef`. `release` is
+ * called with the GIL held, on `dpctl`'s thread or on one that drains.
+ *
+ * @return 0 on success, and 1 if nothing was deferred, in which case `release`
+ * is never called.
+ */
+inline int defer_release(DPCTLSyclQueueRef QRef,
+                         DPCTLSyclEventRef *depERefs,
+                         size_t nDepERefs,
+                         void (*release)(void *),
+                         void *data,
+                         DPCTLSyclEventRef markerERef)
+{
+    using dpctl::syclinterface::unwrap;
+
+    try {
+        sycl::queue &q = unwrap_queue(QRef);
+        sycl::event *marker = unwrap<sycl::event>(markerERef);
+        if (!release || !marker) {
+            throw std::invalid_argument("A release and an event are required");
+        }
+
+        *marker = dpctl::detail::submit_keep_alive_marker(
+            q, unwrap_events(depERefs, nDepERefs));
+
+        dpctl::detail::local_deferred_releases().defer(
+            {*marker}, [release, data]() { release(data); });
+
+        return 0;
+    } catch (...) {
+        // no exception may escape into the calling Cython code
+        return 1;
+    }
+}