Use CommandAllocatorManager to create and track lifetimes of ID3D12CommandAllocators

src/backend/d3d12/CommandAllocatorManager.cpp

@@ -0,0 +1,66 @@
+// Copyright 2017 The NXT Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "CommandAllocatorManager.h"
+
+#include "D3D12Backend.h"
+
+#include "common/BitSetIterator.h"
+
+namespace backend {
+namespace d3d12 {
+
+    CommandAllocatorManager::CommandAllocatorManager(Device* device) : device(device), allocatorCount(0) {
+        freeAllocators.set();
+    }
+
+    ComPtr<ID3D12CommandAllocator> CommandAllocatorManager::ReserveCommandAllocator() {
+        // If there are no free allocators, get the oldest serial in flight and wait on it
+        if (freeAllocators.none()) {
+            const uint64_t firstSerial = inFlightCommandAllocators.FirstSerial();
+            device->WaitForSerial(firstSerial);
+            ResetCompletedAllocators(firstSerial);
+        }
+
+        ASSERT(freeAllocators.any());
+
+        // Get the index of the first free allocator from the bitset
+        unsigned int firstFreeIndex = *(IterateBitSet(freeAllocators).begin());
+
+        if (firstFreeIndex >= allocatorCount) {
+            ASSERT(firstFreeIndex == allocatorCount);
+            allocatorCount++;
+            ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&commandAllocators[firstFreeIndex])));
+        }
+
+        // Mark the command allocator as used
+        freeAllocators.reset(firstFreeIndex);
+
+        // Enqueue the command allocator. It will be scheduled for reset after the next ExecuteCommandLists
+        inFlightCommandAllocators.Enqueue({commandAllocators[firstFreeIndex], firstFreeIndex}, device->GetSerial());
+
+        return commandAllocators[firstFreeIndex];
+    }
+
+    void CommandAllocatorManager::ResetCompletedAllocators(uint64_t lastCompletedSerial) {
+        // Reset all command allocators that are no longer in flight
+        for (auto it : inFlightCommandAllocators.IterateUpTo(lastCompletedSerial)) {
+            ASSERT_SUCCESS(it.commandAllocator->Reset());
+            freeAllocators.set(it.index);
+        }
+        inFlightCommandAllocators.ClearUpTo(lastCompletedSerial);
+    }
+
+}
+}

src/backend/d3d12/CommandAllocatorManager.h

@@ -0,0 +1,58 @@
+// Copyright 2017 The NXT Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef BACKEND_D3D12_COMMANDALLOCATORMANAGER_H_
+#define BACKEND_D3D12_COMMANDALLOCATORMANAGER_H_
+
+#include "d3d12_platform.h"
+
+#include "common/SerialQueue.h"
+
+#include <bitset>
+
+namespace backend {
+namespace d3d12 {
+
+    class Device;
+
+    class CommandAllocatorManager {
+        public:
+            CommandAllocatorManager(Device* device);
+
+            // A CommandAllocator that is reserved must be used before the next Device::Tick where the next serial has completed on the GPU
+            // at this time, the CommandAllocator will be reset
+            ComPtr<ID3D12CommandAllocator> ReserveCommandAllocator();
+            void ResetCompletedAllocators(uint64_t lastCompletedSerial);
+
+        private:
+            Device* device;
+
+            // This must be at least 2 because the Device and Queue use separate command allocators
+            static constexpr unsigned int kMaxCommandAllocators = 32;
+            unsigned int allocatorCount;
+
+            struct IndexedCommandAllocator {
+                ComPtr<ID3D12CommandAllocator> commandAllocator;
+                unsigned int index;
+            };
+
+            ComPtr<ID3D12CommandAllocator> commandAllocators[kMaxCommandAllocators];
+            std::bitset<kMaxCommandAllocators> freeAllocators;
+            SerialQueue<IndexedCommandAllocator> inFlightCommandAllocators;
+    };
+
+}
+}
+
+#endif // BACKEND_D3D12_COMMANDALLOCATORMANAGER_H_

src/backend/d3d12/D3D12Backend.cpp

@@ -44,26 +44,56 @@ namespace d3d12 {
         backendDevice->SetNextRenderTargetDescriptor(renderTargetDescriptor);
     }
 
+    uint64_t GetSerial(const nxtDevice device) {
+        const Device* backendDevice = reinterpret_cast<const Device*>(device);
+        return backendDevice->GetSerial();
+    }
+
+    void NextSerial(nxtDevice device) {
+        Device* backendDevice = reinterpret_cast<Device*>(device);
+        backendDevice->NextSerial();
+    }
+
+    void ExecuteCommandLists(nxtDevice device, std::initializer_list<ID3D12CommandList*> commandLists) {
+        Device* backendDevice = reinterpret_cast<Device*>(device);
+        backendDevice->ExecuteCommandLists(commandLists);
+    }
+
+    void WaitForSerial(nxtDevice device, uint64_t serial) {
+        Device* backendDevice = reinterpret_cast<Device*>(device);
+        backendDevice->WaitForSerial(serial);
+    }
+
+    ComPtr<ID3D12CommandAllocator> ReserveCommandAllocator(nxtDevice device) {
+        Device* backendDevice = reinterpret_cast<Device*>(device);
+        return backendDevice->GetCommandAllocatorManager()->ReserveCommandAllocator();
+    }
+
     void ASSERT_SUCCESS(HRESULT hr) {
         ASSERT(SUCCEEDED(hr));
     }
 
-    Device::Device(ComPtr<ID3D12Device> d3d12Device) : d3d12Device(d3d12Device), resourceUploader(this) {
+    Device::Device(ComPtr<ID3D12Device> d3d12Device)
+        : d3d12Device(d3d12Device),
+          commandAllocatorManager(this),
+          resourceUploader(this),
+          pendingCommands{ commandAllocatorManager.ReserveCommandAllocator() } {
+
         D3D12_COMMAND_QUEUE_DESC queueDesc = {};
         queueDesc.Flags = D3D12_COMMAND_QUEUE_FLAG_NONE;
         queueDesc.Type = D3D12_COMMAND_LIST_TYPE_DIRECT;
         ASSERT_SUCCESS(d3d12Device->CreateCommandQueue(&queueDesc, IID_PPV_ARGS(&commandQueue)));
 
-        ASSERT_SUCCESS(d3d12Device->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&pendingCommandAllocator)));
         ASSERT_SUCCESS(d3d12Device->CreateCommandList(
             0,
             D3D12_COMMAND_LIST_TYPE_DIRECT,
-            pendingCommandAllocator.Get(),
+            pendingCommands.commandAllocator.Get(),
             nullptr,
-            IID_PPV_ARGS(&pendingCommandList)
+            IID_PPV_ARGS(&pendingCommands.commandList)
         ));
+        pendingCommands.open = true;
 
-        ASSERT_SUCCESS(d3d12Device->CreateFence(serial++, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)));
+        ASSERT_SUCCESS(d3d12Device->CreateFence(serial, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)));
         fenceEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
         ASSERT(fenceEvent != nullptr);
     }
@@ -79,12 +109,14 @@ namespace d3d12 {
         return commandQueue;
     }
 
-    ComPtr<ID3D12CommandAllocator> Device::GetPendingCommandAllocator() {
-        return pendingCommandAllocator;
-    }
-
     ComPtr<ID3D12GraphicsCommandList> Device::GetPendingCommandList() {
-        return pendingCommandList;
+        // Callers of GetPendingCommandList do so to record commands. Only reserve a command allocator when it is needed so we don't submit empty command lists
+        if (!pendingCommands.open) {
+            pendingCommands.commandAllocator = commandAllocatorManager.ReserveCommandAllocator();
+            ASSERT_SUCCESS(pendingCommands.commandList->Reset(pendingCommands.commandAllocator.Get(), nullptr));
+            pendingCommands.open = true;
+        }
+        return pendingCommands.commandList;
     }
 
     D3D12_CPU_DESCRIPTOR_HANDLE Device::GetCurrentRenderTargetDescriptor() {
@@ -95,44 +127,50 @@ namespace d3d12 {
         return &resourceUploader;
     }
 
+    CommandAllocatorManager* Device::GetCommandAllocatorManager() {
+        return &commandAllocatorManager;
+    }
+
     void Device::SetNextRenderTargetDescriptor(D3D12_CPU_DESCRIPTOR_HANDLE renderTargetDescriptor) {
         this->renderTargetDescriptor = renderTargetDescriptor;
     }
 
     void Device::TickImpl() {
-        // Execute any pending commands
-        ASSERT_SUCCESS(pendingCommandList->Close());
-        ID3D12CommandList* commandLists[] = { pendingCommandList.Get() };
-        commandQueue->ExecuteCommandLists(_countof(commandLists), commandLists);
-
-        IncrementSerial();
-
-        // Signal when the pending commands have finished
-        ASSERT_SUCCESS(commandQueue->Signal(fence.Get(), GetSerial()));
-
-        // Handle objects awaiting GPU execution
+        // Perform cleanup operations to free unused objects
         const uint64_t lastCompletedSerial = fence->GetCompletedValue();
         resourceUploader.FreeCompletedResources(lastCompletedSerial);
-
-        // TODO(enga@google.com): This will stall on the submit because
-        // the commands must finish exeuting before the ID3D12CommandAllocator is reset.
-        // This should be fixed / optimized by using multiple command allocators.
-        const uint64_t currentFence = GetSerial();
-        if (lastCompletedSerial < currentFence) {
-            ASSERT_SUCCESS(fence->SetEventOnCompletion(currentFence, fenceEvent));
-            WaitForSingleObject(fenceEvent, INFINITE);
-        }
-
-        ASSERT_SUCCESS(pendingCommandAllocator->Reset());
-        ASSERT_SUCCESS(pendingCommandList->Reset(pendingCommandAllocator.Get(), NULL));
+        commandAllocatorManager.ResetCompletedAllocators(lastCompletedSerial);
     }
 
     uint64_t Device::GetSerial() const {
         return serial;
     }
 
-    void Device::IncrementSerial() {
-        serial++;
+    void Device::NextSerial() {
+        ASSERT_SUCCESS(commandQueue->Signal(fence.Get(), serial++));
+    }
+
+    void Device::WaitForSerial(uint64_t serial) {
+        const uint64_t lastCompletedSerial = fence->GetCompletedValue();
+        if (lastCompletedSerial < serial) {
+            ASSERT_SUCCESS(fence->SetEventOnCompletion(serial, fenceEvent));
+            WaitForSingleObject(fenceEvent, INFINITE);
+        }
+    }
+
+    void Device::ExecuteCommandLists(std::initializer_list<ID3D12CommandList*> commandLists) {
+        // If there are pending commands, prepend them to ExecuteCommandLists
+        if (pendingCommands.open) {
+            std::vector<ID3D12CommandList*> lists(commandLists.size() + 1);
+            pendingCommands.commandList->Close();
+            pendingCommands.open = false;
+            lists[0] = pendingCommands.commandList.Get();
+            std::copy(commandLists.begin(), commandLists.end(), lists.begin() + 1);
+            commandQueue->ExecuteCommandLists(commandLists.size() + 1, lists.data());
+        } else {
+            std::vector<ID3D12CommandList*> lists(commandLists);
+            commandQueue->ExecuteCommandLists(commandLists.size(), lists.data());
+        }
     }
 
     BindGroupBase* Device::CreateBindGroup(BindGroupBuilder* builder) {

src/backend/d3d12/D3D12Backend.h

@@ -33,6 +33,7 @@
 
 #include "d3d12_platform.h"
 #include "ResourceUploader.h"
+#include "CommandAllocatorManager.h"
 
 namespace backend {
 namespace d3d12 {
@@ -109,32 +110,42 @@ namespace d3d12 {
 
             ComPtr<ID3D12Device> GetD3D12Device();
             ComPtr<ID3D12CommandQueue> GetCommandQueue();
-            ComPtr<ID3D12CommandAllocator> GetPendingCommandAllocator();
-            ComPtr<ID3D12GraphicsCommandList> GetPendingCommandList();
-            D3D12_CPU_DESCRIPTOR_HANDLE GetCurrentRenderTargetDescriptor();
+            CommandAllocatorManager* GetCommandAllocatorManager();
             ResourceUploader* GetResourceUploader();
 
+            ComPtr<ID3D12GraphicsCommandList> GetPendingCommandList();
+            
+            D3D12_CPU_DESCRIPTOR_HANDLE GetCurrentRenderTargetDescriptor();
             void SetNextRenderTargetDescriptor(D3D12_CPU_DESCRIPTOR_HANDLE renderTargetDescriptor);
 
             uint64_t GetSerial() const;
-            void IncrementSerial();
+            void NextSerial();
+            void WaitForSerial(uint64_t serial);
+
+            void ExecuteCommandLists(std::initializer_list<ID3D12CommandList*> commandLists);
 
             // NXT API
             void Reference();
             void Release();
 
         private:
-            ComPtr<ID3D12Device> d3d12Device;
-            ComPtr<ID3D12CommandQueue> commandQueue;
-            ComPtr<ID3D12CommandAllocator> pendingCommandAllocator;
-            ComPtr<ID3D12GraphicsCommandList> pendingCommandList;
-            D3D12_CPU_DESCRIPTOR_HANDLE renderTargetDescriptor;
-
-            ResourceUploader resourceUploader;
-
             uint64_t serial = 0;
             ComPtr<ID3D12Fence> fence;
             HANDLE fenceEvent;
+
+            ComPtr<ID3D12Device> d3d12Device;
+            ComPtr<ID3D12CommandQueue> commandQueue;
+
+            CommandAllocatorManager commandAllocatorManager;
+            ResourceUploader resourceUploader;
+            
+            struct PendingCommandList {
+                ComPtr<ID3D12CommandAllocator> commandAllocator;
+                ComPtr<ID3D12GraphicsCommandList> commandList;
+                bool open = false;
+            } pendingCommands;
+
+            D3D12_CPU_DESCRIPTOR_HANDLE renderTargetDescriptor;
     };
 
 

src/backend/d3d12/QueueD3D12.cpp

@@ -23,45 +23,32 @@ namespace d3d12 {
     Queue::Queue(Device* device, QueueBuilder* builder)
         : QueueBase(builder), device(device) {
 
-        ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&commandAllocator)));
+        // TODO(enga@google.com): We don't need this allocator, but it's needed for command list initialization. Is there a better way to do this?
+        // Is CommandList creation expensive or can it be done every Queue::Submit?
+        ComPtr<ID3D12CommandAllocator> temporaryCommandAllocator;
+        ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&temporaryCommandAllocator)));
         ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandList(
             0,
             D3D12_COMMAND_LIST_TYPE_DIRECT,
-            commandAllocator.Get(),
+            temporaryCommandAllocator.Get(),
             nullptr,
             IID_PPV_ARGS(&commandList)
         ));
         ASSERT_SUCCESS(commandList->Close());
-
-        ASSERT_SUCCESS(device->GetD3D12Device()->CreateFence(fenceValue, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)));
-        fenceEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
-        ASSERT(fenceEvent != nullptr);
     }
 
     void Queue::Submit(uint32_t numCommands, CommandBuffer* const * commands) {
         device->Tick();
 
-        // TODO(enga@google.com): This will stall on the previous submit because
-        // the commands must finish exeuting before the ID3D12CommandAllocator is reset.
-        // This should be fixed / optimized by using multiple command allocators.
-        const uint64_t currentFence = fenceValue++;
-        ASSERT_SUCCESS(device->GetCommandQueue()->Signal(fence.Get(), fenceValue));
-
-        if (fence->GetCompletedValue() < currentFence) {
-            ASSERT_SUCCESS(fence->SetEventOnCompletion(currentFence, fenceEvent));
-            WaitForSingleObject(fenceEvent, INFINITE);
-        }
-
-        ASSERT_SUCCESS(commandAllocator->Reset());
-        ASSERT_SUCCESS(commandList->Reset(commandAllocator.Get(), NULL));
-
+        ASSERT_SUCCESS(commandList->Reset(device->GetCommandAllocatorManager()->ReserveCommandAllocator().Get(), nullptr));
         for (uint32_t i = 0; i < numCommands; ++i) {
             commands[i]->FillCommands(commandList);
         }
         ASSERT_SUCCESS(commandList->Close());
 
-        ID3D12CommandList* commandLists[] = { commandList.Get() };
-        device->GetCommandQueue()->ExecuteCommandLists(_countof(commandLists), commandLists);
+        device->ExecuteCommandLists({ commandList.Get() });
+
+        device->NextSerial();
     }
 
 }

src/backend/d3d12/QueueD3D12.h

@@ -35,11 +35,7 @@ namespace d3d12 {
         private:
             Device* device;
 
-            ComPtr<ID3D12CommandAllocator> commandAllocator;
             ComPtr<ID3D12GraphicsCommandList> commandList;
-            ComPtr<ID3D12Fence> fence;
-            uint64_t fenceValue = 0;
-            HANDLE fenceEvent;
     };
 
 }

src/backend/d3d12/ResourceUploader.cpp

@@ -70,7 +70,7 @@ namespace d3d12 {
         uploadResource->Unmap(0, &writeRange);
         device->GetPendingCommandList()->CopyBufferRegion(resource.Get(), start, uploadResource.Get(), 0, count);
 
-        uploadingResources.Enqueue(std::move(uploadResource), device->GetSerial() + 1);
+        uploadingResources.Enqueue(std::move(uploadResource), device->GetSerial());
     }
 
     void ResourceUploader::FreeCompletedResources(const uint64_t lastCompletedSerial) {