[SYCL] lazily set context on default constructed events (#6296)

According to the spec, sycl::event() constructed via empty constructor should be initialized with the default context. But simply doing so in that constructor is too expensive - many of those events are created temporarily, never needing the context. So instead, we are adding the context lazily, when it might be needed. The performance impact of both approaches has been measured, the simple direct setting of the context on all empty constructed events is deleterious whereas the method here, setting it lazily, has no appreciable degradation on performance.

There are a couple places in the existing codebase where the lack of a context was used as a proxy indicating event state. This has had to change. Also, the setContextImpl routine was not only setting the context, but setting the atomic state to incomplete. This is no longer desirable. I've moved the setting of that state to its own call on the impl, and the appropriate calls now invoke it.

Author: cperkinsintel

sycl/source/detail/event_impl.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include <CL/sycl/context.hpp>
+#include <CL/sycl/device_selector.hpp>
 #include <detail/event_impl.hpp>
 #include <detail/event_info.hpp>
 #include <detail/plugin.hpp>
@@ -31,11 +32,26 @@ namespace detail {
 extern xpti::trace_event_data_t *GSYCLGraphEvent;
 #endif
 
-// Threat all devices that don't support interoperability as host devices to
-// avoid attempts to call method get on such events.
-bool event_impl::is_host() const { return MHostEvent || !MOpenCLInterop; }
+// If we do not yet have a context, use the default one.
+void event_impl::ensureContextInitialized() {
+  if (MIsContextInitialized)
+    return;
+
+  const device &SyclDevice = default_selector().select_device();
+  this->setContextImpl(
+      detail::queue_impl::getDefaultOrNew(detail::getSyclObjImpl(SyclDevice)));
+}
 
-cl_event event_impl::get() const {
+bool event_impl::is_host() {
+  // We'll need a context before we can answer is_host question.
+  // setting it may adjust the values of MHostEvent and MOpenCLInterop
+  ensureContextInitialized();
+  // Treat all devices that don't support interoperability as host devices to
+  // avoid attempts to call method get on such events.
+  return MHostEvent || !MOpenCLInterop;
+}
+
+cl_event event_impl::get() {
   if (!MOpenCLInterop) {
     throw invalid_object_error(
         "This instance of event doesn't support OpenCL interoperability.",
@@ -91,25 +107,32 @@ void event_impl::setComplete() {
 const RT::PiEvent &event_impl::getHandleRef() const { return MEvent; }
 RT::PiEvent &event_impl::getHandleRef() { return MEvent; }
 
-const ContextImplPtr &event_impl::getContextImpl() { return MContext; }
+const ContextImplPtr &event_impl::getContextImpl() {
+  ensureContextInitialized();
+  return MContext;
+}
+
+const plugin &event_impl::getPlugin() {
+  ensureContextInitialized();
+  return MContext->getPlugin();
+}
 
-const plugin &event_impl::getPlugin() const { return MContext->getPlugin(); }
+void event_impl::setStateIncomplete() { MState = HES_NotComplete; }
 
 void event_impl::setContextImpl(const ContextImplPtr &Context) {
   MHostEvent = Context->is_host();
   MOpenCLInterop = !MHostEvent;
   MContext = Context;
-
-  MState = HES_NotComplete;
+  MIsContextInitialized = true;
 }
 
 event_impl::event_impl(HostEventState State)
     : MIsInitialized(false), MIsFlushed(true), MState(State) {}
 
 event_impl::event_impl(RT::PiEvent Event, const context &SyclContext)
-    : MEvent(Event), MContext(detail::getSyclObjImpl(SyclContext)),
-      MOpenCLInterop(true), MHostEvent(false), MIsFlushed(true),
-      MState(HES_Complete) {
+    : MIsContextInitialized(true), MEvent(Event),
+      MContext(detail::getSyclObjImpl(SyclContext)), MOpenCLInterop(true),
+      MHostEvent(false), MIsFlushed(true), MState(HES_Complete) {
 
   if (MContext->is_host()) {
     throw cl::sycl::invalid_parameter_error(
@@ -133,6 +156,8 @@ event_impl::event_impl(RT::PiEvent Event, const context &SyclContext)
 event_impl::event_impl(const QueueImplPtr &Queue)
     : MQueue{Queue}, MIsProfilingEnabled{Queue->is_host() ||
                                          Queue->MIsProfilingEnabled} {
+  this->setContextImpl(Queue->getContextImplPtr());
+
   if (Queue->is_host()) {
     MState.store(HES_NotComplete);
 
@@ -262,7 +287,7 @@ void event_impl::checkProfilingPreconditions() const {
 
 template <>
 uint64_t
-event_impl::get_profiling_info<info::event_profiling::command_submit>() const {
+event_impl::get_profiling_info<info::event_profiling::command_submit>() {
   checkProfilingPreconditions();
   if (!MHostEvent) {
     if (MEvent)
@@ -279,7 +304,7 @@ event_impl::get_profiling_info<info::event_profiling::command_submit>() const {
 
 template <>
 uint64_t
-event_impl::get_profiling_info<info::event_profiling::command_start>() const {
+event_impl::get_profiling_info<info::event_profiling::command_start>() {
   checkProfilingPreconditions();
   if (!MHostEvent) {
     if (MEvent)
@@ -295,8 +320,7 @@ event_impl::get_profiling_info<info::event_profiling::command_start>() const {
 }
 
 template <>
-uint64_t
-event_impl::get_profiling_info<info::event_profiling::command_end>() const {
+uint64_t event_impl::get_profiling_info<info::event_profiling::command_end>() {
   checkProfilingPreconditions();
   if (!MHostEvent) {
     if (MEvent)
@@ -310,8 +334,7 @@ event_impl::get_profiling_info<info::event_profiling::command_end>() const {
   return MHostProfilingInfo->getEndTime();
 }
 
-template <>
-uint32_t event_impl::get_info<info::event::reference_count>() const {
+template <> uint32_t event_impl::get_info<info::event::reference_count>() {
   if (!MHostEvent && MEvent) {
     return get_event_info<info::event::reference_count>::get(
         this->getHandleRef(), this->getPlugin());
@@ -321,7 +344,7 @@ uint32_t event_impl::get_info<info::event::reference_count>() const {
 
 template <>
 info::event_command_status
-event_impl::get_info<info::event::command_execution_status>() const {
+event_impl::get_info<info::event::command_execution_status>() {
   if (MState == HES_Discarded)
     return info::event_command_status::ext_oneapi_unknown;
 
@@ -344,13 +367,9 @@ void HostProfilingInfo::start() { StartTime = getTimestamp(); }
 
 void HostProfilingInfo::end() { EndTime = getTimestamp(); }
 
-pi_native_handle event_impl::getNative() const {
-  if (!MContext) {
-    static context SyclContext;
-    MContext = getSyclObjImpl(SyclContext);
-    MHostEvent = MContext->is_host();
-    MOpenCLInterop = !MHostEvent;
-  }
+pi_native_handle event_impl::getNative() {
+  ensureContextInitialized();
+
   auto Plugin = getPlugin();
   if (!MIsInitialized) {
     MIsInitialized = true;

sycl/source/detail/event_impl.hpp

@@ -60,12 +60,12 @@ class event_impl {
   /// host device to avoid attempts to call method get on such events.
   //
   /// \return true if this event is a SYCL host event.
-  bool is_host() const;
+  bool is_host();
 
   /// Returns a valid OpenCL event interoperability handle.
   ///
   /// \return a valid instance of OpenCL cl_event.
-  cl_event get() const;
+  cl_event get();
 
   /// Waits for the event.
   ///
@@ -103,13 +103,13 @@ class event_impl {
   /// \return depends on template parameter.
   template <info::event_profiling param>
   typename info::param_traits<info::event_profiling, param>::return_type
-  get_profiling_info() const;
+  get_profiling_info();
 
   /// Queries this SYCL event for information.
   ///
   /// \return depends on the information being requested.
   template <info::event param>
-  typename info::param_traits<info::event, param>::return_type get_info() const;
+  typename info::param_traits<info::event, param>::return_type get_info();
 
   ~event_impl();
 
@@ -137,7 +137,7 @@ class event_impl {
 
   /// \return the Plugin associated with the context of this event.
   /// Should be called when this is not a Host Event.
-  const plugin &getPlugin() const;
+  const plugin &getPlugin();
 
   /// Associate event with the context.
   ///
@@ -147,6 +147,9 @@ class event_impl {
   /// @param Context is a shared pointer to an instance of valid context_impl.
   void setContextImpl(const ContextImplPtr &Context);
 
+  /// Clear the event state
+  void setStateIncomplete();
+
   /// Returns command that is associated with the event.
   ///
   /// Scheduler mutex must be locked in read mode when this is called.
@@ -169,7 +172,7 @@ class event_impl {
   /// Gets the native handle of the SYCL event.
   ///
   /// \return a native handle.
-  pi_native_handle getNative() const;
+  pi_native_handle getNative();
 
   /// Returns vector of event dependencies.
   ///
@@ -220,11 +223,15 @@ class event_impl {
   void instrumentationEpilog(void *TelementryEvent, const std::string &Name,
                              int32_t StreamID, uint64_t IId) const;
   void checkProfilingPreconditions() const;
-  mutable bool MIsInitialized = true;
-  mutable RT::PiEvent MEvent = nullptr;
-  mutable ContextImplPtr MContext;
-  mutable bool MOpenCLInterop = false;
-  mutable bool MHostEvent = true;
+  // Events constructed without a context will lazily use the default context
+  // when needed.
+  void ensureContextInitialized();
+  bool MIsInitialized = true;
+  bool MIsContextInitialized = false;
+  RT::PiEvent MEvent = nullptr;
+  ContextImplPtr MContext;
+  bool MOpenCLInterop = false;
+  bool MHostEvent = true;
   std::unique_ptr<HostProfilingInfo> MHostProfilingInfo;
   void *MCommand = nullptr;
   std::weak_ptr<queue_impl> MQueue;
@@ -251,6 +258,10 @@ class event_impl {
 
   std::mutex MMutex;
   std::condition_variable cv;
+
+  friend std::vector<RT::PiEvent>
+  getOrWaitEvents(std::vector<cl::sycl::event> DepEvents,
+                  std::shared_ptr<cl::sycl::detail::context_impl> Context);
 };
 
 } // namespace detail

sycl/source/detail/helpers.cpp

@@ -23,6 +23,12 @@ std::vector<RT::PiEvent> getOrWaitEvents(std::vector<cl::sycl::event> DepEvents,
   std::vector<RT::PiEvent> Events;
   for (auto SyclEvent : DepEvents) {
     auto SyclEventImplPtr = detail::getSyclObjImpl(SyclEvent);
+    // throwaway events created with default constructor will not have a context
+    // (which is set lazily) calling is_host(), getContextImpl() would set that
+    // context, which we wish to avoid as it is expensive.
+    if (SyclEventImplPtr->MIsContextInitialized == false) {
+      continue;
+    }
     if (SyclEventImplPtr->is_host() ||
         SyclEventImplPtr->getContextImpl() != Context) {
       SyclEventImplPtr->waitInternal();

sycl/source/detail/queue_impl.cpp

@@ -49,6 +49,7 @@ prepareUSMEvent(const std::shared_ptr<detail::queue_impl> &QueueImpl,
   auto EventImpl = std::make_shared<detail::event_impl>(QueueImpl);
   EventImpl->getHandleRef() = NativeEvent;
   EventImpl->setContextImpl(detail::getSyclObjImpl(QueueImpl->get_context()));
+  EventImpl->setStateIncomplete();
   return detail::createSyclObjFromImpl<event>(EventImpl);
 }
 

sycl/source/detail/scheduler/commands.cpp

@@ -399,6 +399,7 @@ Command::Command(CommandType Type, QueueImplPtr Queue)
   MSubmittedQueue = MQueue;
   MEvent->setCommand(this);
   MEvent->setContextImpl(MQueue->getContextImplPtr());
+  MEvent->setStateIncomplete();
   MEnqueueStatus = EnqueueResultT::SyclEnqueueReady;
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
@@ -1091,6 +1092,7 @@ pi_int32 ReleaseCommand::enqueueImp() {
 
     EventImplPtr UnmapEventImpl(new event_impl(Queue));
     UnmapEventImpl->setContextImpl(Queue->getContextImplPtr());
+    UnmapEventImpl->setStateIncomplete();
     RT::PiEvent &UnmapEvent = UnmapEventImpl->getHandleRef();
 
     void *Src = CurAllocaIsHost
@@ -1293,9 +1295,9 @@ MemCpyCommand::MemCpyCommand(Requirement SrcReq,
       MSrcQueue(SrcQueue), MSrcReq(std::move(SrcReq)),
       MSrcAllocaCmd(SrcAllocaCmd), MDstReq(std::move(DstReq)),
       MDstAllocaCmd(DstAllocaCmd) {
-  if (!MSrcQueue->is_host())
+  if (!MSrcQueue->is_host()) {
     MEvent->setContextImpl(MSrcQueue->getContextImplPtr());
-
+  }
   emitInstrumentationDataProxy();
 }
 
@@ -1475,8 +1477,9 @@ MemCpyCommandHost::MemCpyCommandHost(Requirement SrcReq,
     : Command(CommandType::COPY_MEMORY, std::move(DstQueue)),
       MSrcQueue(SrcQueue), MSrcReq(std::move(SrcReq)),
       MSrcAllocaCmd(SrcAllocaCmd), MDstReq(std::move(DstReq)), MDstPtr(DstPtr) {
-  if (!MSrcQueue->is_host())
+  if (!MSrcQueue->is_host()) {
     MEvent->setContextImpl(MSrcQueue->getContextImplPtr());
+  }
 
   emitInstrumentationDataProxy();
 }

sycl/source/handler.cpp

@@ -285,6 +285,7 @@ event handler::finalize() {
     } else {
       NewEvent = std::make_shared<detail::event_impl>(MQueue);
       NewEvent->setContextImpl(MQueue->getContextImplPtr());
+      NewEvent->setStateIncomplete();
       OutEvent = &NewEvent->getHandleRef();
 
       if (PI_SUCCESS != EnqueueKernel())