Eliminated use of ATL in StructuredCacheStore implementation

1 files changed, 39 insertions, 41 deletions

diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index 4f16b81f2..5e93ebaa9 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -22,8 +22,6 @@
 #include <gsl/gsl-lite.hpp>
 #include <unordered_map>
 
-#include <atlfile.h>
-
 //////////////////////////////////////////////////////////////////////////
 
 namespace zen {
@@ -292,10 +290,6 @@ struct ZenCacheDiskLayer::CacheBucket
 	static bool Delete(std::filesystem::path BucketDir);
 
 	bool Get(const IoHash& HashKey, ZenCacheValue& OutValue);
-
-	bool GetStandaloneCacheValue(const IoHash& HashKey, ZenCacheValue& OutValue, const DiskLocation& Loc);
-
-	bool GetInlineCacheValue(const DiskLocation& Loc, ZenCacheValue& OutValue);
 	void Put(const IoHash& HashKey, const ZenCacheValue& Value);
 	void Drop();
 	void Flush();
@@ -310,15 +304,19 @@ private:
 	bool				  m_Ok					 = false;
 	uint64_t			  m_LargeObjectThreshold = 64 * 1024;
 
+	// These files are used to manage storage of small objects for this bucket
+
 	BasicFile					m_SobsFile;
 	TCasLogFile<DiskIndexEntry> m_SlogFile;
 
-	void BuildPath(WideStringBuilderBase& Path, const IoHash& HashKey);
-	void PutLargeObject(const IoHash& HashKey, const ZenCacheValue& Value);
-
 	RwLock												 m_IndexLock;
 	tsl::robin_map<IoHash, DiskLocation, IoHash::Hasher> m_Index;
 	uint64_t											 m_WriteCursor = 0;
+
+	void BuildPath(WideStringBuilderBase& Path, const IoHash& HashKey);
+	void PutLargeObject(const IoHash& HashKey, const ZenCacheValue& Value);
+	bool GetStandaloneCacheValue(const IoHash& HashKey, ZenCacheValue& OutValue, const DiskLocation& Loc);
+	bool GetInlineCacheValue(const DiskLocation& Loc, ZenCacheValue& OutValue);
 };
 
 ZenCacheDiskLayer::CacheBucket::CacheBucket(CasStore& Cas) : m_CasStore(Cas)
@@ -353,27 +351,24 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir)
 	std::filesystem::path SobsPath{m_BucketDir / "zen.sobs"};
 	std::filesystem::path SlogPath{m_BucketDir / "zen.slog"};
 
-	CAtlFile ManifestFile;
+	BasicFile ManifestFile;
 
 	// Try opening existing manifest file first
 
 	bool IsNew = false;
 
-	HRESULT hRes = ManifestFile.Create(ManifestPath.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, OPEN_EXISTING);
+	std::error_code Ec;
+	ManifestFile.Open(ManifestPath, /* IsCreate */ false, Ec);
 
-	if (SUCCEEDED(hRes))
+	if (!Ec)
 	{
-		ULONGLONG FileSize;
-		ManifestFile.GetSize(FileSize);
+		uint64_t FileSize = ManifestFile.FileSize();
 
 		if (FileSize == sizeof(Oid))
 		{
-			hRes = ManifestFile.Read(&m_BucketId, sizeof(Oid));
+			ManifestFile.Read(&m_BucketId, sizeof(Oid), 0);
 
-			if (SUCCEEDED(hRes))
-			{
-				m_Ok = true;
-			}
+			m_Ok = true;
 		}
 
 		if (!m_Ok)
@@ -386,16 +381,16 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir)
 	{
 		// No manifest file found, this is a new bucket
 
-		hRes = ManifestFile.Create(ManifestPath.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, CREATE_ALWAYS);
+		ManifestFile.Open(ManifestPath, /* IsCreate */ true, Ec);
 
-		if (FAILED(hRes))
+		if (Ec)
 		{
-			ThrowLastError("Failed to create bucket manifest '{}'"_format(ManifestPath));
+			throw std::system_error(Ec, "Failed to create bucket manifest '{}'"_format(ManifestPath));
 		}
 
 		m_BucketId.Generate();
 
-		hRes = ManifestFile.Write(&m_BucketId, sizeof(Oid));
+		ManifestFile.Write(&m_BucketId, sizeof(Oid), /* FileOffset */ 0);
 
 		IsNew = true;
 	}
@@ -565,7 +560,7 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx)
 {
 	std::vector<DiskIndexEntry> StandaloneFiles;
 
-	std::vector<IoHash> BadChunks;
+	std::vector<IoHash>	  BadChunks;
 	std::vector<IoBuffer> BadStandaloneChunks;
 
 	{
@@ -621,35 +616,38 @@ ZenCacheDiskLayer::CacheBucket::PutLargeObject(const IoHash& HashKey, const ZenC
 	WideStringBuilder<128> DataFilePath;
 	BuildPath(DataFilePath, HashKey);
 
-	// TODO: replace this process with a more efficient implementation with proper atomic rename
-	// and also avoid creating directories if we can
-
-	std::filesystem::path ParentPath = std::filesystem::path(DataFilePath.c_str()).parent_path();
-	CreateDirectories(ParentPath);
+	TemporaryFile DataFile;
 
-	CAtlTemporaryFile DataFile;
+	std::error_code Ec;
+	DataFile.CreateTemporary(m_BucketDir.c_str(), Ec);
 
-	HRESULT hRes = DataFile.Create(m_BucketDir.c_str());
-
-	if (FAILED(hRes))
+	if (Ec)
 	{
-		ThrowSystemException(hRes, "Failed to open temporary file for put at '{}'"_format(m_BucketDir));
+		throw std::system_error(Ec, "Failed to open temporary file for put at '{}'"_format(m_BucketDir));
 	}
 
-	hRes = DataFile.Write(Value.Value.Data(), gsl::narrow<DWORD>(Value.Value.Size()));
+	DataFile.WriteAll(Value.Value, Ec);
 
-	if (FAILED(hRes))
+	if (Ec)
 	{
-		ThrowSystemException(hRes, "Failed to write payload ({} bytes) to file"_format(NiceBytes(Value.Value.Size())));
+		throw std::system_error(Ec, "Failed to write payload ({} bytes) to file"_format(NiceBytes(Value.Value.Size())));
 	}
 
-	// Move file into place (note: not fully atomic!)
+	// Move file into place (atomically)
 
-	hRes = DataFile.Close(DataFilePath.c_str());
+	DataFile.MoveTemporaryIntoPlace(DataFilePath.c_str(), Ec);
 
-	if (FAILED(hRes))
+	if (Ec)
 	{
-		ThrowSystemException(hRes, "Failed to finalize file '{}'"_format(WideToUtf8(DataFilePath)));
+		std::filesystem::path ParentPath = std::filesystem::path(DataFilePath.c_str()).parent_path();
+		CreateDirectories(ParentPath);
+
+		DataFile.MoveTemporaryIntoPlace(DataFilePath.c_str(), Ec);
+
+		if (Ec)
+		{
+			throw std::system_error(Ec, "Failed to finalize file '{}'"_format(WideToUtf8(DataFilePath)));
+		}
 	}
 
 	// Update index