clean up GcContributor and GcStorage to be pure interfaces (#485)

10 files changed, 31 insertions, 46 deletions

diff --git a/src/zenserver/cache/structuredcachestore.cpp b/src/zenserver/cache/structuredcachestore.cpp
index 05b4c2e58..786053adc 100644
--- a/src/zenserver/cache/structuredcachestore.cpp
+++ b/src/zenserver/cache/structuredcachestore.cpp
@@ -63,8 +63,7 @@ ZenCacheNamespace::ZenCacheNamespace(GcManager&								  Gc,
 									 const std::filesystem::path&			  RootDir,
 									 bool									  EnableReferenceCaching,
 									 const ZenCacheMemoryLayer::Configuration MemLayerConfig)
-: GcStorage(Gc)
-, GcContributor(Gc)
+: m_Gc(Gc)
 , m_RootDir(RootDir)
 , m_JobQueue(JobQueue)
 , m_MemLayer(m_JobQueue, MemLayerConfig)
@@ -74,10 +73,15 @@ ZenCacheNamespace::ZenCacheNamespace(GcManager&								  Gc,
 	CreateDirectories(RootDir);
 
 	m_DiskLayer.DiscoverBuckets();
+
+	m_Gc.AddGcContributor(this);
+	m_Gc.AddGcStorage(this);
 }
 
 ZenCacheNamespace::~ZenCacheNamespace()
 {
+	m_Gc.RemoveGcStorage(this);
+	m_Gc.RemoveGcContributor(this);
 }
 
 bool
diff --git a/src/zenserver/cache/structuredcachestore.h b/src/zenserver/cache/structuredcachestore.h
index c67b23e93..28b2189ae 100644
--- a/src/zenserver/cache/structuredcachestore.h
+++ b/src/zenserver/cache/structuredcachestore.h
@@ -112,6 +112,7 @@ public:
 	CacheValueDetails::NamespaceDetails GetValueDetails(const std::string_view BucketFilter, const std::string_view ValueFilter) const;
 
 private:
+	GcManager&			  m_Gc;
 	std::filesystem::path m_RootDir;
 	JobQueue&			  m_JobQueue;
 	ZenCacheMemoryLayer	  m_MemLayer;
diff --git a/src/zenserver/projectstore/projectstore.cpp b/src/zenserver/projectstore/projectstore.cpp
index 0010f09f5..430d5f693 100644
--- a/src/zenserver/projectstore/projectstore.cpp
+++ b/src/zenserver/projectstore/projectstore.cpp
@@ -1664,9 +1664,8 @@ ProjectStore::Project::TouchOplog(std::string_view Oplog) const
 //////////////////////////////////////////////////////////////////////////
 
 ProjectStore::ProjectStore(CidStore& Store, std::filesystem::path BasePath, GcManager& Gc, JobQueue& JobQueue)
-: GcStorage(Gc)
-, GcContributor(Gc)
-, m_Log(logging::Get("project"))
+: m_Log(logging::Get("project"))
+, m_Gc(Gc)
 , m_CidStore(Store)
 , m_JobQueue(JobQueue)
 , m_ProjectBasePath(BasePath)
@@ -1674,11 +1673,15 @@ ProjectStore::ProjectStore(CidStore& Store, std::filesystem::path BasePath, GcMa
 {
 	ZEN_INFO("initializing project store at '{}'", m_ProjectBasePath);
 	// m_Log.set_level(spdlog::level::debug);
+	m_Gc.AddGcContributor(this);
+	m_Gc.AddGcStorage(this);
 }
 
 ProjectStore::~ProjectStore()
 {
 	ZEN_INFO("closing project store at '{}'", m_ProjectBasePath);
+	m_Gc.RemoveGcStorage(this);
+	m_Gc.RemoveGcContributor(this);
 }
 
 std::filesystem::path
diff --git a/src/zenserver/projectstore/projectstore.h b/src/zenserver/projectstore/projectstore.h
index 0ed9ef532..5aede88b0 100644
--- a/src/zenserver/projectstore/projectstore.h
+++ b/src/zenserver/projectstore/projectstore.h
@@ -361,6 +361,7 @@ public:
 
 private:
 	spdlog::logger&						m_Log;
+	GcManager&							m_Gc;
 	CidStore&							m_CidStore;
 	JobQueue&							m_JobQueue;
 	std::filesystem::path				m_ProjectBasePath;
diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp
index a6bd71bd7..98ad20b98 100644
--- a/src/zenstore/compactcas.cpp
+++ b/src/zenstore/compactcas.cpp
@@ -114,12 +114,14 @@ namespace {
 
 //////////////////////////////////////////////////////////////////////////
 
-CasContainerStrategy::CasContainerStrategy(GcManager& Gc) : GcStorage(Gc), m_Log(logging::Get("containercas"))
+CasContainerStrategy::CasContainerStrategy(GcManager& Gc) : m_Log(logging::Get("containercas")), m_Gc(Gc)
 {
+	m_Gc.AddGcStorage(this);
 }
 
 CasContainerStrategy::~CasContainerStrategy()
 {
+	m_Gc.RemoveGcStorage(this);
 }
 
 void
diff --git a/src/zenstore/compactcas.h b/src/zenstore/compactcas.h
index c0cbbac32..478a1f78e 100644
--- a/src/zenstore/compactcas.h
+++ b/src/zenstore/compactcas.h
@@ -81,8 +81,9 @@ private:
 
 	spdlog::logger& Log() { return m_Log; }
 
-	std::filesystem::path		   m_RootDirectory;
 	spdlog::logger&				   m_Log;
+	GcManager&					   m_Gc;
+	std::filesystem::path		   m_RootDirectory;
 	uint64_t					   m_PayloadAlignment = 1u << 4;
 	uint64_t					   m_MaxBlockSize	  = 1u << 28;
 	bool						   m_IsInitialized	  = false;
diff --git a/src/zenstore/filecas.cpp b/src/zenstore/filecas.cpp
index fe568a487..d41f449d3 100644
--- a/src/zenstore/filecas.cpp
+++ b/src/zenstore/filecas.cpp
@@ -115,12 +115,14 @@ FileCasStrategy::ShardingHelper::ShardingHelper(const std::filesystem::path& Roo
 
 //////////////////////////////////////////////////////////////////////////
 
-FileCasStrategy::FileCasStrategy(GcManager& Gc) : GcStorage(Gc), m_Log(logging::Get("filecas"))
+FileCasStrategy::FileCasStrategy(GcManager& Gc) : m_Log(logging::Get("filecas")), m_Gc(Gc)
 {
+	m_Gc.AddGcStorage(this);
 }
 
 FileCasStrategy::~FileCasStrategy()
 {
+	m_Gc.RemoveGcStorage(this);
 }
 
 void
diff --git a/src/zenstore/filecas.h b/src/zenstore/filecas.h
index 10c181c0b..ea7ff8e8c 100644
--- a/src/zenstore/filecas.h
+++ b/src/zenstore/filecas.h
@@ -45,9 +45,10 @@ struct FileCasStrategy final : public GcStorage
 	virtual GcStorageSize  StorageSize() const override;
 
 private:
-	void	 MakeIndexSnapshot();
-	uint64_t ReadIndexFile(const std::filesystem::path& IndexPath, uint32_t& OutVersion);
-	uint64_t ReadLog(const std::filesystem::path& LogPath, uint64_t LogPosition);
+	void			MakeIndexSnapshot();
+	uint64_t		ReadIndexFile(const std::filesystem::path& IndexPath, uint32_t& OutVersion);
+	uint64_t		ReadLog(const std::filesystem::path& LogPath, uint64_t LogPosition);
+	spdlog::logger& Log() { return m_Log; }
 
 	struct IndexEntry
 	{
@@ -57,12 +58,12 @@ private:
 
 	CasStore::InsertResult InsertChunkData(const void* ChunkData, size_t ChunkSize, const IoHash& ChunkHash);
 
+	spdlog::logger&		  m_Log;
+	GcManager&			  m_Gc;
 	std::filesystem::path m_RootDirectory;
 	RwLock				  m_Lock;
 	IndexMap			  m_Index;
 	RwLock				  m_ShardLocks[256];  // TODO: these should be spaced out so they don't share cache lines
-	spdlog::logger&		  m_Log;
-	spdlog::logger&		  Log() { return m_Log; }
 	std::atomic_uint64_t  m_TotalSize{};
 	bool				  m_IsInitialized = false;
 
diff --git a/src/zenstore/gc.cpp b/src/zenstore/gc.cpp
index 3d8e66a8b..7396fcf65 100644
--- a/src/zenstore/gc.cpp
+++ b/src/zenstore/gc.cpp
@@ -318,30 +318,6 @@ GcContext::ClaimGCReserve()
 
 //////////////////////////////////////////////////////////////////////////
 
-GcContributor::GcContributor(GcManager& Gc) : m_Gc(Gc)
-{
-	m_Gc.AddGcContributor(this);
-}
-
-GcContributor::~GcContributor()
-{
-	m_Gc.RemoveGcContributor(this);
-}
-
-//////////////////////////////////////////////////////////////////////////
-
-GcStorage::GcStorage(GcManager& Gc) : m_Gc(Gc)
-{
-	m_Gc.AddGcStorage(this);
-}
-
-GcStorage::~GcStorage()
-{
-	m_Gc.RemoveGcStorage(this);
-}
-
-//////////////////////////////////////////////////////////////////////////
-
 GcManager::GcManager() : m_Log(logging::Get("gc"))
 {
 }
diff --git a/src/zenstore/include/zenstore/gc.h b/src/zenstore/include/zenstore/gc.h
index 1c455abe5..ffdc8f066 100644
--- a/src/zenstore/include/zenstore/gc.h
+++ b/src/zenstore/include/zenstore/gc.h
@@ -100,13 +100,10 @@ private:
 class GcContributor
 {
 public:
-	GcContributor(GcManager& Gc);
-	~GcContributor();
-
 	virtual void GatherReferences(GcContext& GcCtx) = 0;
 
 protected:
-	GcManager& m_Gc;
+	virtual ~GcContributor() {}
 };
 
 struct GcStorageSize
@@ -120,15 +117,12 @@ struct GcStorageSize
 class GcStorage
 {
 public:
-	GcStorage(GcManager& Gc);
-	~GcStorage();
-
 	virtual void		  ScrubStorage(ScrubContext& ScrubCtx) = 0;
 	virtual void		  CollectGarbage(GcContext& GcCtx)	   = 0;
 	virtual GcStorageSize StorageSize() const				   = 0;
 
-private:
-	GcManager& m_Gc;
+protected:
+	virtual ~GcStorage() {}
 };
 
 /** Interface for querying if we are running low on disk space, used to deny put/writes to disk