Do not track created non-resource heaps in residency cache by default. (#649)

bbernhar · web-flow · commit b58b417c2396 · 2022-09-21T18:29:38.000Z
diff --git a/src/gpgmm/d3d12/HeapD3D12.cpp b/src/gpgmm/d3d12/HeapD3D12.cpp
@@ -23,14 +23,35 @@
 
 namespace gpgmm::d3d12 {
 
+    namespace {
+
+        // Returns the resource heap flags or E_INVALIDARG, when the memory type doesn't allow
+        // resources.
+        HRESULT GetResourceHeapFlags(ComPtr<ID3D12Pageable> pageable, D3D12_HEAP_FLAGS* heapFlags) {
+            ComPtr<ID3D12Heap> heap;
+            if (SUCCEEDED(pageable.As(&heap))) {
+                *heapFlags = heap->GetDesc().Flags;
+                return S_OK;
+            }
+
+            ComPtr<ID3D12Resource> committedResource;
+            if (SUCCEEDED(pageable.As(&committedResource))) {
+                ReturnIfFailed(committedResource->GetHeapProperties(nullptr, heapFlags));
+                return S_OK;
+            }
+
+            return E_INVALIDARG;
+        }
+    }  // namespace
+
     // static
     HRESULT Heap::CreateHeap(const HEAP_DESC& descriptor,
                              ResidencyManager* const pResidencyManager,
                              CreateHeapFn&& createHeapFn,
                              Heap** ppHeapOut) {
         const bool isResidencyDisabled = (pResidencyManager == nullptr);
 
-        // Ensure enough budget exists before allocating to avoid an out-of-memory error.
+        // Ensure enough budget exists before creating the heap to avoid an out-of-memory error.
         if (!isResidencyDisabled && (descriptor.Flags & HEAP_FLAG_ALWAYS_IN_BUDGET)) {
             ReturnIfFailed(pResidencyManager->EnsureInBudget(descriptor.SizeInBytes,
                                                              descriptor.MemorySegmentGroup));
@@ -51,28 +72,33 @@ namespace gpgmm::d3d12 {
         std::unique_ptr<Heap> heap(new Heap(pageable, descriptor, isResidencyDisabled));
 
         if (!isResidencyDisabled) {
-            ReturnIfFailed(pResidencyManager->InsertHeap(heap.get()));
-
             // Check if the underlying memory was implicitly made resident.
-            // This is always the case for resource heaps unless the "not resident" flag was
-            // explicitly used in createHeapFn().
             D3D12_HEAP_FLAGS resourceHeapFlags = D3D12_HEAP_FLAG_NONE;
-
-            ComPtr<ID3D12Heap> d3d12Heap;
-            if (SUCCEEDED(pageable.As(&d3d12Heap))) {
-                resourceHeapFlags = d3d12Heap->GetDesc().Flags;
+            if (SUCCEEDED(GetResourceHeapFlags(pageable, &resourceHeapFlags))) {
+                // Resource heaps created without the "create not resident" flag are always
+                // resident.
+                if (!(resourceHeapFlags & D3D12_HEAP_FLAG_CREATE_NOT_RESIDENT)) {
+                    heap->mState = CURRENT_RESIDENT;
+                } else {
+                    heap->mState = PENDING_RESIDENCY;
+                }
             }
 
-            ComPtr<ID3D12Resource> committedResource;
-            if (SUCCEEDED(pageable.As(&committedResource))) {
-                ReturnIfFailed(committedResource->GetHeapProperties(nullptr, &resourceHeapFlags));
+            // Heap created not resident requires no budget to be created.
+            if (heap->mState == PENDING_RESIDENCY &&
+                (descriptor.Flags & HEAP_FLAG_ALWAYS_IN_BUDGET)) {
+                gpgmm::ErrorLog() << "Creating a heap always in budget cannot be used with "
+                                     "D3D12_HEAP_FLAG_CREATE_NOT_RESIDENT.";
+                return E_INVALIDARG;
             }
 
-            if ((d3d12Heap || committedResource) &&
-                !(resourceHeapFlags & D3D12_HEAP_FLAG_CREATE_NOT_RESIDENT)) {
-                heap->mState = CURRENT_RESIDENT;
-            } else {
-                heap->mState = PENDING_RESIDENCY;
+            // Only heap types that are known to be created resident are eligable for evicition and
+            // should be always inserted in the residency cache. For other heap types (eg.
+            // descriptor heap), they must be manually locked and unlocked to be inserted into the
+            // residency cache. This is to ensure MakeResident is always called on heaps which are
+            // not known (or guarenteed) to be created implicitly resident by D3D12.
+            if (heap->mState != RESIDENCY_UNKNOWN) {
+                ReturnIfFailed(pResidencyManager->InsertHeap(heap.get()));
             }
         }
 
diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp
@@ -289,15 +289,23 @@ namespace gpgmm::d3d12 {
             ReturnIfFailed(MakeResident(pHeap->GetMemorySegmentGroup(), pHeap->GetSize(), 1,
                                         pageable.GetAddressOf()));
             pHeap->SetResidencyState(CURRENT_RESIDENT);
+
+            // Untracked heaps, created not resident, are not already attributed toward residency
+            // usage because they are not in the residency cache.
+            mInfo.CurrentMemoryCount++;
+            mInfo.CurrentMemoryUsage += pHeap->GetSize();
         }
 
         // Since we can't evict the heap, it's unnecessary to track the heap in the LRU Cache.
         if (pHeap->IsInList()) {
             pHeap->RemoveFromList();
 
-            // Untracked heaps are not attributed toward residency usage.
-            mInfo.CurrentMemoryCount++;
-            mInfo.CurrentMemoryUsage += pHeap->GetSize();
+            // Untracked heaps, previously made resident, are not attributed toward residency usage
+            // because they will be removed from the residency cache.
+            if (pHeap->mState == CURRENT_RESIDENT) {
+                mInfo.CurrentMemoryCount++;
+                mInfo.CurrentMemoryUsage += pHeap->GetSize();
+            }
         }
 
         pHeap->AddResidencyLockRef();
diff --git a/src/tests/D3D12Test.cpp b/src/tests/D3D12Test.cpp
@@ -61,16 +61,6 @@ namespace gpgmm::d3d12 {
         ASSERT_SUCCEEDED(dxgiFactory4->EnumAdapterByLuid(adapterLUID, IID_PPV_ARGS(&mAdapter)));
         ASSERT_NE(mAdapter.Get(), nullptr);
 
-        D3D12_FEATURE_DATA_ARCHITECTURE arch = {};
-        ASSERT_SUCCEEDED(
-            mDevice->CheckFeatureSupport(D3D12_FEATURE_ARCHITECTURE, &arch, sizeof(arch)));
-        mIsUMA = arch.UMA;
-
-        D3D12_FEATURE_DATA_D3D12_OPTIONS options = {};
-        ASSERT_SUCCEEDED(
-            mDevice->CheckFeatureSupport(D3D12_FEATURE_D3D12_OPTIONS, &options, sizeof(options)));
-        mResourceHeapTier = options.ResourceHeapTier;
-
         DXGI_ADAPTER_DESC adapterDesc;
         ASSERT_SUCCEEDED(mAdapter->GetDesc(&adapterDesc));
 
@@ -86,23 +76,20 @@ namespace gpgmm::d3d12 {
         DebugLog() << "GPU memory: " << GPGMM_BYTES_TO_GB(adapterDesc.DedicatedVideoMemory)
                    << " GBs.";
 
-        DebugLog() << "Unified memory: " << ((arch.UMA) ? "yes" : "no")
-                   << ((arch.CacheCoherentUMA) ? " (cache-coherent)" : "");
+        Caps* capsPtr = nullptr;
+        ASSERT_SUCCEEDED(Caps::CreateCaps(mDevice.Get(), mAdapter.Get(), &capsPtr));
+        mCaps.reset(capsPtr);
 
-        std::unique_ptr<Caps> caps;
-        {
-            Caps* capsPtr = nullptr;
-            ASSERT_SUCCEEDED(Caps::CreateCaps(mDevice.Get(), mAdapter.Get(), &capsPtr));
-            caps.reset(capsPtr);
-        }
+        DebugLog() << "Unified memory: " << ((mCaps->IsAdapterUMA()) ? "yes" : "no")
+                   << ((mCaps->IsAdapterCacheCoherentUMA()) ? " (cache-coherent)" : "");
 
-        DebugLog() << "Max resource size: " << GPGMM_BYTES_TO_MB(caps->GetMaxResourceSize())
+        DebugLog() << "Max resource size: " << GPGMM_BYTES_TO_MB(mCaps->GetMaxResourceSize())
                    << " MBs";
-        DebugLog() << "Max resource heap tier: " << caps->GetMaxResourceHeapTierSupported();
+        DebugLog() << "Max resource heap tier: " << mCaps->GetMaxResourceHeapTierSupported();
         DebugLog() << "Max resource heap size: "
-                   << GPGMM_BYTES_TO_GB(caps->GetMaxResourceHeapSize()) << " GBs";
+                   << GPGMM_BYTES_TO_GB(mCaps->GetMaxResourceHeapSize()) << " GBs";
         DebugLog() << "Creation of non-resident heaps: "
-                   << ((caps->IsCreateHeapNotResidentSupported()) ? "Supported" : "Not supported");
+                   << ((mCaps->IsCreateHeapNotResidentSupported()) ? "Supported" : "Not supported");
     }
 
     void D3D12TestBase::TearDown() {
@@ -115,7 +102,7 @@ namespace gpgmm::d3d12 {
         // Required parameters.
         desc.Adapter = mAdapter;
         desc.Device = mDevice;
-        desc.ResourceHeapTier = mResourceHeapTier;
+        desc.ResourceHeapTier = mCaps->GetMaxResourceHeapTierSupported();
 
         desc.MinLogLevel = GetMessageSeverity(GetLogLevel());
 
diff --git a/src/tests/D3D12Test.h b/src/tests/D3D12Test.h
@@ -30,6 +30,7 @@
 namespace gpgmm::d3d12 {
 
     struct ALLOCATOR_DESC;
+    class Caps;
     class ResourceAllocator;
     class ResourceAllocation;
 
@@ -59,9 +60,7 @@ namespace gpgmm::d3d12 {
       protected:
         ComPtr<IDXGIAdapter3> mAdapter;
         ComPtr<ID3D12Device> mDevice;
-
-        bool mIsUMA = false;
-        D3D12_RESOURCE_HEAP_TIER mResourceHeapTier = D3D12_RESOURCE_HEAP_TIER_1;
+        std::unique_ptr<Caps> mCaps;
     };
 
 }  // namespace gpgmm::d3d12
diff --git a/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp b/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp
@@ -17,6 +17,7 @@
 
 #include "gpgmm/common/SizeClass.h"
 #include "gpgmm/common/TraceEventPhase.h"
+#include "gpgmm/d3d12/CapsD3D12.h"
 #include "gpgmm/d3d12/ErrorD3D12.h"
 #include "gpgmm/d3d12/UtilsD3D12.h"
 #include "gpgmm/utils/Log.h"
@@ -300,18 +301,19 @@ class D3D12EventTraceReplay : public D3D12TestBase, public CaptureReplayTestWith
                         ASSERT_FALSE(snapshot.empty());
 
                         if (GetLogLevel() <= gpgmm::LogSeverity::Warning &&
-                            mIsUMA != snapshot["IsUMA"].asBool() && iterationIndex == 0) {
+                            mCaps->IsAdapterUMA() != snapshot["IsUMA"].asBool() &&
+                            iterationIndex == 0) {
                             gpgmm::WarningLog()
                                 << "Capture device does not match playback device (IsUMA: " +
                                        std::to_string(snapshot["IsUMA"].asBool()) + " vs " +
-                                       std::to_string(mIsUMA) + ").";
+                                       std::to_string(mCaps->IsAdapterUMA()) + ").";
                             GPGMM_SKIP_TEST_IF(!envParams.IsIgnoreCapsMismatchEnabled);
                         }
 
                         RESIDENCY_DESC residencyDesc = {};
                         residencyDesc.Device = mDevice;
                         residencyDesc.Adapter = mAdapter;
-                        residencyDesc.IsUMA = mIsUMA;
+                        residencyDesc.IsUMA = mCaps->IsAdapterUMA();
                         residencyDesc.MaxPctOfVideoMemoryToBudget =
                             snapshot["MaxPctOfVideoMemoryToBudget"].asFloat();
                         residencyDesc.MaxBudgetInBytes = snapshot["MaxBudgetInBytes"].asUInt64();
@@ -494,8 +496,8 @@ class D3D12EventTraceReplay : public D3D12TestBase, public CaptureReplayTestWith
                         HEAP_DESC resourceHeapDesc = {};
                         resourceHeapDesc.SizeInBytes = args["Heap"]["SizeInBytes"].asUInt64();
                         resourceHeapDesc.Alignment = args["Heap"]["Alignment"].asUInt64();
-                        resourceHeapDesc.MemorySegmentGroup =
-                            GetMemorySegmentGroup(heapProperties.MemoryPoolPreference, mIsUMA);
+                        resourceHeapDesc.MemorySegmentGroup = GetMemorySegmentGroup(
+                            heapProperties.MemoryPoolPreference, mCaps->IsAdapterUMA());
 
                         ResidencyManager* residencyManager =
                             createdResidencyManagerToID[currentResidencyID].Get();
diff --git a/src/tests/end2end/D3D12ResidencyManagerTests.cpp b/src/tests/end2end/D3D12ResidencyManagerTests.cpp
diff --git a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp
