From 5e43b80df4cf8fff6bd350139783fb15d9d25207 Mon Sep 17 00:00:00 2001
From: Dan Engelbrecht <dan.engelbrecht@epicgames.com>
Date: Thu, 7 Apr 2022 23:34:15 +0200
Subject: correct expire vs contribute

---
 zenserver/cache/structuredcachestore.cpp | 19 +++++++++----------
 zenserver/cache/structuredcachestore.h   |  4 +++-
 zenstore/cidstore.cpp                    | 23 +++++++++++------------
 zenstore/gc.cpp                          |  9 +--------
 zenstore/include/zenstore/gc.h           |  3 +--
 5 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index 5c71ad7bb..d28964502 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -1694,21 +1694,20 @@ ZenCacheDiskLayer::CacheBucket::GatherReferences(GcContext& GcCtx)
 
 	std::vector<IoHash> ExpiredKeys;
 	ExpiredKeys.reserve(1024);
+
+	std::vector<IoHash> Cids;
+	Cids.reserve(1024);
+
 	for (const auto& Entry : Index)
 	{
+		const IoHash& Key = Entry.first;
 		if (Entry.second.LastAccess < ExpireTicks)
 		{
-			ExpiredKeys.push_back(Entry.first);
+			ExpiredKeys.push_back(Key);
+			continue;
 		}
-	}
 
-	std::vector<IoHash> Cids;
-	Cids.reserve(1024);
-
-	for (const auto& Key : ExpiredKeys)
-	{
-		IndexEntry&			Entry = Index[Key];
-		const DiskLocation& Loc	  = Entry.Location;
+		const DiskLocation& Loc = Entry.second.Location;
 
 		if (Loc.IsFlagSet(DiskLocation::kStructured))
 		{
@@ -1743,7 +1742,7 @@ ZenCacheDiskLayer::CacheBucket::GatherReferences(GcContext& GcCtx)
 	}
 
 	GcCtx.ContributeCids(Cids);
-	GcCtx.ContributeCacheKeys(m_BucketName, {}, std::move(ExpiredKeys));
+	GcCtx.ContributeCacheKeys(m_BucketName, std::move(ExpiredKeys));
 }
 
 void
diff --git a/zenserver/cache/structuredcachestore.h b/zenserver/cache/structuredcachestore.h
index 8e8b6ee78..c107983b5 100644
--- a/zenserver/cache/structuredcachestore.h
+++ b/zenserver/cache/structuredcachestore.h
@@ -271,7 +271,9 @@ private:
 			IndexEntry() : Location(), LastAccess() {}
 			IndexEntry(const DiskLocation& Loc, const int64_t Timestamp) : Location(Loc), LastAccess(Timestamp) {}
 			IndexEntry(const IndexEntry& E) : Location(E.Location), LastAccess(E.LastAccess.load(std::memory_order_relaxed)) {}
-			IndexEntry(IndexEntry&& E) : Location(std::move(E.Location)), LastAccess(E.LastAccess.load(std::memory_order_relaxed)) {}
+			IndexEntry(IndexEntry&& E) noexcept : Location(std::move(E.Location)), LastAccess(E.LastAccess.load(std::memory_order_relaxed))
+			{
+			}
 
 			IndexEntry& operator=(const IndexEntry& E) { return *this = IndexEntry(E); }
 			IndexEntry& operator=(IndexEntry&& E) noexcept
diff --git a/zenstore/cidstore.cpp b/zenstore/cidstore.cpp
index 509d21abe..6cf9ee734 100644
--- a/zenstore/cidstore.cpp
+++ b/zenstore/cidstore.cpp
@@ -234,23 +234,22 @@ struct CidStore::Impl
 
 	void RemoveCids(CasChunkSet& CasChunks)
 	{
-		RwLock::ExclusiveLockScope _(m_Lock);
-
-		for (auto It = m_CidMap.begin(), End = m_CidMap.end(); It != End;)
+		std::vector<IndexEntry> RemovedEntries;
+		RemovedEntries.reserve(CasChunks.GetSize());
 		{
-			if (CasChunks.ContainsChunk(It->second))
-			{
-				const IoHash& BadHash = It->first;
-
-				// Log a tombstone record
-				LogMapping(BadHash, IoHash::Zero);
-				It = m_CidMap.erase(It);
-			}
-			else
+			RwLock::SharedLockScope _(m_Lock);
+			for (auto It = m_CidMap.begin(), End = m_CidMap.end(); It != End;)
 			{
+				if (CasChunks.ContainsChunk(It->second))
+				{
+					RemovedEntries.push_back({It->first, IoHash::Zero});
+					It = m_CidMap.erase(It);
+					continue;
+				}
 				++It;
 			}
 		}
+		m_LogFile.Append(RemovedEntries);
 	}
 
 	uint64_t m_LastScrubTime = 0;
diff --git a/zenstore/gc.cpp b/zenstore/gc.cpp
index 856f9af02..21522e46a 100644
--- a/zenstore/gc.cpp
+++ b/zenstore/gc.cpp
@@ -212,9 +212,8 @@ GcContext::ContributeCas(std::span<const IoHash> Cas)
 }
 
 void
-GcContext::ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash> ValidKeys, std::vector<IoHash> ExpiredKeys)
+GcContext::ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash>&& ExpiredKeys)
 {
-	m_State->m_CacheBuckets[Bucket].ValidKeys	= std::move(ValidKeys);
 	m_State->m_CacheBuckets[Bucket].ExpiredKeys = std::move(ExpiredKeys);
 }
 
@@ -254,12 +253,6 @@ GcContext::DeletedCas()
 	return m_State->m_DeletedCasChunks;
 }
 
-std::span<const IoHash>
-GcContext::ValidCacheKeys(const std::string& Bucket) const
-{
-	return m_State->m_CacheBuckets[Bucket].ValidKeys;
-}
-
 std::span<const IoHash>
 GcContext::ExpiredCacheKeys(const std::string& Bucket) const
 {
diff --git a/zenstore/include/zenstore/gc.h b/zenstore/include/zenstore/gc.h
index bc8dee9a3..6268588ec 100644
--- a/zenstore/include/zenstore/gc.h
+++ b/zenstore/include/zenstore/gc.h
@@ -53,7 +53,7 @@ public:
 
 	void ContributeCids(std::span<const IoHash> Cid);
 	void ContributeCas(std::span<const IoHash> Hash);
-	void ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash> ValidKeys, std::vector<IoHash> ExpiredKeys);
+	void ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash>&& ExpiredKeys);
 
 	void IterateCids(std::function<void(const IoHash&)> Callback);
 
@@ -64,7 +64,6 @@ public:
 	void		 DeletedCas(std::span<const IoHash> Cas);
 	CasChunkSet& DeletedCas();
 
-	std::span<const IoHash> ValidCacheKeys(const std::string& Bucket) const;
 	std::span<const IoHash> ExpiredCacheKeys(const std::string& Bucket) const;
 
 	bool IsDeletionMode() const;
-- 
cgit v1.2.3