From 080dcfe857b7a7502e6ce99ec4b92ed36660c3ff Mon Sep 17 00:00:00 2001
From: Laurent Morichetti <laurent.morichetti@amd.com>
Date: Mon, 24 Aug 2020 17:52:34 -0700
Subject: [PATCH] Improve queueLock and lastCmdLock

Reduce the size of the queueLock and lastCmdLock critical sections
to improve lock contention performance. The smaller the critical
sections are the better.

lasCmdLock is still needed to guarantee that getLastEnqueueCommand_
can retain the command before it is swapped out and released.

Change-Id: Id35d4a77c035b2da0de4c15568b153d49e958bb7
---
 rocclr/platform/commandqueue.cpp | 21 ++++++++++++++++-----
 rocclr/platform/commandqueue.hpp |  3 ---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/rocclr/platform/commandqueue.cpp b/rocclr/platform/commandqueue.cpp
index e860297c25..454015486a 100644
--- a/rocclr/platform/commandqueue.cpp
+++ b/rocclr/platform/commandqueue.cpp
@@ -185,17 +185,26 @@ void HostQueue::append(Command& command) {
   }
   command.retain();
   command.setStatus(CL_QUEUED);
-  ScopedLock l(lastCmdLock_);
   queue_.enqueue(&command);
   if (!IS_HIP) {
     return;
   }
   // Set last submitted command
-  if (lastEnqueueCommand_ != nullptr) {
-    lastEnqueueCommand_->release();
+  Command* prevLastEnqueueCommand;
+  command.retain();
+  {
+    // lastCmdLock_ ensures that lastEnqueueCommand() can retain the command before it is swapped
+    // out. We want to keep this critical section as short as possible, so the command should be
+    // released outside this section.
+    ScopedLock l(lastCmdLock_);
+
+    prevLastEnqueueCommand = lastEnqueueCommand_;
+    lastEnqueueCommand_ = &command;
+  }
+
+  if (prevLastEnqueueCommand != nullptr) {
+    prevLastEnqueueCommand->release();
   }
-  lastEnqueueCommand_ = &command;
-  lastEnqueueCommand_->retain();
 }
 
 bool HostQueue::isEmpty() {
@@ -207,6 +216,8 @@ Command* HostQueue::getLastQueuedCommand(bool retain) {
   // Get last submitted command
   ScopedLock l(lastCmdLock_);
 
+  // Since the lastCmdLock_ is acquired, it is safe to read and retain the lastEnqueueCommand. It is
+  // guaranteed that the pointer will not change.
   if (retain && lastEnqueueCommand_ != nullptr) {
     lastEnqueueCommand_->retain();
   }
diff --git a/rocclr/platform/commandqueue.hpp b/rocclr/platform/commandqueue.hpp
index 0dacb9a1f8..11dc7115b6 100644
--- a/rocclr/platform/commandqueue.hpp
+++ b/rocclr/platform/commandqueue.hpp
@@ -234,9 +234,6 @@ class HostQueue : public CommandQueue {
 
   //! Get last enqueued command
   Command* getLastQueuedCommand(bool retain);
-
-  //! Set last enqueued command
-  void setLastQueuedCommand(Command* lastCommand) {}
 };