review feedback

1 files changed, 29 insertions, 28 deletions

diff --git a/zenstore/compactcas.cpp b/zenstore/compactcas.cpp
index 933a7c712..2171fd3c0 100644
--- a/zenstore/compactcas.cpp
+++ b/zenstore/compactcas.cpp
@@ -39,8 +39,6 @@ struct CasDiskIndexHeader
 static_assert(sizeof(CasDiskIndexHeader) == 32);
 
 namespace {
-	uint64_t AlignPositon(uint64_t Offset, uint64_t Alignment) { return (Offset + Alignment - 1) & ~(Alignment - 1); }
-
 	static const char* HexLUT = "0123456789abcdef";
 
 	bool ParseHex(const std::string HexString, uint32_t& OutValue)
@@ -93,7 +91,7 @@ struct CasContainerStrategy::ChunkBlock
 	void						 Create(uint64_t InitialSize);
 	void						 MarkAsDeleteOnClose(std::error_code& Ec);
 	uint64_t					 FileSize();
-	IoBuffer					 GetRange(uint64_t Offset, uint64_t Size);
+	IoBuffer					 GetChunk(uint64_t Offset, uint64_t Size);
 	void						 Read(void* Data, uint64_t Size, uint64_t FileOffset);
 	void						 Write(const void* Data, uint64_t Size, uint64_t FileOffset);
 	void						 Flush();
@@ -103,7 +101,7 @@ private:
 	const std::filesystem::path m_Path;
 	std::atomic<bool>			m_IsOpened;
 	RwLock						m_OpenLock;
-	BasicFile					m_SmallObjectFile;
+	BasicFile					m_File;
 	IoBuffer					m_IoBuffer;
 };
 
@@ -115,7 +113,7 @@ CasContainerStrategy::ChunkBlock::~ChunkBlock()
 {
 	if (m_IsOpened.load())
 	{
-		m_SmallObjectFile.Detach();
+		m_File.Detach();
 	}
 }
 
@@ -141,9 +139,9 @@ CasContainerStrategy::ChunkBlock::Open()
 		return;
 	}
 
-	m_SmallObjectFile.Open(m_Path, false);
-	void* FileHandle = m_SmallObjectFile.Handle();
-	m_IoBuffer		 = IoBuffer(IoBuffer::File, FileHandle, 0, m_SmallObjectFile.FileSize());
+	m_File.Open(m_Path, false);
+	void* FileHandle = m_File.Handle();
+	m_IoBuffer		 = IoBuffer(IoBuffer::File, FileHandle, 0, m_File.FileSize());
 	m_IsOpened.store(true);
 }
 
@@ -153,12 +151,12 @@ CasContainerStrategy::ChunkBlock::Create(uint64_t InitialSize)
 	ZEN_ASSERT(!m_IsOpened.load());
 
 	CreateDirectories(m_Path.parent_path());
-	m_SmallObjectFile.Open(m_Path, true);
+	m_File.Open(m_Path, true);
 	if (InitialSize > 0)
 	{
-		m_SmallObjectFile.SetFileSize(InitialSize);
+		m_File.SetFileSize(InitialSize);
 	}
-	void* FileHandle = m_SmallObjectFile.Handle();
+	void* FileHandle = m_File.Handle();
 	m_IoBuffer		 = IoBuffer(IoBuffer::File, FileHandle, 0, InitialSize);
 	m_IsOpened.store(true);
 }
@@ -167,7 +165,7 @@ uint64_t
 CasContainerStrategy::ChunkBlock::FileSize()
 {
 	ZEN_ASSERT(m_IsOpened.load());
-	return m_SmallObjectFile.FileSize();
+	return m_File.FileSize();
 }
 
 void
@@ -176,7 +174,7 @@ CasContainerStrategy::ChunkBlock::MarkAsDeleteOnClose(std::error_code& Ec)
 	RwLock::ExclusiveLockScope _(m_OpenLock);
 	if (m_IsOpened.load())
 	{
-		m_SmallObjectFile.MarkAsDeleteOnClose(Ec);
+		m_File.MarkAsDeleteOnClose(Ec);
 		return;
 	}
 	if (std::filesystem::is_regular_file(m_Path))
@@ -187,7 +185,7 @@ CasContainerStrategy::ChunkBlock::MarkAsDeleteOnClose(std::error_code& Ec)
 }
 
 IoBuffer
-CasContainerStrategy::ChunkBlock::GetRange(uint64_t Offset, uint64_t Size)
+CasContainerStrategy::ChunkBlock::GetChunk(uint64_t Offset, uint64_t Size)
 {
 	Open();
 	return IoBuffer(m_IoBuffer, Offset, Size);
@@ -197,14 +195,14 @@ void
 CasContainerStrategy::ChunkBlock::Read(void* Data, uint64_t Size, uint64_t FileOffset)
 {
 	ZEN_ASSERT(m_IsOpened.load());
-	m_SmallObjectFile.Read(Data, Size, FileOffset);
+	m_File.Read(Data, Size, FileOffset);
 }
 
 void
 CasContainerStrategy::ChunkBlock::Write(const void* Data, uint64_t Size, uint64_t FileOffset)
 {
 	ZEN_ASSERT(m_IsOpened.load());
-	m_SmallObjectFile.Write(Data, Size, FileOffset);
+	m_File.Write(Data, Size, FileOffset);
 }
 
 void
@@ -214,7 +212,7 @@ CasContainerStrategy::ChunkBlock::Flush()
 	{
 		return;
 	}
-	m_SmallObjectFile.Flush();
+	m_File.Flush();
 }
 
 void
@@ -223,7 +221,7 @@ CasContainerStrategy::ChunkBlock::StreamByteRange(uint64_t												 FileOffse
 												  std::function<void(const void* Data, uint64_t Size)>&& ChunkFun)
 {
 	ZEN_ASSERT(m_IsOpened.load());
-	m_SmallObjectFile.StreamByteRange(FileOffset, Size, std::move(ChunkFun));
+	m_File.StreamByteRange(FileOffset, Size, std::move(ChunkFun));
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -303,13 +301,13 @@ CasContainerStrategy::InsertChunk(const void* ChunkData, size_t ChunkSize, const
 	}
 	const uint64_t InsertOffset = m_CurrentInsertOffset;
 	WriteBlock->Write(ChunkData, ChunkSize, InsertOffset);
-	m_CurrentInsertOffset = AlignPositon(InsertOffset + ChunkSize, m_PayloadAlignment);
+	m_CurrentInsertOffset = RoundUp(InsertOffset + ChunkSize, m_PayloadAlignment);
 
 	const CasLocation Location(WriteBlockIndex, InsertOffset, ChunkSize);
 	CasDiskIndexEntry IndexEntry{.Key = ChunkHash, .Location = CasDiskLocation(Location, m_PayloadAlignment)};
 
 	RwLock::ExclusiveLockScope __(m_LocationMapLock);
-	m_LocationMap[ChunkHash] = CasDiskLocation(Location, m_PayloadAlignment);
+	m_LocationMap.emplace(ChunkHash, CasDiskLocation(Location, m_PayloadAlignment));
 	m_TotalSize.fetch_add(static_cast<uint64_t>(ChunkSize));
 	m_CasLog.Append(IndexEntry);
 
@@ -336,7 +334,7 @@ CasContainerStrategy::FindChunk(const IoHash& ChunkHash)
 			if (BlockIt->second)  // This happens if the data associated with the block is not found - ie the ucas file is deleted but
 								  // the index not updated
 			{
-				return BlockIt->second->GetRange(Location.Offset, Location.Size);
+				return BlockIt->second->GetChunk(Location.Offset, Location.Size);
 			}
 		}
 	}
@@ -369,7 +367,7 @@ CasContainerStrategy::FilterChunks(CasChunkSet& InOutChunks)
 void
 CasContainerStrategy::Flush()
 {
-	RwLock::ExclusiveLockScope _i(m_InsertLock);
+	RwLock::ExclusiveLockScope _(m_InsertLock);
 	m_CasLog.Flush();
 	if (auto WriteBlock = m_WriteBlock.lock())
 	{
@@ -536,8 +534,6 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 			WriteBlock->Flush();
 		}
 
-		BlocksToReWrite.reserve(m_ChunkBlocks.size());
-
 		if (m_LocationMap.empty())
 		{
 			ZEN_INFO("garbage collect SKIPPED, for '{}', container is empty", m_Config.RootDirectory / m_ContainerBaseName);
@@ -545,7 +541,11 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 		}
 
 		const uint64_t TotalChunkCount = m_LocationMap.size();
-		uint64_t	   TotalSize	   = m_TotalSize.load();
+
+		BlocksToReWrite.reserve(m_ChunkBlocks.size());
+		BlockIndexToKeepChunksMap.reserve(m_ChunkBlocks.size());
+		KeepChunks.reserve(m_LocationMap.size());
+		DeletedChunks.reserve(m_LocationMap.size());
 
 		std::vector<IoHash> TotalChunkHashes;
 		TotalChunkHashes.reserve(m_LocationMap.size());
@@ -581,6 +581,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 
 		if (!PerformDelete)
 		{
+			uint64_t TotalSize = m_TotalSize.load();
 			ZEN_INFO("garbage collect from '{}' DISABLED, found #{} {} chunks of total #{} {}",
 					 m_Config.RootDirectory / m_ContainerBaseName,
 					 DeletedChunks.size(),
@@ -733,7 +734,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 				CasLocation NewChunkLocation(NewBlockIndex, WriteOffset, Chunk.size());
 				Entry.second			 = CasDiskLocation(NewChunkLocation, m_PayloadAlignment);
 				MovedBlocks[Entry.first] = Entry.second;
-				WriteOffset				 = AlignPositon(WriteOffset + Chunk.size(), m_PayloadAlignment);
+				WriteOffset				 = RoundUp(WriteOffset + Chunk.size(), m_PayloadAlignment);
 			}
 			Chunk.clear();
 
@@ -1063,7 +1064,7 @@ CasContainerStrategy::OpenContainer(bool IsNewStore)
 				NewBlockFile->Write(Chunk.data(), Chunk.size(), WriteOffset);
 				CasLocation NewChunkLocation(NewBlockIndex, WriteOffset, Chunk.size());
 				m_CasLog.Append({.Key = ChunkHash, .Location = CasDiskLocation(NewChunkLocation, m_PayloadAlignment)});
-				WriteOffset = AlignPositon(WriteOffset + Chunk.size(), m_PayloadAlignment);
+				WriteOffset = RoundUp(WriteOffset + Chunk.size(), m_PayloadAlignment);
 			}
 			m_CasLog.Close();
 
@@ -1193,7 +1194,7 @@ CasContainerStrategy::OpenContainer(bool IsNewStore)
 	if (OpenExistingBlock)
 	{
 		m_WriteBlock		  = m_ChunkBlocks[m_WriteBlockIndex];
-		m_CurrentInsertOffset = AlignPositon(SmallestBlockSize, m_PayloadAlignment);
+		m_CurrentInsertOffset = RoundUp(SmallestBlockSize, m_PayloadAlignment);
 	}
 
 	// Create GC reserve file if possible