From e6e4339f0bd2b5ad3087c5dfa9de1fb66dc03857 Mon Sep 17 00:00:00 2001
From: Jeff Bloomfield <jeffbloo@microsoft.com>
Date: Tue, 31 Mar 2020 20:30:22 +0000
Subject: [PATCH] Merged PR 4408731: Perf: Command lists should be preemptively
 reset in DML when flushing

Noticing this while analyzing perf of small models, e.g. emotionferplus.  Resetting command lists right after flushing rather than when they're used next ensures that the CPU work occurs when the GPU is busy.

Related work items: #25512194
---
 .../src/DmlCommandRecorder.h                    |  5 +++++
 .../src/ExecutionContext.cpp                    | 17 ++++++-----------
 .../DmlExecutionProvider/src/ExecutionContext.h |  3 ---
 .../DmlExecutionProvider/src/ICommandRecorder.h |  2 ++
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/DmlCommandRecorder.h b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/DmlCommandRecorder.h
index 273f172a21..9becf3ad81 100644
--- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/DmlCommandRecorder.h
+++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/DmlCommandRecorder.h
@@ -56,6 +56,11 @@ namespace Dml
         
         void SetAllocator(std::weak_ptr<BucketizedBufferAllocator> allocator);
 
+        bool HasUnsubmittedWork() override
+        {
+            return m_operationsRecordedInCurrentCommandList || !m_pendingCommandLists.empty();
+        }
+
     private:
 
         std::shared_ptr<CommandQueue> m_queue;
diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.cpp b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.cpp
index 6a776f734c..52cbc63fda 100644
--- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.cpp
+++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.cpp
@@ -132,14 +132,6 @@ namespace Dml
         m_dmlRecorder.GetCommandList().CopyTo(commandList);
     }
 
-    void ExecutionContext::Wait(ID3D12Fence* fence, uint64_t value)
-    {
-        assert(!m_closed);
-        Flush();
-        m_queue->Wait(fence, value);
-        ReleaseCompletedReferences();
-    }
-
     void ExecutionContext::SetCommandRecorder(ICommandRecorder* newRecorder)
     {
         assert(!m_closed);
@@ -162,7 +154,7 @@ namespace Dml
     {
         assert(!m_closed);
 
-        if (!m_currentRecorder)
+        if (!m_currentRecorder || !m_currentRecorder->HasUnsubmittedWork())
         {
             // Nothing to flush
             return;
@@ -171,8 +163,11 @@ namespace Dml
         m_currentRecorder->CloseAndExecute();
         ReleaseCompletedReferences();
 
-        // Just submitted our command list, so we have neither DML or D3D12 work recorded on any of our command lists.
+        // Pre-emptively set the DML command recorder.  It's the only command recorder right now,
+        // and doing this here causes work and allocations resetting the command list to occur at
+        // a point where it's going to be parallelized with GPU work.
         m_currentRecorder = nullptr;
+        SetCommandRecorder(&m_dmlRecorder);
     }
     
     void ExecutionContext::QueueReference(IUnknown* object) 
@@ -203,7 +198,7 @@ namespace Dml
 
         // If something has been recorded into a command list but not submitted yet, it means that the *next* fence
         // value is the one to signal completion.
-        const bool unflushedWorkExists = (m_currentRecorder != nullptr);
+        const bool unflushedWorkExists = (m_currentRecorder != nullptr) && m_currentRecorder->HasUnsubmittedWork();
         if (unflushedWorkExists)
         {
             ++event.fenceValue;
diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.h b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.h
index 523d9d5541..f70ee13572 100644
--- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.h
+++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ExecutionContext.h
@@ -68,9 +68,6 @@ namespace Dml
 
         void GetCommandListForRecording(ID3D12GraphicsCommandList** commandList);
 
-        // See ID3D12CommandQueue::Wait
-        void Wait(ID3D12Fence* fence, uint64_t value);
-
         // Forces all queued work to begin executing on the GPU. This method returns immediately and does not wait
         // for the submitted work to complete execution on the GPU.
         void Flush();
diff --git a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ICommandRecorder.h b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ICommandRecorder.h
index 28e49c8e6f..b0dad7e128 100644
--- a/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ICommandRecorder.h
+++ b/onnxruntime/core/providers/dml/DmlExecutionProvider/src/ICommandRecorder.h
@@ -15,6 +15,8 @@ namespace Dml
         // Forces all queued work to begin executing on the GPU. This method returns immediately and does not wait
         // for the submitted work to complete execution on the GPU.
         virtual void CloseAndExecute() = 0;
+
+        virtual bool HasUnsubmittedWork() = 0;
     };
 
 } // namespace Dml