From a3e84adab9f205d2c530a4a7e7032b1d19ad8007 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Sun, 27 Sep 2026 15:00:44 +0300 Subject: [PATCH] CachingFileLoader: keep the partial last block of the file since 7e6405291 only full 64 KB reads get cached, so any read touching the shorter last block of a file came back short. count blocks with blocks_.size() too, so failed reads can't inflate the count until MakeCacheSpaceFor loops forever. also stop read-ahead from asking the backend for blocks past the end of the file. --- Core/FileLoaders/CachingFileLoader.cpp | 23 ++++++---- Core/FileLoaders/CachingFileLoader.h | 1 - unittest/UnitTest.cpp | 63 ++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/Core/FileLoaders/CachingFileLoader.cpp b/Core/FileLoaders/CachingFileLoader.cpp index ca63cdf8e8..9aa9992bb7 100644 --- a/Core/FileLoaders/CachingFileLoader.cpp +++ b/Core/FileLoaders/CachingFileLoader.cpp @@ -100,7 +100,6 @@ size_t CachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flag } void CachingFileLoader::InitCache() { - cacheSize_ = 0; oldestGeneration_ = 0; generation_ = 0; } @@ -120,7 +119,6 @@ void CachingFileLoader::ShutdownCache() { delete [] block.second.ptr; } blocks_.clear(); - cacheSize_ = 0; } size_t CachingFileLoader::ReadFromCache(s64 pos, size_t bytes, void *data) { @@ -152,6 +150,8 @@ size_t CachingFileLoader::ReadFromCache(s64 pos, size_t bytes, void *data) { void CachingFileLoader::SaveIntoCache(s64 pos, size_t bytes, Flags flags, bool readingAhead) { s64 cacheStartPos = pos >> BLOCK_SHIFT; s64 cacheEndPos = (pos + bytes - 1) >> BLOCK_SHIFT; + // Read-ahead can ask for blocks past the end of the file. + cacheEndPos = std::min(cacheEndPos, (filesize_ - 1) >> BLOCK_SHIFT); std::lock_guard guard(blocksMutex_); size_t blocksToRead = 0; @@ -183,8 +183,9 @@ void CachingFileLoader::SaveIntoCache(s64 pos, size_t bytes, Flags flags, bool r // Only cache a block we actually fully read - a short/failed read (e.g. a // dropped connection on a Remote ISO) must not be cached as if valid, or // every later read of this block would silently return the uninitialized - // tail of `buf` as if it were real file data. - if (readBytes == BLOCK_SIZE) { + // tail of `buf` as if it were real file data. The last block of the file + // is shorter, and complete if the read reached the end. + if (readBytes == BLOCK_SIZE || (readBytes > 0 && (cacheStartPos << BLOCK_SHIFT) + (s64)readBytes == filesize_)) { blocks_[cacheStartPos] = BlockInfo{buf}; } else { delete [] buf; @@ -198,6 +199,10 @@ void CachingFileLoader::SaveIntoCache(s64 pos, size_t bytes, Flags flags, bool r u8 *wholeRead = new u8[blocksToRead << BLOCK_SHIFT]; size_t readBytes = backend_->ReadAt(cacheStartPos << BLOCK_SHIFT, blocksToRead << BLOCK_SHIFT, wholeRead, flags); size_t wholeBlocksRead = readBytes >> BLOCK_SHIFT; + if ((readBytes & (BLOCK_SIZE - 1)) != 0 && (cacheStartPos << BLOCK_SHIFT) + (s64)readBytes == filesize_) { + // The short last block of the file. + wholeBlocksRead++; + } blocksMutex_.lock(); for (size_t i = 0; i < wholeBlocksRead; ++i) { @@ -212,19 +217,18 @@ void CachingFileLoader::SaveIntoCache(s64 pos, size_t bytes, Flags flags, bool r delete[] wholeRead; } - cacheSize_ += blocksToRead; ++generation_; } bool CachingFileLoader::MakeCacheSpaceFor(size_t blocks, bool readingAhead) { size_t goal = MAX_BLOCKS_CACHED - blocks; - if (readingAhead && cacheSize_ > goal) { + if (readingAhead && blocks_.size() > goal) { return false; } std::lock_guard guard(blocksMutex_); - while (cacheSize_ > goal) { + while (blocks_.size() > goal) { u64 minGeneration = generation_; // We increment the iterator inside because we delete things inside. @@ -240,10 +244,9 @@ bool CachingFileLoader::MakeCacheSpaceFor(size_t blocks, bool readingAhead) { s64 pos = it->first; delete [] it->second.ptr; blocks_.erase(it); - --cacheSize_; // Our iterator is invalid now. Keep going? - if (cacheSize_ > goal) { + if (blocks_.size() > goal) { // This finds the one at that position. it = blocks_.lower_bound(pos); } else { @@ -267,7 +270,7 @@ void CachingFileLoader::StartReadAhead(s64 pos) { // Already going. return; } - if (cacheSize_ + BLOCK_READAHEAD > MAX_BLOCKS_CACHED) { + if (blocks_.size() + BLOCK_READAHEAD > MAX_BLOCKS_CACHED) { // Not enough space to readahead. return; } diff --git a/Core/FileLoaders/CachingFileLoader.h b/Core/FileLoaders/CachingFileLoader.h index 6a45424ff8..491fcbdafb 100644 --- a/Core/FileLoaders/CachingFileLoader.h +++ b/Core/FileLoaders/CachingFileLoader.h @@ -62,7 +62,6 @@ private: int isDirectory_ = -1; u64 generation_ = 0; u64 oldestGeneration_ = 0; - size_t cacheSize_ = 0; struct BlockInfo { u8 *ptr; diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index f6de2fb15b..7d442ac8c6 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -37,9 +37,11 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -103,6 +105,7 @@ #include "Common/UI/View.h" #include "Common/UI/ViewGroup.h" #include "Core/Debugger/MemBlockInfo.h" +#include "Core/FileLoaders/CachingFileLoader.h" #include "Core/FileSystems/FileSystem.h" #include "Core/FileSystems/ISOFileSystem.h" #include "Core/MemMap.h" @@ -2034,6 +2037,65 @@ bool TestParseLBN() { return true; } +// Serves byte i as (u8)(i * 31 + 7), clamped to the file size like HTTPFileLoader. +// The read counter lives outside, since CachingFileLoader deletes its backend. +class PatternFileLoader : public FileLoader { +public: + PatternFileLoader(s64 size, std::atomic *reads) : size_(size), reads_(reads) {} + bool Exists() override { return true; } + bool IsDirectory() override { return false; } + s64 FileSize() override { return size_; } + Path GetPath() const override { return Path(); } + size_t ReadAt(s64 pos, size_t bytes, size_t count, void *data, Flags flags) override { + (*reads_)++; + s64 end = std::min(pos + (s64)(bytes * count), size_); + for (s64 i = pos; i < end; i++) { + ((u8 *)data)[i - pos] = (u8)(i * 31 + 7); + } + return pos < end ? (size_t)(end - pos) / bytes : 0; + } + +private: + s64 size_; + std::atomic *reads_; +}; + +static bool MatchesPattern(const u8 *data, s64 pos, size_t bytes) { + for (size_t i = 0; i < bytes; i++) { + if (data[i] != (u8)((pos + i) * 31 + 7)) { + return false; + } + } + return true; +} + +static bool TestCachingFileLoader() { + // The last 64 KB block of the file is short. + const s64 size = 3 * 65536 + 1234; + std::atomic reads{}; + std::unique_ptr loader(new CachingFileLoader(new PatternFileLoader(size, &reads))); + std::vector buf(65536 * 2); + + s64 pos = 3 * 65536 + 100; + EXPECT_EQ_INT(loader->ReadAt(pos, 100, buf.data()), 100); + EXPECT_TRUE(MatchesPattern(buf.data(), pos, 100)); + pos = 2 * 65536 + 10; + EXPECT_EQ_INT(loader->ReadAt(pos, (size_t)(size - pos), buf.data()), (int)(size - pos)); + EXPECT_TRUE(MatchesPattern(buf.data(), pos, (size_t)(size - pos))); + + // Both reads ended in the last block, so everything they touched is cached and there's + // nothing left to read ahead. Nothing further should reach the backend, even past EOF. + int readsBefore = reads; + pos = 3 * 65536 + 100; + for (int i = 0; i < 100; i++) { + EXPECT_EQ_INT(loader->ReadAt(pos, 100, buf.data()), 100); + } + // Waits for any read-ahead. + loader.reset(); + EXPECT_EQ_INT(reads, readsBefore); + return true; +} + // So we can use EXPECT_TRUE, etc. struct AlignedMem { AlignedMem(size_t sz, size_t alignment = 16) { @@ -3060,6 +3122,7 @@ TestItem availableTests[] = { TEST_ITEM(Jit), TEST_ITEM(VFPUMatrixTranspose), TEST_ITEM(ParseLBN), + TEST_ITEM(CachingFileLoader), TEST_ITEM(QuickTexHash), TEST_ITEM(CLZ), TEST_ITEM(MemMap),