Skip to content

Commit cf28074

Browse files
TheMostDiligentTheMostDiligent
committed
GPUUploadManagerTest: add test for destroying while texture updates are running
1 parent 65d1173 commit cf28074

File tree

3 files changed

+148
-37
lines changed

3 files changed

+148
-37
lines changed

‎Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ class GPUUploadManagerImpl final : public ObjectBase<IGPUUploadManager>
153153
// Returns true if the page was not previously enqueued, false otherwise.
154154
bool TryEnqueue();
155155

156-
UploadStream* GetStream() const { return m_pStream; }
156+
// Return the page to the stream's pool of free pages.
157+
void Recycle();
157158

158159
Uint64 GetFenceValue() const { return m_FenceValue; }
159160
Uint32 GetSize() const { return m_Size; }

‎Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -617,11 +617,11 @@ void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext)
617617

618618
if (pContext != nullptr && m_pData == nullptr)
619619
{
620-
const MAP_FLAGS MapFlags = m_PersistentMapped ?
621-
MAP_FLAG_DO_NOT_WAIT :
622-
MAP_FLAG_NONE;
623620
if (m_pStagingBuffer)
624621
{
622+
const MAP_FLAGS MapFlags = m_PersistentMapped ?
623+
MAP_FLAG_DO_NOT_WAIT :
624+
MAP_FLAG_NONE;
625625
pContext->MapBuffer(m_pStagingBuffer, MAP_WRITE, MapFlags, m_pData);
626626
}
627627
else if (m_pStagingAtlas)
@@ -642,6 +642,11 @@ bool GPUUploadManagerImpl::Page::TryEnqueue()
642642
return m_Enqueued.compare_exchange_strong(Expected, true, std::memory_order_acq_rel);
643643
}
644644

645+
void GPUUploadManagerImpl::Page::Recycle()
646+
{
647+
m_pStream->AddFreePage(this);
648+
}
649+
645650
void GPUUploadManagerImpl::Page::ReleaseStagingBuffer(IDeviceContext* pContext)
646651
{
647652
if (m_pData != nullptr)
@@ -1113,9 +1118,6 @@ void GPUUploadManagerImpl::ScheduleTextureUpdate(const ScheduleTextureUpdateInfo
11131118
0,
11141119
};
11151120

1116-
if (m_Stopping.load(std::memory_order_acquire))
1117-
return;
1118-
11191121
UploadStream* pStream = m_pTextureStreams ?
11201122
m_pTextureStreams->GetStreamForFormat(UpdateData.UpdateInfo.pContext, TexDesc.Format) :
11211123
&GetStreamForUpdateSize(static_cast<Uint32>(UpdateData.CopyInfo.MemorySize));
@@ -1258,7 +1260,7 @@ void GPUUploadManagerImpl::ReclaimCompletedPages(IDeviceContext* pContext)
12581260
{
12591261
for (Page* P : m_TmpPages)
12601262
{
1261-
P->GetStream()->AddFreePage(P);
1263+
P->Recycle();
12621264
}
12631265
m_TmpPages.clear();
12641266
}

‎Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp‎

Lines changed: 137 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -420,42 +420,56 @@ TEST(GPUUploadManagerTest, DestroyWhileBufferUpdatesAreRunning)
420420

421421
GPUTestingEnvironment::ScopedReset AutoReset;
422422

423-
RefCntAutoPtr<IGPUUploadManager> pUploadManager;
424-
GPUUploadManagerCreateInfo CreateInfo{pDevice, pContext, 1024, 2048};
425-
CreateGPUUploadManager(CreateInfo, &pUploadManager);
426-
ASSERT_TRUE(pUploadManager != nullptr);
427-
428-
const size_t kNumThreads = 4;
429-
std::vector<std::thread> Threads;
430-
std::atomic<Uint32> NumUpdatesRunning{0};
431-
Threading::Signal AllThreadsRunningSignal;
432-
for (size_t t = 0; t < kNumThreads; ++t)
423+
constexpr size_t kNumIterations = 10;
424+
for (size_t i = 0; i < kNumIterations; ++i)
433425
{
434-
Threads.emplace_back(
435-
[&]() {
436-
if (NumUpdatesRunning.fetch_add(1) == kNumThreads - 1)
437-
{
438-
AllThreadsRunningSignal.Trigger();
439-
}
440-
pUploadManager->ScheduleBufferUpdate({nullptr, 0, 4096, nullptr});
441-
NumUpdatesRunning.fetch_sub(1);
442-
});
443-
}
426+
RefCntAutoPtr<IGPUUploadManager> pUploadManager;
427+
// Use small page size to make the uploaded attempt to create a new page and block in ScheduleBufferUpdate
428+
GPUUploadManagerCreateInfo CreateInfo{pDevice, pContext, 1024, 2048};
429+
CreateGPUUploadManager(CreateInfo, &pUploadManager);
430+
ASSERT_TRUE(pUploadManager != nullptr);
431+
432+
const size_t kNumThreads = 4;
433+
std::vector<std::thread> Threads;
434+
std::atomic<Uint32> NumUpdatesRunning{0};
435+
Threading::Signal AllThreadsRunningSignal;
436+
for (size_t t = 0; t < kNumThreads; ++t)
437+
{
438+
Threads.emplace_back(
439+
[&]() {
440+
if (NumUpdatesRunning.fetch_add(1) == kNumThreads - 1)
441+
{
442+
AllThreadsRunningSignal.Trigger();
443+
}
444+
// Set update size to be larger than page size to make the manager create new large page and block in ScheduleBufferUpdate
445+
pUploadManager->ScheduleBufferUpdate({nullptr, 0, 4096, nullptr});
446+
NumUpdatesRunning.fetch_sub(1);
447+
});
448+
}
444449

445-
AllThreadsRunningSignal.Wait();
450+
AllThreadsRunningSignal.Wait();
446451

447-
std::this_thread::sleep_for(10ms);
448-
EXPECT_EQ(NumUpdatesRunning.load(), kNumThreads) << "All threads should be running updates because RenderThreadUpdate() was not called";
452+
// Wait for some time to ensure that ScheduleTextureUpdate starts.
453+
std::this_thread::sleep_for(10ms);
454+
EXPECT_EQ(NumUpdatesRunning.load(), kNumThreads) << "All threads sh0ould be running updates because RenderThreadUpdate() was not called";
449455

450-
LogUploadManagerStats(pUploadManager);
456+
if (i == kNumIterations - 1)
457+
{
458+
LogUploadManagerStats(pUploadManager);
459+
}
451460

452-
pUploadManager.Release();
461+
pUploadManager.Release();
453462

454-
for (std::thread& thread : Threads)
455-
{
456-
thread.join();
463+
for (std::thread& thread : Threads)
464+
{
465+
thread.join();
466+
}
467+
EXPECT_EQ(NumUpdatesRunning.load(), 0u);
468+
469+
pContext->Flush();
470+
pContext->FinishFrame();
471+
pDevice->ReleaseStaleResources();
457472
}
458-
EXPECT_EQ(NumUpdatesRunning.load(), 0u);
459473
}
460474

461475

@@ -1264,4 +1278,98 @@ TEST(GPUUploadManagerTest, ParallelBufferAndTextureUpdates)
12641278
VerifyTextureContents(pTexture, SubresData);
12651279
}
12661280

1281+
1282+
TEST(GPUUploadManagerTest, DestroyWhileTextureUpdatesAreRunning)
1283+
{
1284+
GPUTestingEnvironment* pEnv = GPUTestingEnvironment::GetInstance();
1285+
IRenderDevice* pDevice = pEnv->GetDevice();
1286+
IDeviceContext* pContext = pEnv->GetDeviceContext();
1287+
1288+
GPUTestingEnvironment::ScopedReset AutoReset;
1289+
1290+
TextureDesc TexDesc;
1291+
TexDesc.Type = RESOURCE_DIM_TEX_2D_ARRAY;
1292+
TexDesc.Usage = USAGE_DEFAULT;
1293+
TexDesc.Width = 512;
1294+
TexDesc.Height = 512;
1295+
TexDesc.ArraySize = 32;
1296+
TexDesc.MipLevels = 4;
1297+
TexDesc.BindFlags = BIND_SHADER_RESOURCE;
1298+
1299+
std::array<RefCntAutoPtr<ITexture>, 4> pTextures;
1300+
std::array<TEXTURE_FORMAT, 4> Formats = {TEX_FORMAT_RGBA8_UNORM, TEX_FORMAT_BC1_UNORM, TEX_FORMAT_RG16_FLOAT, TEX_FORMAT_BC3_UNORM};
1301+
for (size_t i = 0; i < pTextures.size(); ++i)
1302+
{
1303+
TextureDesc Desc = TexDesc;
1304+
Desc.Format = Formats[i];
1305+
std::string Name = "GPUUploadManagerTest texture " + std::to_string(i);
1306+
Desc.Name = Name.c_str();
1307+
pDevice->CreateTexture(Desc, nullptr, &pTextures[i]);
1308+
ASSERT_NE(pTextures[i], nullptr);
1309+
}
1310+
1311+
constexpr size_t kNumIterations = 10;
1312+
for (size_t i = 0; i < kNumIterations; ++i)
1313+
{
1314+
RefCntAutoPtr<IGPUUploadManager> pUploadManager;
1315+
// Use small page size to make the uploaded attempt to create a new page and block in ScheduleTextureUpdate
1316+
GPUUploadManagerCreateInfo CreateInfo{pDevice, pContext, 1024, 2048};
1317+
CreateGPUUploadManager(CreateInfo, &pUploadManager);
1318+
ASSERT_TRUE(pUploadManager != nullptr);
1319+
1320+
const size_t kNumThreads = pTextures.size();
1321+
std::vector<std::thread> Threads;
1322+
std::atomic<Uint32> NumUpdatesRunning{0};
1323+
Threading::Signal AllThreadsRunningSignal;
1324+
std::vector<Uint32> Data(256 * 256 * 4);
1325+
1326+
for (size_t t = 0; t < kNumThreads; ++t)
1327+
{
1328+
Threads.emplace_back(
1329+
[&](ITexture* pTexture) {
1330+
if (NumUpdatesRunning.fetch_add(1) == kNumThreads - 1)
1331+
{
1332+
AllThreadsRunningSignal.Trigger();
1333+
}
1334+
1335+
ScheduleTextureUpdateInfo UpdateInfo;
1336+
UpdateInfo.pSrcData = Data.data();
1337+
UpdateInfo.Stride = 256 * 4;
1338+
UpdateInfo.pDstTexture = pTexture;
1339+
UpdateInfo.DstMipLevel = 0;
1340+
UpdateInfo.DstSlice = 0;
1341+
// Set a large box to make the manager request a new large page for update and block in ScheduleTextureUpdate.
1342+
UpdateInfo.DstBox = {0, 256, 0, 256};
1343+
pUploadManager->ScheduleTextureUpdate(UpdateInfo);
1344+
1345+
NumUpdatesRunning.fetch_sub(1);
1346+
},
1347+
pTextures[t]);
1348+
}
1349+
1350+
AllThreadsRunningSignal.Wait();
1351+
1352+
// Wait for some time to ensure that ScheduleTextureUpdate starts.
1353+
std::this_thread::sleep_for(10ms);
1354+
EXPECT_EQ(NumUpdatesRunning.load(), kNumThreads) << "All threads should be running updates because RenderThreadUpdate() was not called";
1355+
1356+
if (i == kNumIterations - 1)
1357+
{
1358+
LogUploadManagerStats(pUploadManager);
1359+
}
1360+
1361+
pUploadManager.Release();
1362+
1363+
for (std::thread& thread : Threads)
1364+
{
1365+
thread.join();
1366+
}
1367+
EXPECT_EQ(NumUpdatesRunning.load(), 0u);
1368+
1369+
pContext->Flush();
1370+
pContext->FinishFrame();
1371+
pDevice->ReleaseStaleResources();
1372+
}
1373+
}
1374+
12671375
} // namespace

0 commit comments

Comments
 (0)