From 4fa05327195854a69ccab4989d973bd614fa418b Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 27 Nov 2024 14:46:59 +0000 Subject: [PATCH] Use transient self-deleting file handles for AS cache * This avoids the problem where an application exits so fast on shutdown that we don't have time to execute any cleanup functions to delete cached ASs. * To avoid opening a huge number of file handles we also collate cached ASs into batched files --- renderdoc/driver/d3d12/d3d12_initstate.cpp | 12 +- renderdoc/driver/d3d12/d3d12_manager.cpp | 156 +++++++++++++++++---- renderdoc/driver/d3d12/d3d12_manager.h | 60 +++++++- renderdoc/os/os_specific.cpp | 8 ++ renderdoc/os/os_specific.h | 2 + renderdoc/os/posix/posix_stringio.cpp | 7 + renderdoc/os/win32/win32_stringio.cpp | 38 +++++ 7 files changed, 247 insertions(+), 36 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_initstate.cpp b/renderdoc/driver/d3d12/d3d12_initstate.cpp index adc665559..5166380ca 100644 --- a/renderdoc/driver/d3d12/d3d12_initstate.cpp +++ b/renderdoc/driver/d3d12/d3d12_initstate.cpp @@ -618,7 +618,7 @@ uint64_t D3D12ResourceManager::GetSize_InitialState(ResourceId id, const D3D12In if(buildData->buffer) ret += 64 + buildData->buffer->Size(); - ret += 64 + buildData->bytesOnDisk; + ret += 64 + buildData->diskCache.size; return ret; } @@ -1385,9 +1385,9 @@ bool D3D12ResourceManager::Serialise_InitialState(SerialiserType &ser, ResourceI ret = false; } } - else if(!initial->buildData->filename.empty()) + else if(initial->buildData->diskCache.Valid()) { - ContentsLength = initial->buildData->bytesOnDisk; + ContentsLength = initial->buildData->diskCache.size; } buildData = initial->buildData; @@ -1447,11 +1447,9 @@ bool D3D12ResourceManager::Serialise_InitialState(SerialiserType &ser, ResourceI BufferContents = tempAlloc = new byte[(size_t)ContentsLength]; } - if(!buildData->filename.empty() && ser.IsWriting()) + if(buildData->diskCache.Valid() && ser.IsWriting()) { - StreamReader reader(FileIO::fopen(buildData->filename, FileIO::ReadBinary)); - - ser.SerialiseStream("BufferContents"_lit, reader); + GetRTManager()->ReadDiskCache(ser, "BufferContents"_lit, buildData->diskCache); } else { diff --git a/renderdoc/driver/d3d12/d3d12_manager.cpp b/renderdoc/driver/d3d12/d3d12_manager.cpp index 4cfe95d0c..1ab73e15d 100644 --- a/renderdoc/driver/d3d12/d3d12_manager.cpp +++ b/renderdoc/driver/d3d12/d3d12_manager.cpp @@ -1033,6 +1033,126 @@ void D3D12RTManager::TickASManagement() CheckASCaching(); } +FILE *OpenCacheFile() +{ + rdcstr filename = StringFormat::Fmt( + "%s/rdoc_as_%llu_%llu.bin", get_dirname(RenderDoc::Inst().GetCaptureFileTemplate()).c_str(), + Timing::GetTick(), Threading::GetCurrentID()); + FILE *file = FileIO::OpenTransientFileHandle(filename, FileIO::OverwriteBinary); + if(!file) + { + FileIO::CreateParentDirectory(filename); + file = FileIO::OpenTransientFileHandle(filename, FileIO::OverwriteBinary); + } + + return file; +} + +DiskCachedAS D3D12RTManager::AllocDiskCache(uint64_t byteSize) +{ + DiskCachedAS ret; + ret.size = byteSize; + + uint64_t blocksNeeded = AlignUp(byteSize, DiskCacheFile::blockSize) / DiskCacheFile::blockSize; + + if(blocksNeeded >= DiskCacheFile::blocksInFile) + RDCWARN("disk cache sized insufficiently for allocation %llu, allocating dedicated file", + byteSize); + + if(blocksNeeded < DiskCacheFile::blocksInFile) + { + SCOPED_LOCK(m_DiskCacheLock); + + for(size_t i = 0; i < m_DiskCache.size(); i++) + { + DiskCacheFile &diskCache = m_DiskCache[i]; + + uint64_t firstBlock = ~0U; + uint64_t blocksFree = 0; + for(uint64_t b = 0; b < (uint64_t)ARRAY_COUNT(DiskCacheFile::blocksUsed); b++) + { + if(b + blocksNeeded > ARRAY_COUNT(DiskCacheFile::blocksUsed)) + break; + + if(diskCache.blocksUsed[b]) + { + blocksFree = 0; + firstBlock = ~0U; + } + else + { + blocksFree++; + if(firstBlock == ~0U) + firstBlock = b; + + if(blocksFree == blocksNeeded) + break; + } + } + + if(blocksFree == blocksNeeded) + { + ret.fileIndex = i; + ret.offset = firstBlock * DiskCacheFile::blockSize; + memset(&m_DiskCache[i].blocksUsed[firstBlock], 1, blocksNeeded * sizeof(bool)); + return ret; + } + } + } + + // if this was oversized it will allow silent writing off the end by the user + { + SCOPED_LOCK(m_DiskCacheLock); + FILE *f = OpenCacheFile(); + DiskCacheFile cache; + cache.file = f; + + // if this was an outsized file, only reset the first N blocks for our tracking. The file will + // be larger forever, but that's fine + if(blocksNeeded > DiskCacheFile::blocksInFile) + blocksNeeded = DiskCacheFile::blocksInFile; + + memset(cache.blocksUsed, 1, blocksNeeded * sizeof(bool)); + + { + SCOPED_LOCK(m_DiskCacheLock); + ret.fileIndex = m_DiskCache.size(); + m_DiskCache.push_back(cache); + } + + return ret; + } +} + +void D3D12RTManager::ReleaseDiskCache(DiskCachedAS diskCache) +{ + uint64_t blocksNeeded = + AlignUp(diskCache.size, DiskCacheFile::blockSize) / DiskCacheFile::blockSize; + uint64_t blockOffset = diskCache.offset / DiskCacheFile::blockSize; + + // if this was an outsized file, only reset the first N blocks for our tracking. The file will be + // larger forever, but that's fine + if(blocksNeeded > DiskCacheFile::blocksInFile) + blocksNeeded = DiskCacheFile::blocksInFile; + + if(diskCache.fileIndex >= m_DiskCache.size()) + { + RDCERR("Invalid disk cache file %zu vs %zu", diskCache.fileIndex, m_DiskCache.size()); + return; + } + + memset(&m_DiskCache[diskCache.fileIndex].blocksUsed[blockOffset], 0, blocksNeeded * sizeof(bool)); +} + +void D3D12RTManager::FillDiskCache(DiskCachedAS diskCache, void *data) +{ + FileIO::fseek64(m_DiskCache[diskCache.fileIndex].file, diskCache.offset, SEEK_SET); + + StreamWriter writer(m_DiskCache[diskCache.fileIndex].file, Ownership::Nothing); + // de-interleave positions in geoms here if their stride is greater than vertex format to save space? + writer.Write(data, diskCache.size); +} + void D3D12RTManager::PushDiskCacheTask(std::function task) { { @@ -1172,34 +1292,20 @@ void D3D12RTManager::CheckASCaching() // grab parameters for the task D3D12GpuBuffer *buf = buildData->buffer; - rdcstr filename = StringFormat::Fmt( - "%s/rdoc_as_%llu_%llu.bin", get_dirname(RenderDoc::Inst().GetCaptureFileTemplate()).c_str(), - Timing::GetTick(), Threading::GetCurrentID()); // immediately update the build data as if the cache has completed. We flush the thread - buildData->bytesOnDisk = buildData->buffer->Size(); + DiskCachedAS diskCache = buildData->diskCache = AllocDiskCache(buildData->buffer->Size()); buildData->buffer = NULL; - buildData->filename = filename; if(D3D12_Debug_RT_Auditing()) { RDCDEBUG("Flushing AS build data of size %llu to disk", buf->Size()); } - PushDiskCacheTask([buf, filename]() { - FILE *f = FileIO::fopen(filename, FileIO::WriteBinary); + PushDiskCacheTask([this, diskCache, buf]() { + // de-interleave positions in geoms here if their stride is greater than vertex format to save space? - if(!f) - { - FileIO::CreateParentDirectory(filename); - f = FileIO::fopen(filename, FileIO::WriteBinary); - } - - { - StreamWriter writer(f, Ownership::Stream); - // de-interleave positions in geoms here if their stride is greater than vertex format to save space? - writer.Write(buf->Map(), buf->Size()); - } + FillDiskCache(diskCache, buf->Map()); buf->Unmap(); buf->Release(); @@ -1255,13 +1361,13 @@ void D3D12RTManager::GatherRTStatistics(ASStats &blasAges, ASStats &tlasAges, for(ASBuildData *buildData : m_DiskCachedASBuildDatas) { - if(buildData && !buildData->filename.empty()) + if(buildData && buildData->diskCache.Valid()) { ASStats &ages = buildData->Type == D3D12_RAYTRACING_ACCELERATION_STRUCTURE_TYPE_TOP_LEVEL ? tlasAges : blasAges; - ages.diskBytes += buildData->bytesOnDisk; + ages.diskBytes += buildData->diskCache.size; ages.diskCached++; } } @@ -1277,7 +1383,7 @@ void D3D12RTManager::GatherRTStatistics(ASStats &blasAges, ASStats &tlasAges, : blasAges; // should never encounter this - if(!buildData->filename.empty()) + if(buildData->diskCache.Valid()) continue; uint64_t size = buildData->buffer ? buildData->buffer->Size() : 0; @@ -3486,13 +3592,9 @@ void ASBuildData::Release() SAFE_RELEASE(buffer); - if(!filename.empty()) + if(diskCache.size && rtManager) { - if(rtManager) - { - rdcstr fileToDelete = filename; - rtManager->PushDiskCacheTask([fileToDelete]() { FileIO::Delete(fileToDelete); }); - } + rtManager->ReleaseDiskCache(diskCache); } delete this; diff --git a/renderdoc/driver/d3d12/d3d12_manager.h b/renderdoc/driver/d3d12/d3d12_manager.h index dd96b41f4..f8034cdcd 100644 --- a/renderdoc/driver/d3d12/d3d12_manager.h +++ b/renderdoc/driver/d3d12/d3d12_manager.h @@ -1104,6 +1104,15 @@ struct RTGPUPatchingStats double totalDispatchesMS; }; +struct DiskCachedAS +{ + size_t fileIndex = ~0U; + uint64_t offset = 0; + uint64_t size = 0; + + bool Valid() const { return fileIndex != ~0U; } +}; + // this is a refcounted GPU buffer with the build data, together with the metadata struct ASBuildData { @@ -1178,8 +1187,7 @@ struct ASBuildData void Release(); D3D12GpuBuffer *buffer = NULL; - rdcstr filename; - uint64_t bytesOnDisk = 0; + DiskCachedAS diskCache; uint32_t query = 0; std::function cleanupCallback; @@ -1258,6 +1266,54 @@ public: void AddPendingASBuilds(ID3D12Fence *fence, UINT64 waitValue, const rdcarray> &callbacks); void TickASManagement(); + + // this disk cache is primarily single threaded - either the disk cache thread owns + // seeking/writing to the files, or during initial states that thread owns seeking/reading. + // we lock around this access only for allocating from blocks + struct DiskCacheFile + { + FILE *file = NULL; + + // each block is 1kB to split the difference between caching lots of tiny ASs and wasting space, + // vs tracking many blocks + static const uint64_t blockSize = 1 * 1024; + static const uint64_t blocksInFile = 64 * 1024; + + // one per block + bool blocksUsed[blocksInFile] = {}; + }; + Threading::CriticalSection m_DiskCacheLock; + rdcarray m_DiskCache; + + DiskCachedAS AllocDiskCache(uint64_t byteSize); + void FillDiskCache(DiskCachedAS diskCache, void *data); + void ReleaseDiskCache(DiskCachedAS diskCache); + template + void ReadDiskCache(SerialiserType &ser, rdcliteral name, DiskCachedAS diskCache) + { + if(!diskCache.Valid()) + return; + + // this lock should have no contention, we should only be doing this during initial state + // serialisation when nothing is allocating and the disk cache thread has been flushed + SCOPED_LOCK(m_DiskCacheLock); + + if(diskCache.fileIndex >= m_DiskCache.size()) + { + RDCERR("Invalid disk cache file %zu vs %zu", diskCache.fileIndex, m_DiskCache.size()); + return; + } + + FILE *f = m_DiskCache[diskCache.fileIndex].file; + + FileIO::fseek64(f, diskCache.offset, SEEK_SET); + + { + StreamReader reader(f, diskCache.size, Ownership::Nothing); + ser.SerialiseStream(name, reader); + } + } + void PushDiskCacheTask(std::function task); void FlushDiskCacheThread(); diff --git a/renderdoc/os/os_specific.cpp b/renderdoc/os/os_specific.cpp index 2b25513f1..633eca903 100644 --- a/renderdoc/os/os_specific.cpp +++ b/renderdoc/os/os_specific.cpp @@ -199,6 +199,14 @@ TEST_CASE("Test OS-specific functions", "[osspecific]") FileIO::GetLibraryFilename(libPath); CHECK_FALSE(libPath.empty()); } + SECTION("OpenTransientFileHandle") + { + rdcstr filename = FileIO::GetTempFolderFilename() + "/rdcunittestfile"; + FILE *f = FileIO::OpenTransientFileHandle(filename, FileIO::OverwriteBinary); + CHECK(FileIO::exists(filename)); + FileIO::fclose(f); + CHECK_FALSE(FileIO::exists(filename)); + } SECTION("Environment Variables") { rdcstr var = Process::GetEnvVariable("TMP"); diff --git a/renderdoc/os/os_specific.h b/renderdoc/os/os_specific.h index 173fdce28..a11dec357 100644 --- a/renderdoc/os/os_specific.h +++ b/renderdoc/os/os_specific.h @@ -325,6 +325,8 @@ enum FileMode }; FILE *fopen(const rdcstr &filename, FileMode mode); +FILE *OpenTransientFileHandle(const rdcstr &filename, FileMode mode); + size_t fread(void *buf, size_t elementSize, size_t count, FILE *f); size_t fwrite(const void *buf, size_t elementSize, size_t count, FILE *f); diff --git a/renderdoc/os/posix/posix_stringio.cpp b/renderdoc/os/posix/posix_stringio.cpp index 5d0778727..aa817c429 100644 --- a/renderdoc/os/posix/posix_stringio.cpp +++ b/renderdoc/os/posix/posix_stringio.cpp @@ -411,6 +411,13 @@ FILE *fopen(const rdcstr &filename, FileMode mode) return ::fopen(filename.c_str(), modeString[mode]); } +FILE *OpenTransientFileHandle(const rdcstr &filename, FileMode mode) +{ + FILE *ret = ::fopen(filename.c_str(), modeString[mode]); + ::unlink(filename.c_str()); + return ret; +} + rdcstr ErrorString() { return strerror(errno); diff --git a/renderdoc/os/win32/win32_stringio.cpp b/renderdoc/os/win32/win32_stringio.cpp index c54ed1a3b..3b9b32a9e 100644 --- a/renderdoc/os/win32/win32_stringio.cpp +++ b/renderdoc/os/win32/win32_stringio.cpp @@ -605,6 +605,44 @@ FILE *fopen(const rdcstr &filename, FileMode mode) return ret; } +FILE *FileIO::OpenTransientFileHandle(const rdcstr &filename, FileMode mode) +{ + rdcwstr wfn = StringFormat::UTF82Wide(filename); + + // specify the handle as non-inheriting + SECURITY_ATTRIBUTES security = {}; + security.nLength = sizeof(security); + security.bInheritHandle = FALSE; + + HANDLE handle = CreateFileW(wfn.c_str(), GENERIC_READ | GENERIC_WRITE, 0, &security, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, NULL); + + if(!handle || handle == INVALID_HANDLE_VALUE) + { + return NULL; + } + + int fd = _open_osfhandle((intptr_t)handle, 0); + + if(fd < 0) + { + CloseHandle(handle); + RDCERR("Failed to convert handle to fd: %d", errno); + return NULL; + } + + FILE *ret = _wfdopen(fd, modeString[mode]); + + if(!ret) + { + RDCERR("Failed to open fd as FILE: %d", errno); + _close(fd); + return NULL; + } + + return ret; +} + bool IsUntrustedFile(const rdcstr &filename) { IPersistFile *file = NULL;