make RemoveExpiredData and PreCache serial to reduce CPU overhead / lock contention (#385)

* make RemoveExpiredData and PreCache serial to reduce CPU overhead / lock contention

1 files changed, 61 insertions, 85 deletions

diff --git a/src/zenstore/gc.cpp b/src/zenstore/gc.cpp
index 5bd34fc37..f6b3dca6f 100644
--- a/src/zenstore/gc.cpp
+++ b/src/zenstore/gc.cpp
@@ -709,7 +709,7 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 		RwLock																		StoreCompactorsLock;
 		std::unordered_map<std::unique_ptr<GcReferenceValidator>, size_t>			ReferenceValidators;
 		RwLock																		ReferenceValidatorsLock;
-		WorkerThreadPool&															PreCachePhaseThreadPool =
+		WorkerThreadPool&															ParallelWorkThreadPool =
 			  Settings.SingleThread ? GetSyncWorkerPool() : GetSmallWorkerPool(EWorkloadType::Background);
 
 		if (!m_GcReferencers.empty())
@@ -721,7 +721,6 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 			ZEN_INFO("GCV2: Removing expired data from {} referencers", m_GcReferencers.size());
 			ZEN_TRACE_CPU("GcV2::RemoveExpiredData");
 
-			Latch WorkLeft(1);
 			{
 				// First remove any cache keys that may own references
 				SCOPED_TIMER(Result.RemoveExpiredDataMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()); if (Ctx.Settings.Verbose) {
@@ -733,56 +732,45 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 				{
 					if (CheckGCCancel())
 					{
-						WorkLeft.CountDown();
-						WorkLeft.Wait();
 						return Sum(Result, true);
 					}
 					GcReferencer*							   Owner = m_GcReferencers[Index];
 					std::pair<std::string, GcReferencerStats>* Stats = &Result.ReferencerStats[Index];
-					WorkLeft.AddCount(1);
-					PreCachePhaseThreadPool.ScheduleWork([this, &Ctx, &WorkLeft, Owner, Stats, &StoreCompactorsLock, &StoreCompactors]() {
-						ZEN_MEMSCOPE(GetGcTag());
-
-						auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-						try
+					try
+					{
+						Stats->first = Owner->GetGcName(Ctx);
+						SCOPED_TIMER(Stats->second.RemoveExpiredDataStats.ElapsedMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+						std::unique_ptr<GcStoreCompactor> StoreCompactor(
+							Owner->RemoveExpiredData(Ctx, Stats->second.RemoveExpiredDataStats));
+						if (StoreCompactor)
 						{
-							Stats->first = Owner->GetGcName(Ctx);
-							SCOPED_TIMER(Stats->second.RemoveExpiredDataStats.ElapsedMS =
-											 std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-							std::unique_ptr<GcStoreCompactor> StoreCompactor(
-								Owner->RemoveExpiredData(Ctx, Stats->second.RemoveExpiredDataStats));
-							if (StoreCompactor)
-							{
-								RwLock::ExclusiveLockScope __(StoreCompactorsLock);
-								StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->second.CompactStoreStats);
-							}
+							RwLock::ExclusiveLockScope __(StoreCompactorsLock);
+							StoreCompactors.insert_or_assign(std::move(StoreCompactor), &Stats->second.CompactStoreStats);
 						}
-						catch (const std::system_error& Ex)
+					}
+					catch (const std::system_error& Ex)
+					{
+						if (IsOOD(Ex) || IsOOM(Ex))
 						{
-							if (IsOOD(Ex) || IsOOM(Ex))
-							{
-								ZEN_WARN("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
-							}
-							else
-							{
-								ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
-							}
-							SetCancelGC(true);
+							ZEN_WARN("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
 						}
-						catch (const std::bad_alloc& Ex)
-						{
-							ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
-							SetCancelGC(true);
-						}
-						catch (const std::exception& Ex)
+						else
 						{
 							ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
-							SetCancelGC(true);
 						}
-					});
+						SetCancelGC(true);
+					}
+					catch (const std::bad_alloc& Ex)
+					{
+						ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
+						SetCancelGC(true);
+					}
+					catch (const std::exception& Ex)
+					{
+						ZEN_ERROR("GCV2: Failed removing expired data for {}. Reason: '{}'", Owner->GetGcName(Ctx), Ex.what());
+						SetCancelGC(true);
+					}
 				}
-				WorkLeft.CountDown();
-				WorkLeft.Wait();
 			}
 		}
 
@@ -827,7 +815,7 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 						GcReferenceStore*							   ReferenceStore = m_GcReferenceStores[Index];
 						std::pair<std::string, GcReferenceStoreStats>* Stats		  = &Result.ReferenceStoreStats[Index];
 						WorkLeft.AddCount(1);
-						PreCachePhaseThreadPool.ScheduleWork(
+						ParallelWorkThreadPool.ScheduleWork(
 							[this, &Ctx, ReferenceStore, Stats, Index, &WorkLeft, &ReferencePrunersLock, &ReferencePruners]() {
 								ZEN_MEMSCOPE(GetGcTag());
 
@@ -925,7 +913,7 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 								GcReferencer*							   Referencer = m_GcReferencers[Index];
 								std::pair<std::string, GcReferencerStats>* Stats	  = &Result.ReferencerStats[Index];
 								WorkLeft.AddCount(1);
-								PreCachePhaseThreadPool.ScheduleWork(
+								ParallelWorkThreadPool.ScheduleWork(
 									[this, &Ctx, &WorkLeft, Referencer, Index, Stats, &ReferenceCheckersLock, &ReferenceCheckers]() {
 										ZEN_MEMSCOPE(GetGcTag());
 
@@ -1031,15 +1019,15 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 							GcReferencer*							   Referencer	   = m_GcReferencers[Index];
 							std::pair<std::string, GcReferencerStats>* ReferemcerStats = &Result.ReferencerStats[Index];
 							WorkLeft.AddCount(1);
-							PreCachePhaseThreadPool.ScheduleWork([this,
-																  &Ctx,
-																  &WorkLeft,
-																  Referencer,
-																  Index,
-																  Result = &Result,
-																  ReferemcerStats,
-																  &ReferenceValidatorsLock,
-																  &ReferenceValidators]() {
+							ParallelWorkThreadPool.ScheduleWork([this,
+																 &Ctx,
+																 &WorkLeft,
+																 Referencer,
+																 Index,
+																 Result = &Result,
+																 ReferemcerStats,
+																 &ReferenceValidatorsLock,
+																 &ReferenceValidators]() {
 								ZEN_MEMSCOPE(GetGcTag());
 
 								auto							   _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
@@ -1117,8 +1105,6 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 					ZEN_INFO("GCV2: Precaching state for {} reference checkers", ReferenceCheckers.size());
 					ZEN_TRACE_CPU("GcV2::PreCache");
 
-					Latch WorkLeft(1);
-
 					{
 						SCOPED_TIMER(Result.PreCacheStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs());
 									 if (Ctx.Settings.Verbose) {
@@ -1130,50 +1116,40 @@ GcManager::CollectGarbage(const GcSettings& Settings)
 						{
 							if (CheckGCCancel())
 							{
-								WorkLeft.CountDown();
-								WorkLeft.Wait();
 								return Sum(Result, true);
 							}
 
 							GcReferenceChecker*						   Checker = It.first.get();
 							size_t									   Index   = It.second;
 							std::pair<std::string, GcReferencerStats>* Stats   = &Result.ReferencerStats[Index];
-							WorkLeft.AddCount(1);
-							PreCachePhaseThreadPool.ScheduleWork([this, &Ctx, Checker, Index, Stats, &WorkLeft]() {
-								ZEN_MEMSCOPE(GetGcTag());
-
-								auto _ = MakeGuard([&WorkLeft]() { WorkLeft.CountDown(); });
-								try
-								{
-									SCOPED_TIMER(Stats->second.PreCacheStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
-									Checker->PreCache(Ctx);
-								}
-								catch (const std::system_error& Ex)
-								{
-									if (IsOOD(Ex) || IsOOM(Ex))
-									{
-										ZEN_WARN("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
-									}
-									else
-									{
-										ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
-									}
-									SetCancelGC(true);
-								}
-								catch (const std::bad_alloc& Ex)
+							try
+							{
+								SCOPED_TIMER(Stats->second.PreCacheStateMS = std::chrono::milliseconds(Timer.GetElapsedTimeMs()););
+								Checker->PreCache(Ctx);
+							}
+							catch (const std::system_error& Ex)
+							{
+								if (IsOOD(Ex) || IsOOM(Ex))
 								{
-									ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
-									SetCancelGC(true);
+									ZEN_WARN("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
 								}
-								catch (const std::exception& Ex)
+								else
 								{
 									ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
-									SetCancelGC(true);
 								}
-							});
+								SetCancelGC(true);
+							}
+							catch (const std::bad_alloc& Ex)
+							{
+								ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
+								SetCancelGC(true);
+							}
+							catch (const std::exception& Ex)
+							{
+								ZEN_ERROR("GCV2: Failed precaching for {}. Reason: '{}'", Checker->GetGcName(Ctx), Ex.what());
+								SetCancelGC(true);
+							}
 						}
-						WorkLeft.CountDown();
-						WorkLeft.Wait();
 					}
 				}