reduce work when there are no blocks to compact (#558)

* reduce work when there are no blocks to compact
* fix lock scopes

1 files changed, 54 insertions, 51 deletions

diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp
index 1d608be8d..02db5f848 100644
--- a/src/zenstore/compactcas.cpp
+++ b/src/zenstore/compactcas.cpp
@@ -604,70 +604,73 @@ public:
 					m_CasContainerStrategy.m_BlockStore.GetBlocksToCompact(BlockUsage, Ctx.Settings.CompactBlockUsageThresholdPercent);
 				BlockCompactState.IncludeBlocks(BlocksToCompact);
 
+				if (BlocksToCompact.size() > 0)
 				{
-					RwLock::SharedLockScope __(m_CasContainerStrategy.m_LocationMapLock);
-					for (const auto& Entry : m_CasContainerStrategy.m_LocationMap)
 					{
-						size_t						  Index = Entry.second;
-						const BlockStoreDiskLocation& Loc	= m_CasContainerStrategy.m_Locations[Index];
-
-						if (!BlockCompactState.AddKeepLocation(Loc.Get(m_CasContainerStrategy.m_PayloadAlignment)))
+						RwLock::SharedLockScope __(m_CasContainerStrategy.m_LocationMapLock);
+						for (const auto& Entry : m_CasContainerStrategy.m_LocationMap)
 						{
-							continue;
+							size_t						  Index = Entry.second;
+							const BlockStoreDiskLocation& Loc	= m_CasContainerStrategy.m_Locations[Index];
+
+							if (!BlockCompactState.AddKeepLocation(Loc.Get(m_CasContainerStrategy.m_PayloadAlignment)))
+							{
+								continue;
+							}
+							BlockCompactStateKeys.push_back(Entry.first);
 						}
-						BlockCompactStateKeys.push_back(Entry.first);
 					}
-				}
 
-				if (Ctx.Settings.IsDeleteMode)
-				{
-					if (Ctx.Settings.Verbose)
+					if (Ctx.Settings.IsDeleteMode)
 					{
-						ZEN_INFO("GCV2: compactcas [COMPACT] '{}': compacting {} blocks",
-								 m_CasContainerStrategy.m_RootDirectory / m_CasContainerStrategy.m_ContainerBaseName,
-								 BlocksToCompact.size());
-					}
-
-					m_CasContainerStrategy.m_BlockStore.CompactBlocks(
-						BlockCompactState,
-						m_CasContainerStrategy.m_PayloadAlignment,
-						[&](const BlockStore::MovedChunksArray& MovedArray, uint64_t FreedDiskSpace) {
-							std::vector<CasDiskIndexEntry> MovedEntries;
-							RwLock::ExclusiveLockScope	   _(m_CasContainerStrategy.m_LocationMapLock);
-							for (const std::pair<size_t, BlockStoreLocation>& Moved : MovedArray)
-							{
-								size_t		  ChunkIndex = Moved.first;
-								const IoHash& Key		 = BlockCompactStateKeys[ChunkIndex];
+						if (Ctx.Settings.Verbose)
+						{
+							ZEN_INFO("GCV2: compactcas [COMPACT] '{}': compacting {} blocks",
+									 m_CasContainerStrategy.m_RootDirectory / m_CasContainerStrategy.m_ContainerBaseName,
+									 BlocksToCompact.size());
+						}
 
-								if (auto It = m_CasContainerStrategy.m_LocationMap.find(Key);
-									It != m_CasContainerStrategy.m_LocationMap.end())
+						m_CasContainerStrategy.m_BlockStore.CompactBlocks(
+							BlockCompactState,
+							m_CasContainerStrategy.m_PayloadAlignment,
+							[&](const BlockStore::MovedChunksArray& MovedArray, uint64_t FreedDiskSpace) {
+								std::vector<CasDiskIndexEntry> MovedEntries;
+								RwLock::ExclusiveLockScope	   _(m_CasContainerStrategy.m_LocationMapLock);
+								for (const std::pair<size_t, BlockStoreLocation>& Moved : MovedArray)
 								{
-									BlockStoreDiskLocation&	  Location	  = m_CasContainerStrategy.m_Locations[It->second];
-									const BlockStoreLocation& OldLocation = BlockCompactState.GetLocation(ChunkIndex);
-									if (Location.Get(m_CasContainerStrategy.m_PayloadAlignment) != OldLocation)
+									size_t		  ChunkIndex = Moved.first;
+									const IoHash& Key		 = BlockCompactStateKeys[ChunkIndex];
+
+									if (auto It = m_CasContainerStrategy.m_LocationMap.find(Key);
+										It != m_CasContainerStrategy.m_LocationMap.end())
 									{
-										// Someone has moved our chunk so lets just skip the new location we were provided, it will be GC:d
-										// at a later time
-										continue;
+										BlockStoreDiskLocation&	  Location	  = m_CasContainerStrategy.m_Locations[It->second];
+										const BlockStoreLocation& OldLocation = BlockCompactState.GetLocation(ChunkIndex);
+										if (Location.Get(m_CasContainerStrategy.m_PayloadAlignment) != OldLocation)
+										{
+											// Someone has moved our chunk so lets just skip the new location we were provided, it will be
+											// GC:d at a later time
+											continue;
+										}
+										const BlockStoreLocation& NewLocation = Moved.second;
+
+										Location = BlockStoreDiskLocation(NewLocation, m_CasContainerStrategy.m_PayloadAlignment);
+										MovedEntries.push_back(CasDiskIndexEntry{.Key = Key, .Location = Location});
 									}
-									const BlockStoreLocation& NewLocation = Moved.second;
-
-									Location = BlockStoreDiskLocation(NewLocation, m_CasContainerStrategy.m_PayloadAlignment);
-									MovedEntries.push_back(CasDiskIndexEntry{.Key = Key, .Location = Location});
 								}
-							}
-							m_CasContainerStrategy.m_CasLog.Append(MovedEntries);
-							Stats.RemovedDisk += FreedDiskSpace;
-						},
-						ClaimDiskReserveCallback);
-				}
-				else
-				{
-					if (Ctx.Settings.Verbose)
+								m_CasContainerStrategy.m_CasLog.Append(MovedEntries);
+								Stats.RemovedDisk += FreedDiskSpace;
+							},
+							ClaimDiskReserveCallback);
+					}
+					else
 					{
-						ZEN_INFO("GCV2: compactcas [COMPACT] '{}': skipped compacting of {} eligible blocks",
-								 m_CasContainerStrategy.m_RootDirectory / m_CasContainerStrategy.m_ContainerBaseName,
-								 BlocksToCompact.size());
+						if (Ctx.Settings.Verbose)
+						{
+							ZEN_INFO("GCV2: compactcas [COMPACT] '{}': skipped compacting of {} eligible blocks",
+									 m_CasContainerStrategy.m_RootDirectory / m_CasContainerStrategy.m_ContainerBaseName,
+									 BlocksToCompact.size());
+						}
 					}
 				}
 			}