Converted nvmCacheState_ to std::optional to simplify NVM cache state handling when NVM cache state is not enabled

Author: victoria-mcgrath

cachelib/allocator/CacheAllocator-inl.h

@@ -46,9 +46,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(Config config)
           [this](Item* it) -> ItemHandle { return acquire(it); })),
       chainedItemLocks_(config_.chainedItemsLockPower,
                         std::make_shared<MurmurHash2>()),
-      cacheCreationTime_{util::getCurrentTimeSec()},
-      nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(),
-                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
+      cacheCreationTime_{util::getCurrentTimeSec()} {
   // TODO(MEMORY_TIER)
   if (std::holds_alternative<FileShmSegmentOpts>(
       memoryTierConfigs[0].getShmTypeOpts())) {
@@ -94,9 +92,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
           [this](Item* it) -> ItemHandle { return acquire(it); })),
       chainedItemLocks_(config_.chainedItemsLockPower,
                         std::make_shared<MurmurHash2>()),
-      cacheCreationTime_{util::getCurrentTimeSec()},
-      nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(),
-                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
+      cacheCreationTime_{util::getCurrentTimeSec()} {
   initCommon(false);
   shmManager_->removeShm(detail::kShmInfoName,
     PosixSysVSegmentOpts(config_.isUsingPosixShm()));
@@ -131,9 +127,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemAttachT, Config config)
           [this](Item* it) -> ItemHandle { return acquire(it); })),
       chainedItemLocks_(config_.chainedItemsLockPower,
                         std::make_shared<MurmurHash2>()),
-      cacheCreationTime_{*metadata_.cacheCreationTime_ref()},
-      nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(),
-                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
+      cacheCreationTime_{*metadata_.cacheCreationTime_ref()} {
   for (auto pid : *metadata_.compactCachePools_ref()) {
     isCompactCachePool_[pid] = true;
   }
@@ -204,7 +198,7 @@ CacheAllocator<CacheTrait>::restoreCCacheManager() {
 
 template <typename CacheTrait>
 void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
-  if (config_.nvmConfig.has_value()) {
+  if (config_.isNvmCacheEnabled()) {
     if (config_.nvmCacheAP) {
       nvmAdmissionPolicy_ = config_.nvmCacheAP;
     } else if (config_.rejectFirstAPNumEntries) {
@@ -227,25 +221,28 @@ void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
 
 template <typename CacheTrait>
 void CacheAllocator<CacheTrait>::initNvmCache(bool dramCacheAttached) {
-  if (!config_.nvmConfig.has_value()) {
+  if (!config_.isNvmCacheEnabled()) {
     return;
   }
 
+  nvmCacheState_.emplace(NvmCacheState(config_.cacheDir, config_.isNvmCacheEncryptionEnabled(),
+                                       config_.isNvmCacheTruncateAllocSizeEnabled()));
+
   // for some usecases that create pools, restoring nvmcache when dram cache
   // is not persisted is not supported.
   const bool shouldDrop = config_.dropNvmCacheOnShmNew && !dramCacheAttached;
 
   // if we are dealing with persistency, cache directory should be enabled
   const bool truncate = config_.cacheDir.empty() ||
-                        nvmCacheState_.shouldStartFresh() || shouldDrop;
+                        nvmCacheState_.value().shouldStartFresh() || shouldDrop;
   if (truncate) {
-    nvmCacheState_.markTruncated();
+    nvmCacheState_.value().markTruncated();
   }
 
   nvmCache_ = std::make_unique<NvmCacheT>(*this, *config_.nvmConfig, truncate,
                                           config_.itemDestructor);
   if (!config_.cacheDir.empty()) {
-    nvmCacheState_.clearPrevState();
+    nvmCacheState_.value().clearPrevState();
   }
 }
 
@@ -3113,7 +3110,7 @@ std::optional<bool> CacheAllocator<CacheTrait>::saveNvmCache() {
     return false;
   }
 
-  nvmCacheState_.markSafeShutDown();
+  nvmCacheState_.value().markSafeShutDown();
   return true;
 }
 
@@ -3310,8 +3307,8 @@ GlobalCacheStats CacheAllocator<CacheTrait>::getGlobalCacheStats() const {
 
   const uint64_t currTime = util::getCurrentTimeSec();
   ret.ramUpTime = currTime - cacheCreationTime_;
-  ret.nvmUpTime = currTime - nvmCacheState_.getCreationTime();
   ret.nvmCacheEnabled = nvmCache_ ? nvmCache_->isEnabled() : false;
+  ret.nvmUpTime = currTime - getNVMCacheCreationTime();
   ret.reaperStats = getReaperStats();
   ret.numActiveHandles = getNumActiveHandles();
 

cachelib/allocator/CacheAllocator.h

@@ -1082,8 +1082,17 @@ class CacheAllocator : public CacheBase {
   //
   // @return  time when the cache was created.
   time_t getCacheCreationTime() const noexcept { return cacheCreationTime_; }
+
+  // unix timestamp when the NVM cache was created. If NVM cahce isn't enaled,
+  // the cache creation time is returned instead.
+  //
+  // @return  time when the NVM cache was created.
   time_t getNVMCacheCreationTime() const {
-    return nvmCacheState_.getCreationTime();
+    auto result = getCacheCreationTime();
+    if (nvmCacheState_.has_value()) {
+      result = nvmCacheState_.value().getCreationTime();
+    }
+    return result;
   }
 
   // Inspects the cache without changing its state.
@@ -1939,7 +1948,7 @@ class CacheAllocator : public CacheBase {
   folly::ThreadLocal<TlsActiveItemRing, DummyTlsActiveItemRingTag> ring_;
 
   // state for the nvmcache
-  NvmCacheState nvmCacheState_;
+  std::optional<NvmCacheState> nvmCacheState_{};
 
   // admission policy for nvmcache
   std::shared_ptr<NvmAdmissionPolicy<CacheT>> nvmAdmissionPolicy_;

cachelib/allocator/CacheAllocatorConfig.h

@@ -94,6 +94,8 @@ class CacheAllocatorConfig {
   // Config for NvmCache. If enabled, cachelib will also make use of flash.
   CacheAllocatorConfig& enableNvmCache(NvmCacheConfig config);
 
+  bool isNvmCacheEnabled() const;
+
   // enable the reject first admission policy through its parameters
   // @param numEntries          the number of entries to track across all splits
   // @param numSplits           the number of splits. we drop a whole split by
@@ -688,6 +690,11 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::enableNvmCache(
   return *this;
 }
 
+template <typename T>
+bool CacheAllocatorConfig<T>::isNvmCacheEnabled() const {
+  return nvmConfig.has_value();
+}
+
 template <typename T>
 CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setNvmCacheAdmissionPolicy(
     std::shared_ptr<NvmAdmissionPolicy<T>> policy) {