Merge branch 'main' of https://github.com/EpicGames/zen

3 files changed, 29 insertions, 8 deletions

diff --git a/zenstore/CAS.cpp b/zenstore/CAS.cpp
index eaf72cb41..1db2b50bf 100644
--- a/zenstore/CAS.cpp
+++ b/zenstore/CAS.cpp
@@ -50,7 +50,7 @@ CasChunkSet::RemoveChunksIf(std::function<bool(const IoHash& CandidateHash)>&& P
 void
 CasChunkSet::IterateChunks(std::function<void(const IoHash& ChunkHash)>&& Callback)
 {
-	for (auto It = begin(m_ChunkSet), ItEnd = end(m_ChunkSet); It != ItEnd;)
+	for (auto It = begin(m_ChunkSet), ItEnd = end(m_ChunkSet); It != ItEnd; ++It)
 	{
 		Callback(*It);
 	}
diff --git a/zenstore/cidstore.cpp b/zenstore/cidstore.cpp
index 08a3192ff..d76058bd1 100644
--- a/zenstore/cidstore.cpp
+++ b/zenstore/cidstore.cpp
@@ -45,11 +45,23 @@ struct CidStore::Impl
 		ZEN_ASSERT(Compressed != IoHash::Zero);
 
 		RwLock::ExclusiveLockScope _(m_Lock);
-		m_CidMap.insert_or_assign(DecompressedId, Compressed);
-		// TODO: it's pretty wasteful to log even idempotent updates
-		// however we can't simply use the boolean returned by insert_or_assign
-		// since there's not a 1:1 mapping between compressed and uncompressed
-		// so if we want a last-write-wins policy then we have to log each update
+
+        auto It = m_CidMap.try_emplace(DecompressedId, Compressed);
+		if (!It.second)
+		{
+			if (It.first.value() != Compressed)
+			{
+				It.first.value() = Compressed;
+			}
+			else
+			{
+				// No point logging an update that won't change anything
+				return;
+			}
+		}
+
+		// It's not ideal to do this while holding the lock in case
+		// we end up in blocking I/O but that's for later
 		LogMapping(DecompressedId, Compressed);
 	}
 
@@ -68,6 +80,10 @@ struct CidStore::Impl
 			{
 				CompressedHash = It->second;
 			}
+			else
+			{
+				return {};
+			}
 		}
 
 		ZEN_ASSERT(CompressedHash != IoHash::Zero);
@@ -84,7 +100,7 @@ struct CidStore::Impl
 
 		if (It == m_CidMap.end())
 		{
-			// Not in map, or tombstone
+			// Not in map
 			return false;
 		}
 
@@ -171,7 +187,7 @@ struct CidStore::Impl
 					const IoHash& BadHash = It->first;
 
 					// Log a tombstone record
-					m_LogFile.Append({.Uncompressed = BadHash, .Compressed = IoHash::Zero});
+					LogMapping(BadHash, IoHash::Zero);
 
 					BadChunks.push_back(BadHash);
 
diff --git a/zenstore/include/zenstore/cidstore.h b/zenstore/include/zenstore/cidstore.h
index f4439e083..2eea04164 100644
--- a/zenstore/include/zenstore/cidstore.h
+++ b/zenstore/include/zenstore/cidstore.h
@@ -25,6 +25,11 @@ class IoBuffer;
  * be used to deal with other kinds of indirections in the future. For example, if we want
  * to support chunking then a CID may represent a list of chunks which could be concatenated
  * to form the referenced chunk.
+ * 
+ * It would likely be possible to implement this mapping in a more efficient way if we
+ * integrate it into the CAS store itself, so we can avoid maintaining copies of large 
+ * hashes in multiple locations. This would also allow us to consolidate commit logs etc
+ * which would be more resilient than the current split log scheme
  *
  */
 class CidStore