[SYCL] Release commands with no dependencies after they're enqueued (#2492)

Presently, commands that do not have memory dependencies are only released if .wait() is called. If it is not, several resources (event, queue, command, kernel) are held onto.
This fix deletes stores that command event in the USMEvents ( so it is 'owned') and then deletes the command itself. Added a lit test to verify resource release.

Signed-off-by: Chris Perkins <[email protected]>

Author: cperkinsintel

sycl/source/detail/queue_impl.cpp

@@ -60,7 +60,7 @@ event queue_impl::memset(const shared_ptr_class<detail::queue_impl> &Self,
     return event();
 
   event ResEvent = prepareUSMEvent(Self, NativeEvent);
-  addUSMEvent(ResEvent);
+  addSharedEvent(ResEvent);
   return ResEvent;
 }
 
@@ -74,7 +74,7 @@ event queue_impl::memcpy(const shared_ptr_class<detail::queue_impl> &Self,
     return event();
 
   event ResEvent = prepareUSMEvent(Self, NativeEvent);
-  addUSMEvent(ResEvent);
+  addSharedEvent(ResEvent);
   return ResEvent;
 }
 
@@ -92,19 +92,30 @@ event queue_impl::mem_advise(const shared_ptr_class<detail::queue_impl> &Self,
                                                    Advice, &NativeEvent);
 
   event ResEvent = prepareUSMEvent(Self, NativeEvent);
-  addUSMEvent(ResEvent);
+  addSharedEvent(ResEvent);
   return ResEvent;
 }
 
 void queue_impl::addEvent(const event &Event) {
-  std::weak_ptr<event_impl> EventWeakPtr{getSyclObjImpl(Event)};
-  std::lock_guard<mutex_class> Lock(MMutex);
-  MEvents.push_back(std::move(EventWeakPtr));
+  EventImplPtr Eimpl = getSyclObjImpl(Event);
+  Command *Cmd = (Command *)(Eimpl->getCommand());
+  if (!Cmd) {
+    // if there is no command on the event, we cannot track it with MEventsWeak
+    // as that will leave it with no owner. Track in MEventsShared
+    addSharedEvent(Event);
+  } else {
+    std::weak_ptr<event_impl> EventWeakPtr{Eimpl};
+    std::lock_guard<mutex_class> Lock{MMutex};
+    MEventsWeak.push_back(std::move(EventWeakPtr));
+  }
 }
 
-void queue_impl::addUSMEvent(const event &Event) {
+/// addSharedEvent - queue_impl tracks events with weak pointers
+/// but some events have no other owner. In this case,
+/// addSharedEvent will have the queue track the events via a shared pointer.
+void queue_impl::addSharedEvent(const event &Event) {
   std::lock_guard<mutex_class> Lock(MMutex);
-  MUSMEvents.push_back(Event);
+  MEventsShared.push_back(Event);
 }
 
 void *queue_impl::instrumentationProlog(const detail::code_location &CodeLoc,
@@ -204,8 +215,8 @@ void queue_impl::wait(const detail::code_location &CodeLoc) {
   vector_class<event> USMEvents;
   {
     std::lock_guard<mutex_class> Lock(MMutex);
-    Events = std::move(MEvents);
-    USMEvents = std::move(MUSMEvents);
+    Events = std::move(MEventsWeak);
+    USMEvents = std::move(MEventsShared);
   }
 
   for (std::weak_ptr<event_impl> &EventImplWeakPtr : Events)

sycl/source/detail/queue_impl.hpp

@@ -400,10 +400,12 @@ class queue_impl {
 
   void initHostTaskAndEventCallbackThreadPool();
 
-  /// Stores a USM operation event that should be associated with the queue
+  /// queue_impl.addEvent tracks events with weak pointers
+  /// but some events have no other owners. addSharedEvent()
+  /// follows events with a shared pointer.
   ///
   /// \param Event is the event to be stored
-  void addUSMEvent(const event &Event);
+  void addSharedEvent(const event &Event);
 
   /// Stores an event that should be associated with the queue
   ///
@@ -415,10 +417,14 @@ class queue_impl {
 
   DeviceImplPtr MDevice;
   const ContextImplPtr MContext;
-  vector_class<std::weak_ptr<event_impl>> MEvents;
-  // USM operations are not added to the scheduler command graph,
-  // queue is the only owner on the runtime side.
-  vector_class<event> MUSMEvents;
+
+  /// These events are tracked, but not owned, by the queue.
+  vector_class<std::weak_ptr<event_impl>> MEventsWeak;
+
+  /// Events without data dependencies (such as USM) need an owner,
+  /// additionally, USM operations are not added to the scheduler command graph,
+  /// queue is the only owner on the runtime side.
+  vector_class<event> MEventsShared;
   exception_list MExceptions;
   const async_handler MAsyncHandler;
   const property_list MPropList;

sycl/source/detail/scheduler/scheduler.cpp

@@ -101,6 +101,12 @@ EventImplPtr Scheduler::addCG(std::unique_ptr<detail::CG> CommandGroup,
 
       if (IsKernel)
         Streams = ((ExecCGCommand *)NewCmd)->getStreams();
+
+      if (NewCmd->MDeps.size() == 0 && NewCmd->MUsers.size() == 0) {
+        NewEvent->setCommand(nullptr); // if there are no memory dependencies,
+                                       // decouple and free the command
+        delete NewCmd;
+      }
     }
   }
 

sycl/test/basic_tests/queue/release.cpp

@@ -0,0 +1,19 @@
+// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple  %s -o %t.out
+// RUN: env SYCL_PI_TRACE=2 %GPU_RUN_PLACEHOLDER %t.out | FileCheck %s
+
+#include <CL/sycl.hpp>
+int main() {
+  sycl::queue q;
+
+  q.single_task<class test>([]() {});
+  // no wait. Ensure resources are released anyway.
+
+  return 0;
+}
+
+//CHECK: ---> piEnqueueKernelLaunch(
+//CHECK: ---> piQueueRelease(
+//CHECK: ---> piEventRelease(
+//CHECK: ---> piContextRelease(
+//CHECK: ---> piKernelRelease(
+//CHECK: ---> piProgramRelease(