Merge pull request #3795 from EmptyChaos/fix-diskreader

DriveReader: Fix View > Show Drives

Source/Core/DiscIO/Blob.cpp

@@ -2,8 +2,8 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <cstddef>
-#include <cstring>
 #include <limits>
 #include <memory>
 #include <string>
@@ -22,87 +22,156 @@
 namespace DiscIO
 {
 
-// Provides caching and split-operation-to-block-operations facilities.
-// Used for compressed blob reading and direct drive reading.
-
 void SectorReader::SetSectorSize(int blocksize)
 {
+	m_block_size = std::max(blocksize, 0);
 	for (auto& cache_entry : m_cache)
-		cache_entry.resize(blocksize);
+	{
+		cache_entry.Reset();
+		cache_entry.data.resize(m_chunk_blocks * m_block_size);
+	}
+}
 
-	m_cache_tags.fill(std::numeric_limits<u64>::max());
-	m_blocksize = blocksize;
+void SectorReader::SetChunkSize(int block_cnt)
+{
+	m_chunk_blocks = std::max(block_cnt, 1);
+	// Clear cache and resize the data arrays
+	SetSectorSize(m_block_size);
 }
 
 SectorReader::~SectorReader()
 {
 }
 
-const std::vector<u8>& SectorReader::GetBlockData(u64 block_num)
+const SectorReader::Cache* SectorReader::FindCacheLine(u64 block_num)
 {
-	// TODO : Expand usage of the cache to more than one block :P
-	if (m_cache_tags[0] == block_num)
-		return m_cache[0];
+	auto itr = std::find_if(m_cache.begin(), m_cache.end(), [&](const Cache& entry)
+	{
+		return entry.Contains(block_num);
+	});
+	if (itr == m_cache.end())
+		return nullptr;
 
-	GetBlock(block_num, m_cache[0].data());
-	m_cache_tags[0] = block_num;
-	return m_cache[0];
+	itr->MarkUsed();
+	return &*itr;
+}
+
+SectorReader::Cache* SectorReader::GetEmptyCacheLine()
+{
+	Cache* oldest = &m_cache[0];
+	// Find the Least Recently Used cache line to replace.
+	for (auto& cache_entry : m_cache)
+	{
+		if (cache_entry.IsLessRecentlyUsedThan(*oldest))
+			oldest = &cache_entry;
+		cache_entry.ShiftLRU();
+	}
+	oldest->Reset();
+	return oldest;
+}
+
+const SectorReader::Cache* SectorReader::GetCacheLine(u64 block_num)
+{
+	if (auto entry = FindCacheLine(block_num))
+		return entry;
+
+	// Cache miss. Fault in the missing entry.
+	Cache* cache = GetEmptyCacheLine();
+	// We only read aligned chunks, this avoids duplicate overlapping entries.
+	u64 chunk_idx = block_num / m_chunk_blocks;
+	u32 blocks_read = ReadChunk(cache->data.data(), chunk_idx);
+	if (!blocks_read)
+		return nullptr;
+	cache->Fill(chunk_idx * m_chunk_blocks, blocks_read);
+
+	// Secondary check for out-of-bounds read.
+	// If we got less than m_chunk_blocks, we may still have missed.
+	// We do this after the cache fill since the cache line itself is
+	// fine, the problem is being asked to read past the end of the disk.
+	return cache->Contains(block_num) ? cache : nullptr;
 }
 
 bool SectorReader::Read(u64 offset, u64 size, u8* out_ptr)
 {
-	u64 startingBlock = offset / m_blocksize;
 	u64 remain = size;
-
-	int positionInBlock = (int)(offset % m_blocksize);
-	u64 block = startingBlock;
+	u64 block  = 0;
+	u32 position_in_block = static_cast<u32>(offset % m_block_size);
 
 	while (remain > 0)
 	{
-		// Check if we are ready to do a large block read. > instead of >= so we don't bother if remain is only one block.
-		if (positionInBlock == 0 && remain > (u64)m_blocksize)
-		{
-			u64 num_blocks = remain / m_blocksize;
-			ReadMultipleAlignedBlocks(block, num_blocks, out_ptr);
-			block += num_blocks;
-			out_ptr += num_blocks * m_blocksize;
-			remain -= num_blocks * m_blocksize;
-			continue;
-		}
+		block = offset / m_block_size;
 
-		const std::vector<u8>& data = GetBlockData(block);
+		const Cache* cache = GetCacheLine(block);
+		if (!cache)
+			return false;
 
-		u32 to_copy = m_blocksize - positionInBlock;
-		if (to_copy >= remain)
-		{
-			// Yay, we are done!
-			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + remain, out_ptr);
-			return true;
-		}
-		else
-		{
-			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + to_copy, out_ptr);
-			out_ptr += to_copy;
-			remain -= to_copy;
-			positionInBlock = 0;
-			block++;
-		}
-	}
+		// Cache entries are aligned chunks, we may not want to read from the start
+		u32 read_offset = static_cast<u32>(block - cache->block_idx) * m_block_size + position_in_block;
+		u32 can_read = m_block_size * cache->num_blocks - read_offset;
+		u32 was_read = static_cast<u32>(std::min<u64>(can_read, remain));
+
+		std::copy(cache->data.begin() + read_offset,
+		          cache->data.begin() + read_offset + was_read,
+		          out_ptr);
 
+		offset  += was_read;
+		out_ptr += was_read;
+		remain  -= was_read;
+		position_in_block = 0;
+	}
 	return true;
 }
 
-bool SectorReader::ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr)
+// Crap default implementation if not overridden.
+bool SectorReader::ReadMultipleAlignedBlocks(u64 block_num, u64 cnt_blocks, u8* out_ptr)
 {
-	for (u64 i = 0; i < num_blocks; i++)
+	for (u64 i = 0; i < cnt_blocks; ++i)
 	{
-		const std::vector<u8>& data = GetBlockData(block_num + i);
-		const u64 offset = i * m_blocksize;
+		if (!GetBlock(block_num + i, out_ptr))
+			return false;
+		out_ptr += m_block_size;
+	}
+	return true;
+}
 
-		std::copy(data.begin(), data.end(), out_ptr + offset);
+u32 SectorReader::ReadChunk(u8* buffer, u64 chunk_num)
+{
+	u64 block_num  = chunk_num * m_chunk_blocks;
+	u32 cnt_blocks = m_chunk_blocks;
+
+	// If we are reading the end of a disk, there may not be enough blocks to
+	// read a whole chunk. We need to clamp down in that case.
+	u64 end_block = GetDataSize() / m_block_size;
+	if (end_block)
+		cnt_blocks = static_cast<u32>(std::min<u64>(m_chunk_blocks, end_block - block_num));
+
+	if (ReadMultipleAlignedBlocks(block_num, cnt_blocks, buffer))
+	{
+		if (cnt_blocks < m_chunk_blocks)
+		{
+			std::fill(buffer + cnt_blocks * m_block_size,
+			          buffer + m_chunk_blocks * m_block_size,
+			          0u);
+		}
+		return cnt_blocks;
 	}
 
-	return true;
+	// end_block may be zero on real disks if we fail to get the media size.
+	// We have to fallback to probing the disk instead.
+	if (!end_block)
+	{
+		for (u32 i = 0; i < cnt_blocks; ++i)
+		{
+			if (!GetBlock(block_num + i, buffer))
+			{
+				std::fill(buffer, buffer + (cnt_blocks - i) * m_block_size, 0u);
+				return i;
+			}
+			buffer += m_block_size;
+		}
+		return cnt_blocks;
+	}
+	return 0;
 }
 
 std::unique_ptr<IBlobReader> CreateBlobReader(const std::string& filename)

Source/Core/DiscIO/Blob.h

@@ -50,32 +50,112 @@ class IBlobReader
 };
 
 
-// Provides caching and split-operation-to-block-operations facilities.
-// Used for compressed blob reading and direct drive reading.
-// Currently only uses a single entry cache.
-// Multi-block reads are not cached.
+// Provides caching and byte-operation-to-block-operations facilities.
+// Used for compressed blob and direct drive reading.
+// NOTE: GetDataSize() is expected to be evenly divisible by the sector size.
 class SectorReader : public IBlobReader
 {
 public:
-	virtual ~SectorReader();
+	virtual ~SectorReader() = 0;
 
-	bool Read(u64 offset, u64 size, u8 *out_ptr) override;
-	friend class DriveReader;
+	bool Read(u64 offset, u64 size, u8* out_ptr) override;
 
 protected:
 	void SetSectorSize(int blocksize);
-	virtual void GetBlock(u64 block_num, u8 *out) = 0;
-	// This one is uncached. The default implementation is to simply call GetBlockData multiple times and memcpy.
-	virtual bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8 *out_ptr);
+	int GetSectorSize() const
+	{
+		return m_block_size;
+	}
 
-private:
-	// A reference returned by GetBlockData is invalidated as soon as GetBlockData, Read, or ReadMultipleAlignedBlocks is called again.
-	const std::vector<u8>& GetBlockData(u64 block_num);
+	// Set the chunk size -> the number of blocks to read at a time.
+	// Default value is 1 but that is too low for physical devices
+	// like CDROMs. Setting this to a higher value helps reduce seeking
+	// and IO overhead by batching reads. Do not set it too high either
+	// as large reads are slow and will take too long to resolve.
+	void SetChunkSize(int blocks);
+	int GetChunkSize() const
+	{
+		return m_chunk_blocks;
+	}
 
-	enum { CACHE_SIZE = 32 };
-	int m_blocksize;
-	std::array<std::vector<u8>, CACHE_SIZE> m_cache;
-	std::array<u64, CACHE_SIZE> m_cache_tags;
+	// Read a single block/sector.
+	virtual bool GetBlock(u64 block_num, u8* out) = 0;
+
+	// Read multiple contiguous blocks.
+	// Default implementation just calls GetBlock in a loop, it should be
+	// overridden in derived classes where possible.
+	virtual bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr);
+
+private:
+	struct Cache
+	{
+		std::vector<u8> data;
+		u64 block_idx  = 0;
+		u32 num_blocks = 0;
+
+		// [Pseudo-] Least Recently Used Shift Register
+		// When an empty cache line is needed, the line with the lowest value
+		// is taken and reset; the LRU register is then shifted down 1 place
+		// on all lines (low bit discarded). When a line is used, the high bit
+		// is set marking it as most recently used.
+		u32 lru_sreg = 0;
+
+		void Reset()
+		{
+			block_idx  = 0;
+			num_blocks = 0;
+			lru_sreg   = 0;
+		}
+		void Fill(u64 block, u32 count)
+		{
+			block_idx  = block;
+			num_blocks = count;
+			// NOTE: Setting only the high bit means the newest line will
+			//   be selected for eviction if every line in the cache was
+			//   touched. This gives MRU behavior which is probably
+			//   desirable in that case.
+			MarkUsed();
+		}
+		bool Contains(u64 block) const
+		{
+			return block >= block_idx && block - block_idx < num_blocks;
+		}
+		void MarkUsed()
+		{
+			lru_sreg |= 0x80000000;
+		}
+		void ShiftLRU()
+		{
+			lru_sreg >>= 1;
+		}
+		bool IsLessRecentlyUsedThan(const Cache& other) const
+		{
+			return lru_sreg < other.lru_sreg;
+		}
+	};
+
+	// Gets the cache line that contains the given block, or nullptr.
+	// NOTE: The cache record only lasts until it expires (next GetEmptyCacheLine)
+	const Cache* FindCacheLine(u64 block_num);
+
+	// Finds the least recently used cache line, resets and returns it.
+	Cache* GetEmptyCacheLine();
+
+	// Combines FindCacheLine with GetEmptyCacheLine and ReadChunk.
+	// Always returns a valid cache line (loading the data if needed).
+	// May return nullptr only if the cache missed and the read failed.
+	const Cache* GetCacheLine(u64 block_num);
+
+	// Read all bytes from a chunk of blocks into a buffer.
+	// Returns the number of blocks read (may be less than m_chunk_blocks
+	// if chunk_num is the last chunk on the disk and the disk size is not
+	// evenly divisible into chunks). Returns zero if it fails.
+	u32 ReadChunk(u8* buffer, u64 chunk_num);
+
+	static constexpr int CACHE_LINES = 32;
+	u32 m_block_size   = 0;  // Bytes in a sector/block
+	u32 m_chunk_blocks = 1;  // Number of sectors/blocks in a chunk
+	std::array<Cache, CACHE_LINES> m_cache;
 };
 
 class CBlobBigEndianReader

Source/Core/DiscIO/CompressedBlob.cpp

@@ -79,7 +79,7 @@ u64 CompressedBlobReader::GetBlockCompressedSize(u64 block_num) const
 	return 0;
 }
 
-void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
+bool CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 {
 	bool uncompressed = false;
 	u32 comp_block_size = (u32)GetBlockCompressedSize(block_num);
@@ -97,7 +97,13 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 	memset(&m_zlib_buffer[comp_block_size], 0, m_zlib_buffer.size() - comp_block_size);
 
 	m_file.Seek(offset, SEEK_SET);
-	m_file.ReadBytes(m_zlib_buffer.data(), comp_block_size);
+	if (!m_file.ReadBytes(m_zlib_buffer.data(), comp_block_size))
+	{
+		PanicAlertT("The disc image \"%s\" is truncated, some of the data is missing.",
+		            m_file_name.c_str());
+		m_file.Clear();
+		return false;
+	}
 
 	// First, check hash.
 	u32 block_hash = HashAdler32(m_zlib_buffer.data(), comp_block_size);
@@ -133,8 +139,12 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 		}
 		inflateEnd(&z);
 		if (uncomp_size != m_header.block_size)
+		{
 			PanicAlert("Wrong block size");
+			return false;
+		}
 	}
+	return true;
 }
 
 bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u32 sub_type,

Source/Core/DiscIO/CompressedBlob.h

@@ -55,7 +55,7 @@ class CompressedBlobReader : public SectorReader
 	u64 GetDataSize() const override { return m_header.data_size; }
 	u64 GetRawSize() const override { return m_file_size; }
 	u64 GetBlockCompressedSize(u64 block_num) const;
-	void GetBlock(u64 block_num, u8* out_ptr) override;
+	bool GetBlock(u64 block_num, u8* out_ptr) override;
 private:
 	CompressedBlobReader(const std::string& filename);