GPUUploadManager: prevent render-thread deadlock at max page count without allowing worker-thread over-allocation

Author: TheMostDiligent

Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp

@@ -337,8 +337,8 @@ class GPUUploadManagerImpl final : public ObjectBase<IGPUUploadManager>
                      Uint32                InitialPageCount,
                      TEXTURE_FORMAT        Format = TEX_FORMAT_UNKNOWN) noexcept;
 
-        Page* CreatePage(IDeviceContext* pContext, Uint32 RequiredSize = 0);
-        Page* AcquireFreePage(IDeviceContext* pContext, Uint32 RequiredSize = 0);
+        Page* CreatePage(IDeviceContext* pContext, Uint32 RequiredSize = 0, bool AllowOverLimit = false);
+        Page* AcquireFreePage(IDeviceContext* pContext, Uint32 RequiredSize = 0, bool AllowOverLimit = false);
         bool  SealAndSwapCurrentPage(IDeviceContext* pContext);
         bool  TryRotatePage(IDeviceContext* pContext, Page* ExpectedCurrent, Uint32 RequiredSize);
         bool  TryEnqueuePage(Page* P);

Graphics/GraphicsTools/interface/GPUUploadManager.h

@@ -52,7 +52,7 @@ struct GPUUploadManagerCreateInfo
 
     /// Initial number of upload pages. If the manager runs out of pages to write to,
     /// it will create new ones as needed. This parameter controls how many pages are created at startup.
-    Uint32 InitialPageCount DEFAULT_INITIALIZER(1);
+    Uint32 InitialPageCount DEFAULT_INITIALIZER(4);
 
     /// Initial number of large upload pages.
     Uint32 InitialLargePageCount DEFAULT_INITIALIZER(1);

Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp

@@ -1145,11 +1145,9 @@ void GPUUploadManagerImpl::ScheduleTextureUpdate(const ScheduleTextureUpdateInfo
                             });
 }
 
-GPUUploadManagerImpl::Page* GPUUploadManagerImpl::UploadStream::CreatePage(IDeviceContext* pContext, Uint32 RequiredSize)
+GPUUploadManagerImpl::Page* GPUUploadManagerImpl::UploadStream::CreatePage(IDeviceContext* pContext, Uint32 RequiredSize, bool AllowOverLimit)
 {
-    // Always create a new page from the render thread (when pContext is not null) to avoid deadlock in
-    // UploadStream::ScheduleUpdate.
-    if (pContext == nullptr)
+    if (!AllowOverLimit)
     {
         const Uint32 MaxExistingPageSize = m_PageSizeToCount.empty() ? 0 : m_PageSizeToCount.rbegin()->first;
         if (m_MaxPageCount != 0 && m_Pages.size() >= m_MaxPageCount && RequiredSize <= MaxExistingPageSize)
@@ -1201,8 +1199,11 @@ bool GPUUploadManagerImpl::UploadStream::SealAndSwapCurrentPage(IDeviceContext*
 
 bool GPUUploadManagerImpl::UploadStream::TryRotatePage(IDeviceContext* pContext, Page* ExpectedCurrent, Uint32 RequiredSize)
 {
-    // Grab a free page (workers can't create, so pContext=null)
-    Page* Fresh = AcquireFreePage(pContext, RequiredSize);
+    // Allow going over the max page count when rotating the page from the render thread to
+    // prevent deadlock in ScheduleUpdate.
+    bool AllowOverLimit = pContext != nullptr;
+    // Grab a free page.
+    Page* Fresh = AcquireFreePage(pContext, RequiredSize, AllowOverLimit);
     if (!Fresh)
         return false;
 
@@ -1375,15 +1376,17 @@ void GPUUploadManagerImpl::ProcessPendingPages(IDeviceContext* pContext)
     }
 }
 
-GPUUploadManagerImpl::Page* GPUUploadManagerImpl::UploadStream::AcquireFreePage(IDeviceContext* pContext, Uint32 RequiredSize)
+GPUUploadManagerImpl::Page* GPUUploadManagerImpl::UploadStream::AcquireFreePage(IDeviceContext* pContext,
+                                                                                Uint32          RequiredSize,
+                                                                                bool            AllowOverLimit)
 {
     // For texture pages, all sizes are texture dimensions.
     Uint32 MaxPendingUpdateSize = std::max(m_MaxPendingUpdateSize.load(std::memory_order_acquire), RequiredSize);
 
     Page* P = m_FreePages.Pop(MaxPendingUpdateSize);
     if (P == nullptr && pContext != nullptr)
     {
-        P = CreatePage(pContext, MaxPendingUpdateSize);
+        P = CreatePage(pContext, MaxPendingUpdateSize, AllowOverLimit);
     }
 
     if (P != nullptr)

Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp

@@ -1293,6 +1293,8 @@ TEST(GPUUploadManagerTest, ParallelBufferAndTextureUpdates)
 
     LOG_INFO_MESSAGE("Total render thread updates: ", NumRenderThreadUpdates);
 
+    pUploadManager->RenderThreadUpdate(pContext);
+    pContext->WaitForIdle();
     pUploadManager->RenderThreadUpdate(pContext);
 
     for (std::thread& thread : Threads)
@@ -1306,6 +1308,12 @@ TEST(GPUUploadManagerTest, ParallelBufferAndTextureUpdates)
         VerifyTextureContents(Data.pTexture, Data.SubresData);
     }
 
+    GPUUploadManagerStats Stats;
+    pUploadManager->GetStats(Stats);
+    ASSERT_EQ(Stats.NumStreams, 2u);
+    EXPECT_EQ(Stats.pStreamStats[0].NumPages, CreateInfo.MaxPageCount) << "Normal page count should not exceed the specified maximum";
+    EXPECT_EQ(Stats.pStreamStats[1].NumPages, CreateInfo.MaxLargePageCount) << "Large page count should not exceed the specified maximum";
+
     LogUploadManagerStats(pUploadManager);
 }