revert 2cacc99

Author: guptask

cachelib/allocator/CacheAllocator-inl.h

@@ -46,7 +46,10 @@ CacheAllocator<CacheTrait>::CacheAllocator(Config config)
                         std::make_shared<MurmurHash2>()),
       movesMap_(kShards),
       moveLock_(kShards),
-      cacheCreationTime_{util::getCurrentTimeSec()} {
+      cacheCreationTime_{util::getCurrentTimeSec()},
+      nvmCacheState_{cacheCreationTime_, config_.cacheDir,
+                     config_.isNvmCacheEncryptionEnabled(),
+                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
 
   if (numTiers_ > 1 || std::holds_alternative<FileShmSegmentOpts>(
       memoryTierConfigs[0].getShmTypeOpts())) {
@@ -134,7 +137,10 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
                         std::make_shared<MurmurHash2>()),
       movesMap_(kShards),
       moveLock_(kShards),
-      cacheCreationTime_{util::getCurrentTimeSec()} {
+      cacheCreationTime_{util::getCurrentTimeSec()},
+      nvmCacheState_{cacheCreationTime_, config_.cacheDir,
+                     config_.isNvmCacheEncryptionEnabled(),
+                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
   initCommon(false);
   shmManager_->removeShm(detail::kShmInfoName,
     PosixSysVSegmentOpts(config_.isUsingPosixShm()));
@@ -172,8 +178,10 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemAttachT, Config config)
                         std::make_shared<MurmurHash2>()),
       movesMap_(kShards),
       moveLock_(kShards),
-      cacheCreationTime_{*metadata_.cacheCreationTime_ref()} {
-  /* TODO - per tier? */
+      cacheCreationTime_{*metadata_.cacheCreationTime_ref()},
+      nvmCacheState_{cacheCreationTime_, config_.cacheDir,
+                     config_.isNvmCacheEncryptionEnabled(),
+                     config_.isNvmCacheTruncateAllocSizeEnabled()} {
   for (auto pid : *metadata_.compactCachePools_ref()) {
     isCompactCachePool_[pid] = true;
   }
@@ -256,7 +264,7 @@ CacheAllocator<CacheTrait>::restoreCCacheManager(TierId tid) {
 
 template <typename CacheTrait>
 void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
-  if (config_.isNvmCacheEnabled()) {
+  if (config_.nvmConfig.has_value()) {
     if (config_.nvmCacheAP) {
       nvmAdmissionPolicy_ = config_.nvmCacheAP;
     } else if (config_.rejectFirstAPNumEntries) {
@@ -282,29 +290,25 @@ void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
 
 template <typename CacheTrait>
 void CacheAllocator<CacheTrait>::initNvmCache(bool dramCacheAttached) {
-  if (!config_.isNvmCacheEnabled()) {
+  if (!config_.nvmConfig.has_value()) {
     return;
   }
 
-  nvmCacheState_.emplace(NvmCacheState(cacheCreationTime_, 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_.value().shouldStartFresh() || shouldDrop;
+                        nvmCacheState_.shouldStartFresh() || shouldDrop;
   if (truncate) {
-    nvmCacheState_.value().markTruncated();
+    nvmCacheState_.markTruncated();
   }
 
   nvmCache_ = std::make_unique<NvmCacheT>(*this, *config_.nvmConfig, truncate,
                                           config_.itemDestructor);
   if (!config_.cacheDir.empty()) {
-    nvmCacheState_.value().clearPrevState();
+    nvmCacheState_.clearPrevState();
   }
 }
 
@@ -3515,7 +3519,7 @@ std::optional<bool> CacheAllocator<CacheTrait>::saveNvmCache() {
     return false;
   }
 
-  nvmCacheState_.value().markSafeShutDown();
+  nvmCacheState_.markSafeShutDown();
   return true;
 }
 
@@ -3717,14 +3721,14 @@ GlobalCacheStats CacheAllocator<CacheTrait>::getGlobalCacheStats() const {
   const uint64_t currTime = util::getCurrentTimeSec();
   ret.cacheInstanceUpTime = currTime - cacheCreationTime_;
   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();
 
   ret.isNewRamCache = cacheCreationTime_ == cacheCreationTime_;
   ret.isNewNvmCache =
-      nvmCacheState_.value().getCreationTime() == cacheCreationTime_;
+      nvmCacheState_.getCreationTime() == cacheCreationTime_;
 
   return ret;
 }

cachelib/allocator/CacheAllocator.h

@@ -1159,17 +1159,8 @@ 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 {
-    auto result = getCacheCreationTime();
-    if (nvmCacheState_.has_value()) {
-      result = nvmCacheState_.value().getCreationTime();
-    }
-    return result;
+    return nvmCacheState_.getCreationTime();
   }
 
   // Inspects the cache without changing its state.
@@ -2198,7 +2189,7 @@ class CacheAllocator : public CacheBase {
   folly::ThreadLocal<TlsActiveItemRing, DummyTlsActiveItemRingTag> ring_;
 
   // state for the nvmcache
-  std::optional<NvmCacheState> nvmCacheState_{};
+  NvmCacheState nvmCacheState_;
 
   // admission policy for nvmcache
   std::shared_ptr<NvmAdmissionPolicy<CacheT>> nvmAdmissionPolicy_;

cachelib/allocator/CacheAllocatorConfig.h

@@ -96,8 +96,6 @@ 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
@@ -717,11 +715,6 @@ 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) {