Residency Bug: Make Setting Heap's Last Submission Serial Optional

When attempting to allocate more than Dawn's budget within a single
serial, all heaps in the LRU will be un-evictable because the last
submission serial is the same as the current serial. We can work
around this by instead using the LockHeap and UnlockHeap functions
instead of EnsureCanMakeResident when calling CreatePlacedResource.
Also added in some additional comments regarding the last submission serial.

Bug: dawn:193
Change-Id: Ie4ec7ed5350b0858ea817431fbf77df6ca8acd96
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/18622
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>

src/dawn_native/d3d12/BufferD3D12.cpp

@@ -248,7 +248,7 @@ namespace dawn_native { namespace d3d12 {
         // The mapped buffer can be accessed at any time, so it must be locked to ensure it is never
         // evicted. This buffer should already have been made resident when it was created.
         Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap());
-        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockMappableHeap(heap));
+        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockHeap(heap));
 
         mWrittenMappedRange = {0, static_cast<size_t>(GetSize())};
         DAWN_TRY(CheckHRESULT(GetD3D12Resource()->Map(0, &mWrittenMappedRange,
@@ -261,7 +261,7 @@ namespace dawn_native { namespace d3d12 {
         // The mapped buffer can be accessed at any time, so we must make the buffer resident and
         // lock it to ensure it is never evicted.
         Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap());
-        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockMappableHeap(heap));
+        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockHeap(heap));
 
         mWrittenMappedRange = {};
         D3D12_RANGE readRange = {0, static_cast<size_t>(GetSize())};
@@ -280,7 +280,7 @@ namespace dawn_native { namespace d3d12 {
         // The mapped buffer can be accessed at any time, so we must make the buffer resident and
         // lock it to ensure it is never evicted.
         Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap());
-        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockMappableHeap(heap));
+        DAWN_TRY(ToBackend(GetDevice())->GetResidencyManager()->LockHeap(heap));
 
         mWrittenMappedRange = {0, static_cast<size_t>(GetSize())};
         char* data = nullptr;
@@ -299,7 +299,7 @@ namespace dawn_native { namespace d3d12 {
         // When buffers are mapped, they are locked to keep them in resident memory. We must unlock
         // them when they are unmapped.
         Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap());
-        ToBackend(GetDevice())->GetResidencyManager()->UnlockMappableHeap(heap);
+        ToBackend(GetDevice())->GetResidencyManager()->UnlockHeap(heap);
         mWrittenMappedRange = {};
     }
 
@@ -308,7 +308,7 @@ namespace dawn_native { namespace d3d12 {
         // reference on its heap.
         if (IsMapped()) {
             Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap());
-            ToBackend(GetDevice())->GetResidencyManager()->UnlockMappableHeap(heap);
+            ToBackend(GetDevice())->GetResidencyManager()->UnlockHeap(heap);
         }
 
         ToBackend(GetDevice())->DeallocateMemory(mResourceAllocation);

src/dawn_native/d3d12/HeapD3D12.cpp

@@ -71,12 +71,10 @@ namespace dawn_native { namespace d3d12 {
     }
 
     void Heap::IncrementResidencyLock() {
-        ASSERT(mD3d12HeapType != D3D12_HEAP_TYPE_DEFAULT);
         mResidencyLockRefCount++;
     }
 
     void Heap::DecrementResidencyLock() {
-        ASSERT(mD3d12HeapType != D3D12_HEAP_TYPE_DEFAULT);
         mResidencyLockRefCount--;
     }
 

src/dawn_native/d3d12/HeapD3D12.h

@@ -64,7 +64,12 @@ namespace dawn_native { namespace d3d12 {
         D3D12_HEAP_TYPE mD3d12HeapType;
         // mLastUsage denotes the last time this heap was recorded for use.
         Serial mLastUsage = 0;
-        // mLastSubmission denotes the last time this heap was submitted to the GPU.
+        // mLastSubmission denotes the last time this heap was submitted to the GPU. Note that
+        // although this variable often contains the same value as mLastUsage, it can differ in some
+        // situations. When some asynchronous APIs (like SetSubData) are called, mLastUsage is
+        // updated upon the call, but the backend operation is deferred until the next submission
+        // to the GPU. This makes mLastSubmission unique from mLastUsage, and allows us to
+        // accurately identify when heaps are evictable.
         Serial mLastSubmission = 0;
         uint32_t mResidencyLockRefCount = 0;
         uint64_t mSize = 0;

src/dawn_native/d3d12/ResidencyManagerD3D12.cpp

@@ -32,7 +32,7 @@ namespace dawn_native { namespace d3d12 {
     }
 
     // Increments number of locks on a heap to ensure the heap remains resident.
-    MaybeError ResidencyManager::LockMappableHeap(Heap* heap) {
+    MaybeError ResidencyManager::LockHeap(Heap* heap) {
         if (!mResidencyManagementEnabled) {
             return {};
         }
@@ -46,9 +46,9 @@ namespace dawn_native { namespace d3d12 {
         if (!heap->IsInResidencyLRUCache() && !heap->IsResidencyLocked()) {
             DAWN_TRY(EnsureCanMakeResident(heap->GetSize()));
             ID3D12Pageable* pageable = heap->GetD3D12Pageable().Get();
-            DAWN_TRY(
-                CheckHRESULT(mDevice->GetD3D12Device()->MakeResident(1, &pageable),
-                             "Making a heap resident due to an underlying resource being mapped."));
+            DAWN_TRY(CheckHRESULT(mDevice->GetD3D12Device()->MakeResident(1, &pageable),
+                                  "Making a scheduled-to-be-used resource resident in "
+                                  "device local memory"));
         }
 
         // Since we can't evict the heap, it's unnecessary to track the heap in the LRU Cache.
@@ -63,7 +63,7 @@ namespace dawn_native { namespace d3d12 {
 
     // Decrements number of locks on a heap. When the number of locks becomes zero, the heap is
     // inserted into the LRU cache and becomes eligible for eviction.
-    void ResidencyManager::UnlockMappableHeap(Heap* heap) {
+    void ResidencyManager::UnlockHeap(Heap* heap) {
         if (!mResidencyManagementEnabled) {
             return;
         }
@@ -240,8 +240,13 @@ namespace dawn_native { namespace d3d12 {
                 sizeToMakeResident += heap->GetSize();
             }
 
-            mLRUCache.Append(heap);
+            // If we submit a command list to the GPU, we must ensure that heaps referenced by that
+            // command list stay resident at least until that command list has finished execution.
+            // Setting this serial unnecessarily can leave the LRU in a state where nothing is
+            // eligible for eviction, even though some evictions may be possible.
             heap->SetLastSubmission(pendingCommandSerial);
+
+            mLRUCache.Append(heap);
         }
 
         if (heapsToMakeResident.size() != 0) {

src/dawn_native/d3d12/ResidencyManagerD3D12.h

@@ -29,8 +29,8 @@ namespace dawn_native { namespace d3d12 {
       public:
         ResidencyManager(Device* device);
 
-        MaybeError LockMappableHeap(Heap* heap);
-        void UnlockMappableHeap(Heap* heap);
+        MaybeError LockHeap(Heap* heap);
+        void UnlockHeap(Heap* heap);
         MaybeError EnsureCanMakeResident(uint64_t allocationSize);
         MaybeError EnsureHeapsAreResident(Heap** heaps, size_t heapCount);
 

src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp

@@ -260,7 +260,7 @@ namespace dawn_native { namespace d3d12 {
 
         // Before calling CreatePlacedResource, we must ensure the target heap is resident.
         // CreatePlacedResource will fail if it is not.
-        DAWN_TRY(mDevice->GetResidencyManager()->EnsureHeapsAreResident(&heap, 1));
+        DAWN_TRY(mDevice->GetResidencyManager()->LockHeap(heap));
 
         // With placed resources, a single heap can be reused.
         // The resource placed at an offset is only reclaimed
@@ -276,6 +276,10 @@ namespace dawn_native { namespace d3d12 {
                 initialUsage, nullptr, IID_PPV_ARGS(&placedResource)),
             "ID3D12Device::CreatePlacedResource"));
 
+        // After CreatePlacedResource has finished, the heap can be unlocked from residency. This
+        // will insert it into the residency LRU.
+        mDevice->GetResidencyManager()->UnlockHeap(heap);
+
         return ResourceHeapAllocation{allocation.GetInfo(), allocation.GetOffset(),
                                       std::move(placedResource), heap};
     }

src/dawn_native/d3d12/StagingBufferD3D12.cpp

@@ -44,8 +44,8 @@ namespace dawn_native { namespace d3d12 {
 
         // The mapped buffer can be accessed at any time, so it must be locked to ensure it is never
         // evicted. This buffer should already have been made resident when it was created.
-        DAWN_TRY(mDevice->GetResidencyManager()->LockMappableHeap(
-            ToBackend(mUploadHeap.GetResourceHeap())));
+        DAWN_TRY(
+            mDevice->GetResidencyManager()->LockHeap(ToBackend(mUploadHeap.GetResourceHeap())));
 
         return CheckHRESULT(GetResource()->Map(0, nullptr, &mMappedPointer), "ID3D12Resource::Map");
     }
@@ -59,8 +59,7 @@ namespace dawn_native { namespace d3d12 {
 
         // The underlying heap was locked in residency upon creation. We must unlock it when this
         // buffer becomes unmapped.
-        mDevice->GetResidencyManager()->UnlockMappableHeap(
-            ToBackend(mUploadHeap.GetResourceHeap()));
+        mDevice->GetResidencyManager()->UnlockHeap(ToBackend(mUploadHeap.GetResourceHeap()));
 
         // Invalidate the CPU virtual address & flush cache (if needed).
         GetResource()->Unmap(0, nullptr);