[SYCL] Defer buffer release when no host memory to be updated

sycl/include/sycl/buffer.hpp

@@ -117,6 +117,8 @@ class __SYCL_EXPORT buffer_plain {
 
   size_t getSize() const;
 
+  void handleRelease(bool DefaultAllocator) const;
+
   std::shared_ptr<detail::buffer_impl> impl;
 };
 
@@ -457,7 +459,13 @@ class buffer : public detail::buffer_plain {
 
   buffer &operator=(buffer &&rhs) = default;
 
-  ~buffer() = default;
+  ~buffer() {
+    buffer_plain::
+        handleRelease(/*DefaultAllocator = */
+                      std::is_same<
+                          AllocatorT,
+                          detail::sycl_memory_object_allocator<T>>::value);
+  }
 
   bool operator==(const buffer &rhs) const { return impl == rhs.impl; }
 

sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp

@@ -1379,8 +1379,40 @@ pi_result piKernelRelease(pi_kernel) { DIE_NO_IMPLEMENTATION; }
 
 pi_result piEventCreate(pi_context, pi_event *) { DIE_NO_IMPLEMENTATION; }
 
-pi_result piEventGetInfo(pi_event, pi_event_info, size_t, void *, size_t *) {
-  DIE_NO_IMPLEMENTATION;
+pi_result piEventGetInfo(pi_event Event, pi_event_info ParamName,
+                         size_t ParamValueSize, void *ParamValue,
+                         size_t *ParamValueSizeRet) {
+  if (ParamName != PI_EVENT_INFO_COMMAND_EXECUTION_STATUS) {
+    DIE_NO_IMPLEMENTATION;
+  }
+
+  auto CheckAndFillStatus = [&](const cm_support::CM_STATUS &State) {
+    pi_int32 Result = PI_EVENT_RUNNING;
+    if (State == cm_support::CM_STATUS_FINISHED)
+      Result = PI_EVENT_COMPLETE;
+    if (ParamValue) {
+      if (ParamValueSize < sizeof(Result))
+        return PI_ERROR_INVALID_VALUE;
+      *static_cast<pi_int32 *>(ParamValue) = Result;
+    }
+    if (ParamValueSizeRet) {
+      *ParamValueSizeRet = sizeof(Result);
+    }
+    return PI_SUCCESS;
+  };
+  // Dummy event is already completed ones done by CM.
+  if (Event->IsDummyEvent)
+    return CheckAndFillStatus(cm_support::CM_STATUS_FINISHED);
+
+  if (Event->CmEventPtr == nullptr)
+    return PI_ERROR_INVALID_EVENT;
+
+  cm_support::CM_STATUS Status;
+  int32_t Result = Event->CmEventPtr->GetStatus(Status);
+  if (Result != cm_support::CM_SUCCESS)
+    return PI_ERROR_COMMAND_EXECUTION_FAILURE;
+
+  return CheckAndFillStatus(Status);
 }
 
 pi_result piEventGetProfilingInfo(pi_event Event, pi_profiling_info ParamName,

sycl/source/buffer.cpp

@@ -121,6 +121,13 @@ void buffer_plain::addOrReplaceAccessorProperties(
 
 size_t buffer_plain::getSize() const { return impl->getSizeInBytes(); }
 
+void buffer_plain::handleRelease(bool DefaultAllocator) const {
+  // Try to detach memory object only if impl is going to be released.
+  // Buffer copy will have pointer to the same impl.
+  if (impl.use_count() == 1)
+    impl->detachMemoryObject(impl, DefaultAllocator);
+}
+
 } // namespace detail
 } // __SYCL_INLINE_VER_NAMESPACE(_V1)
 } // namespace sycl

sycl/source/detail/event_impl.cpp

@@ -78,17 +78,19 @@ void event_impl::waitInternal() {
 
 void event_impl::setComplete() {
   if (MHostEvent || !MEvent) {
-    std::unique_lock<std::mutex> lock(MMutex);
+    {
+      std::unique_lock<std::mutex> lock(MMutex);
 #ifndef NDEBUG
-    int Expected = HES_NotComplete;
-    int Desired = HES_Complete;
+      int Expected = HES_NotComplete;
+      int Desired = HES_Complete;
 
-    bool Succeeded = MState.compare_exchange_strong(Expected, Desired);
+      bool Succeeded = MState.compare_exchange_strong(Expected, Desired);
 
-    assert(Succeeded && "Unexpected state of event");
+      assert(Succeeded && "Unexpected state of event");
 #else
-    MState.store(static_cast<int>(HES_Complete));
+      MState.store(static_cast<int>(HES_Complete));
 #endif
+    }
     cv.notify_all();
     return;
   }
@@ -443,6 +445,11 @@ void event_impl::cleanDepEventsThroughOneLevel() {
   }
 }
 
+bool event_impl::isCompleted() {
+  return get_info<info::event::command_execution_status>() ==
+         info::event_command_status::complete;
+}
+
 } // namespace detail
 } // __SYCL_INLINE_VER_NAMESPACE(_V1)
 } // namespace sycl

sycl/source/detail/event_impl.hpp

@@ -234,6 +234,8 @@ class event_impl {
   /// state.
   bool isInitialized() const noexcept { return MIsInitialized; }
 
+  bool isCompleted();
+
 private:
   // When instrumentation is enabled emits trace event for event wait begin and
   // returns the telemetry event generated for the wait

sycl/source/detail/global_handler.cpp

@@ -47,6 +47,14 @@ T &GlobalHandler::getOrCreate(InstWithLock<T> &IWL, Types... Args) {
   return *IWL.Inst;
 }
 
+void GlobalHandler::attachScheduler(Scheduler *Scheduler) {
+  // The method is for testing purposes. Do not protect with lock since
+  // releaseResources will cause dead lock due to host queue release
+  if (MScheduler.Inst)
+    MScheduler.Inst->releaseResources();
+  MScheduler.Inst.reset(Scheduler);
+}
+
 Scheduler &GlobalHandler::getScheduler() { return getOrCreate(MScheduler); }
 
 ProgramManager &GlobalHandler::getProgramManager() {
@@ -142,6 +150,8 @@ void GlobalHandler::unloadPlugins() {
 }
 
 void shutdown() {
+  if (GlobalHandler::instance().MScheduler.Inst)
+    GlobalHandler::instance().MScheduler.Inst->releaseResources();
   // Ensure neither host task is working so that no default context is accessed
   // upon its release
   if (GlobalHandler::instance().MHostTaskThreadPool.Inst)

sycl/source/detail/global_handler.hpp

@@ -75,6 +75,9 @@ class GlobalHandler {
 
   void unloadPlugins();
 
+  // For testing purposes only
+  void attachScheduler(Scheduler *Scheduler);
+
 private:
   friend void releaseDefaultContexts();
   friend void shutdown();

sycl/source/detail/scheduler/scheduler.cpp

@@ -26,6 +26,18 @@ namespace sycl {
 __SYCL_INLINE_VER_NAMESPACE(_V1) {
 namespace detail {
 
+bool Scheduler::checkLeavesCompletion(MemObjRecord *Record) {
+  for (Command *Cmd : Record->MReadLeaves) {
+    if (!Cmd->getEvent()->isCompleted())
+      return false;
+  }
+  for (Command *Cmd : Record->MWriteLeaves) {
+    if (!Cmd->getEvent()->isCompleted())
+      return false;
+  }
+  return true;
+}
+
 void Scheduler::waitForRecordToFinish(MemObjRecord *Record,
                                       ReadLockT &GraphReadLock) {
 #ifdef XPTI_ENABLE_INSTRUMENTATION
@@ -271,21 +283,17 @@ void Scheduler::removeMemoryObject(detail::SYCLMemObjI *MemObj) {
   std::vector<std::shared_ptr<const void>> AuxResourcesToDeallocate;
 
   {
-    MemObjRecord *Record = nullptr;
+    MemObjRecord *Record = MGraphBuilder.getMemObjRecord(MemObj);
+    if (!Record)
+      // No operations were performed on the mem object
+      return;
 
     {
       // This only needs a shared mutex as it only involves enqueueing and
       // awaiting for events
       ReadLockT Lock(MGraphLock);
-
-      Record = MGraphBuilder.getMemObjRecord(MemObj);
-      if (!Record)
-        // No operations were performed on the mem object
-        return;
-
       waitForRecordToFinish(Record, Lock);
     }
-
     {
       WriteLockT Lock(MGraphLock, std::defer_lock);
       acquireWriteLock(Lock);
@@ -410,10 +418,31 @@ Scheduler::~Scheduler() {
           "not all resources were released. Please be sure that all kernels "
           "have synchronization points.\n\n");
   }
+  // Please be aware that releaseResources should be called before deletion of
+  // Scheduler. Otherwise there can be the case when objects Scheduler keeps as
+  // fields may need Scheduler for their release and they work with Scheduler
+  // via GlobalHandler::getScheduler that will create new Scheduler object.
+  // Still keep it here but it should do almost nothing if releaseResources
+  // called before.
+  releaseResources();
+}
+
+void Scheduler::releaseResources() {
   // There might be some commands scheduled for post enqueue cleanup that
   // haven't been freed because of the graph mutex being locked at the time,
   // clean them up now.
   cleanupCommands({});
+  DefaultHostQueue.reset();
+
+  // We need loop since sometimes we may need new objects to be added to
+  // deferred mem objects storage during cleanup. Known example is: we cleanup
+  // existing deferred mem objects under write lock, during this process we
+  // cleanup commands related to this record, command may have last reference to
+  // queue_impl, ~queue_impl is called and buffer for assert (which is created
+  // with size only so all confitions for deferred release are satisfied) is
+  // added to deferred mem obj storage. So we may end up with leak.
+  while (!isDeferredMemObjectsEmpty())
+    cleanupDeferredMemObjects(BlockingT::BLOCKING);
 }
 
 void Scheduler::acquireWriteLock(WriteLockT &Lock) {
@@ -442,8 +471,8 @@ MemObjRecord *Scheduler::getMemObjRecord(const Requirement *const Req) {
 }
 
 void Scheduler::cleanupCommands(const std::vector<Command *> &Cmds) {
-  if (Cmds.empty())
-  {
+  cleanupDeferredMemObjects(BlockingT::NON_BLOCKING);
+  if (Cmds.empty()) {
     std::lock_guard<std::mutex> Lock{MDeferredCleanupMutex};
     if (MDeferredCleanupCommands.empty())
       return;
@@ -472,6 +501,54 @@ void Scheduler::cleanupCommands(const std::vector<Command *> &Cmds) {
   }
 }
 
+void Scheduler::deferMemObjRelease(const std::shared_ptr<SYCLMemObjI> &MemObj) {
+  {
+    std::lock_guard<std::mutex> Lock{MDeferredMemReleaseMutex};
+    MDeferredMemObjRelease.push_back(MemObj);
+  }
+  cleanupDeferredMemObjects(BlockingT::NON_BLOCKING);
+}
+
+inline bool Scheduler::isDeferredMemObjectsEmpty() {
+  std::lock_guard<std::mutex> Lock{MDeferredMemReleaseMutex};
+  return MDeferredMemObjRelease.empty();
+}
+
+void Scheduler::cleanupDeferredMemObjects(BlockingT Blocking) {
+  if (isDeferredMemObjectsEmpty())
+    return;
+  if (Blocking == BlockingT::BLOCKING) {
+    std::vector<std::shared_ptr<SYCLMemObjI>> TempStorage;
+    {
+      std::lock_guard<std::mutex> LockDef{MDeferredMemReleaseMutex};
+      MDeferredMemObjRelease.swap(TempStorage);
+    }
+    // if any objects in TempStorage exist - it is leaving scope and being
+    // deleted
+  }
+
+  std::vector<std::shared_ptr<SYCLMemObjI>> ObjsReadyToRelease;
+  {
+
+    ReadLockT Lock = ReadLockT(MGraphLock, std::try_to_lock);
+    if (Lock.owns_lock()) {
+      // Not expected that Blocking == true will be used in parallel with
+      // adding MemObj to storage, no such scenario.
+      std::lock_guard<std::mutex> LockDef{MDeferredMemReleaseMutex};
+      auto MemObjIt = MDeferredMemObjRelease.begin();
+      while (MemObjIt != MDeferredMemObjRelease.end()) {
+        MemObjRecord *Record = MGraphBuilder.getMemObjRecord((*MemObjIt).get());
+        if (!checkLeavesCompletion(Record)) {
+          MemObjIt++;
+          continue;
+        }
+        ObjsReadyToRelease.push_back(*MemObjIt);
+        MemObjIt = MDeferredMemObjRelease.erase(MemObjIt);
+      }
+    }
+  }
+  // if any ObjsReadyToRelease found - it is leaving scope and being deleted
+}
 } // namespace detail
 } // __SYCL_INLINE_VER_NAMESPACE(_V1)
 } // namespace sycl

sycl/source/detail/scheduler/scheduler.hpp

@@ -442,9 +442,13 @@ class Scheduler {
   QueueImplPtr getDefaultHostQueue() { return DefaultHostQueue; }
 
   static MemObjRecord *getMemObjRecord(const Requirement *const Req);
+  // Virtual for testing purposes only
+  void deferMemObjRelease(const std::shared_ptr<detail::SYCLMemObjI> &MemObj);
 
   Scheduler();
   ~Scheduler();
+  void releaseResources();
+  inline bool isDeferredMemObjectsEmpty();
 
 protected:
   // TODO: after switching to C++17, change std::shared_timed_mutex to
@@ -464,6 +468,9 @@ class Scheduler {
   static void enqueueLeavesOfReqUnlocked(const Requirement *const Req,
                                          std::vector<Command *> &ToCleanUp);
 
+  // May lock graph with read and write modes during execution.
+  void cleanupDeferredMemObjects(BlockingT Blocking);
+
   /// Graph builder class.
   ///
   /// The graph builder provides means to change an existing graph (e.g. add
@@ -764,13 +771,17 @@ class Scheduler {
   /// GraphReadLock will be unlocked/locked as needed. Upon return from the
   /// function, GraphReadLock will be left in locked state.
   void waitForRecordToFinish(MemObjRecord *Record, ReadLockT &GraphReadLock);
+  bool checkLeavesCompletion(MemObjRecord *Record);
 
   GraphBuilder MGraphBuilder;
   RWLockT MGraphLock;
 
   std::vector<Command *> MDeferredCleanupCommands;
   std::mutex MDeferredCleanupMutex;
 
+  std::vector<std::shared_ptr<SYCLMemObjI>> MDeferredMemObjRelease;
+  std::mutex MDeferredMemReleaseMutex;
+
   QueueImplPtr DefaultHostQueue;
 
   friend class Command;

sycl/source/detail/sycl_mem_obj_t.cpp

@@ -31,7 +31,7 @@ SYCLMemObjT::SYCLMemObjT(pi_native_handle MemObject, const context &SyclContext,
       MInteropContext(detail::getSyclObjImpl(SyclContext)),
       MOpenCLInterop(true), MHostPtrReadOnly(false), MNeedWriteBack(true),
       MUserPtr(nullptr), MShadowCopy(nullptr), MUploadDataFunctor(nullptr),
-      MSharedPtrStorage(nullptr) {
+      MSharedPtrStorage(nullptr), MHostPtrProvided(true) {
   if (MInteropContext->is_host())
     throw sycl::invalid_parameter_error(
         "Creation of interoperability memory object using host context is "
@@ -147,6 +147,18 @@ void SYCLMemObjT::determineHostPtr(const ContextImplPtr &Context,
   } else
     HostPtrReadOnly = false;
 }
+
+void SYCLMemObjT::detachMemoryObject(const std::shared_ptr<SYCLMemObjT> &Self,
+                                     bool DefaultAllocator) const {
+  //  Check MRecord without read lock because at this point we expect that no
+  //  commands that operate on the buffer can be created. MRecord is nullptr on
+  //  buffer creation and set to meaningfull
+  // value only if any operation on buffer submitted inside addCG call. addCG is
+  // called from queue::submit and buffer destruction could not overlap with it.
+  if (MRecord && !MHostPtrProvided && DefaultAllocator)
+    Scheduler::getInstance().deferMemObjRelease(Self);
+}
+
 } // namespace detail
 } // __SYCL_INLINE_VER_NAMESPACE(_V1)
 } // namespace sycl