gc block size target max size (#180)

* If a block is small (less than half max size) we add it to blocks to compact
Sort blocks when iterating over them
* do compact of block stores even if no new unused are found
* do compact phase even if bucket is empty

4 files changed, 67 insertions, 42 deletions

diff --git a/src/zenstore/blockstore.cpp b/src/zenstore/blockstore.cpp
index 592d1c7fb..2ae51d627 100644
--- a/src/zenstore/blockstore.cpp
+++ b/src/zenstore/blockstore.cpp
@@ -383,6 +383,11 @@ BlockStore::GetBlocksToCompact(const BlockUsageMap& BlockUsage, uint32_t BlockUs
 	BlockEntryCountMap Result;
 	{
 		RwLock::SharedLockScope InsertLock(m_InsertLock);
+
+		const uint64_t SmallBlockLimit = m_MaxBlockSize / 2;
+
+		std::vector<uint32_t> SmallBlockIndexes;
+
 		for (const auto& It : m_ChunkBlocks)
 		{
 			uint32_t BlockIndex = It.first;
@@ -403,35 +408,53 @@ BlockStore::GetBlocksToCompact(const BlockUsageMap& BlockUsage, uint32_t BlockUs
 				UsedCount = UsageIt->second.EntryCount;
 			}
 
-			uint64_t BlockSize = It.second ? It.second->FileSize() : 0u;
-			if (BlockSize == 0)
+			uint64_t PhysicalSize = It.second ? It.second->FileSize() : 0u;
+			if (PhysicalSize == 0)
 			{
 				Result.insert_or_assign(BlockIndex, UsedCount);
 				continue;
 			}
 
+			bool IsBelowUnusedLimit = false;
+
 			if (BlockUsageThresholdPercent == 100)
 			{
-				if (UsedSize < BlockSize)
+				if (UsedSize < PhysicalSize)
 				{
-					Result.insert_or_assign(BlockIndex, UsedCount);
+					IsBelowUnusedLimit = true;
 				}
 			}
 			else if (BlockUsageThresholdPercent == 0)
 			{
 				if (UsedSize == 0)
 				{
-					Result.insert_or_assign(BlockIndex, UsedCount);
+					IsBelowUnusedLimit = true;
 				}
 			}
 			else
 			{
-				const uint32_t UsedPercent = UsedSize < BlockSize ? gsl::narrow<uint32_t>((100 * UsedSize) / BlockSize) : 100u;
+				const uint32_t UsedPercent = UsedSize < PhysicalSize ? gsl::narrow<uint32_t>((100 * UsedSize) / PhysicalSize) : 100u;
 				if (UsedPercent < BlockUsageThresholdPercent)
 				{
-					Result.insert_or_assign(BlockIndex, UsedCount);
+					IsBelowUnusedLimit = true;
 				}
 			}
+
+			if (IsBelowUnusedLimit)
+			{
+				Result.insert_or_assign(BlockIndex, UsedCount);
+			}
+			else if (PhysicalSize < SmallBlockLimit)
+			{
+				Result.insert_or_assign(BlockIndex, UsedCount);
+				SmallBlockIndexes.push_back(BlockIndex);
+			}
+		}
+
+		// If we only find one small block to compact, let it be.
+		if (SmallBlockIndexes.size() == 1 && Result.size() == 1)
+		{
+			Result.erase(SmallBlockIndexes[0]);
 		}
 	}
 	return Result;
@@ -1569,7 +1592,7 @@ BlockStore::CompactBlocks(const BlockStoreCompactState&	  CompactState,
 					 KeepChunkIndexes.size(),
 					 NiceBytes(MovedFromBlock),
 					 GetBlockPath(m_BlocksBasePath, BlockIndex).filename(),
-					 NiceBytes(OldBlockSize - MovedFromBlock));
+					 OldBlockSize > MovedFromBlock ? NiceBytes(OldBlockSize - MovedFromBlock) : 0);
 		}
 		if (TargetFileBuffer)
 		{
diff --git a/src/zenstore/cache/cachedisklayer.cpp b/src/zenstore/cache/cachedisklayer.cpp
index 928165185..fed37824a 100644
--- a/src/zenstore/cache/cachedisklayer.cpp
+++ b/src/zenstore/cache/cachedisklayer.cpp
@@ -3475,10 +3475,6 @@ ZenCacheDiskLayer::CacheBucket::RemoveExpiredData(GcCtx& Ctx, GcStats& Stats)
 		{
 			return nullptr;
 		}
-		if (m_Index.empty())
-		{
-			return nullptr;
-		}
 
 		TotalEntries = m_Index.size();
 
diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp
index 5fca3046c..a5d70a991 100644
--- a/src/zenstore/compactcas.cpp
+++ b/src/zenstore/compactcas.cpp
@@ -871,48 +871,45 @@ public:
 		Stats.CheckedCount			 = m_Cids.size();
 		Stats.FoundCount			 = UnusedCids.size();
 
-		if (UnusedCids.empty())
-		{
-			// Nothing to collect
-			return nullptr;
-		}
-
 		if (!Ctx.Settings.CollectSmallObjects)
 		{
 			return nullptr;
 		}
 
-		if (Ctx.Settings.IsDeleteMode)
+		if (!UnusedCids.empty())
 		{
-			std::vector<CasDiskIndexEntry> ExpiredEntries;
-			ExpiredEntries.reserve(UnusedCids.size());
-
+			if (Ctx.Settings.IsDeleteMode)
 			{
-				RwLock::ExclusiveLockScope __(m_CasContainerStrategy.m_LocationMapLock);
-				if (Ctx.IsCancelledFlag.load())
-				{
-					return nullptr;
-				}
+				std::vector<CasDiskIndexEntry> ExpiredEntries;
+				ExpiredEntries.reserve(UnusedCids.size());
 
-				for (const IoHash& Cid : UnusedCids)
 				{
-					if (auto It = m_CasContainerStrategy.m_LocationMap.find(Cid); It != m_CasContainerStrategy.m_LocationMap.end())
+					RwLock::ExclusiveLockScope __(m_CasContainerStrategy.m_LocationMapLock);
+					if (Ctx.IsCancelledFlag.load())
 					{
-						ExpiredEntries.push_back({.Key		= Cid,
-												  .Location = m_CasContainerStrategy.m_Locations[It->second],
-												  .Flags	= CasDiskIndexEntry::kTombstone});
+						return nullptr;
 					}
-				}
 
-				if (!ExpiredEntries.empty())
-				{
-					for (const CasDiskIndexEntry& Entry : ExpiredEntries)
+					for (const IoHash& Cid : UnusedCids)
+					{
+						if (auto It = m_CasContainerStrategy.m_LocationMap.find(Cid); It != m_CasContainerStrategy.m_LocationMap.end())
+						{
+							ExpiredEntries.push_back({.Key		= Cid,
+													  .Location = m_CasContainerStrategy.m_Locations[It->second],
+													  .Flags	= CasDiskIndexEntry::kTombstone});
+						}
+					}
+
+					if (!ExpiredEntries.empty())
 					{
-						m_CasContainerStrategy.m_LocationMap.erase(Entry.Key);
-						Stats.DeletedCount++;
+						for (const CasDiskIndexEntry& Entry : ExpiredEntries)
+						{
+							m_CasContainerStrategy.m_LocationMap.erase(Entry.Key);
+							Stats.DeletedCount++;
+						}
+						m_CasContainerStrategy.m_CasLog.Append(ExpiredEntries);
+						m_CasContainerStrategy.m_CasLog.Flush();
 					}
-					m_CasContainerStrategy.m_CasLog.Append(ExpiredEntries);
-					m_CasContainerStrategy.m_CasLog.Flush();
 				}
 			}
 		}
diff --git a/src/zenstore/include/zenstore/blockstore.h b/src/zenstore/include/zenstore/blockstore.h
index 5af9aeb10..a98ca5375 100644
--- a/src/zenstore/include/zenstore/blockstore.h
+++ b/src/zenstore/include/zenstore/blockstore.h
@@ -265,10 +265,19 @@ public:
 										  const std::vector<size_t>&			 KeepChunkIndexes,
 										  const std::vector<BlockStoreLocation>& ChunkLocations)> Callback) const
 	{
+		std::vector<uint32_t> BlockOrder;
+		BlockOrder.reserve(m_BlockIndexToChunkMapIndex.size());
 		for (auto It : m_BlockIndexToChunkMapIndex)
 		{
-			size_t ChunkMapIndex = It.second;
-			bool   Continue		 = Callback(It.first, m_KeepChunks[ChunkMapIndex], m_ChunkLocations);
+			BlockOrder.push_back(It.first);
+		}
+		std::sort(BlockOrder.begin(), BlockOrder.end());
+		for (uint32_t BlockIndex : BlockOrder)
+		{
+			auto It = m_BlockIndexToChunkMapIndex.find(BlockIndex);
+			ZEN_ASSERT(It != m_BlockIndexToChunkMapIndex.end());
+			size_t ChunkMapIndex = It->second;
+			bool   Continue		 = Callback(BlockIndex, m_KeepChunks[ChunkMapIndex], m_ChunkLocations);
 			if (!Continue)
 			{
 				break;