Replace atomic_shared_ptr<EventTracker> with unique_ptr in Cache and raw ptr in NVM

Summary:
Every cache operation (find, allocate, insert, remove, eviction) call recordEvent(), which calls getEventTracker(). Previously, getEventTracker() called folly::atomic_shared_ptr<EventTracker>::load(), which returns a shared_ptr — incurring an atomic refcount increment on load and an atomic decrement on destruction. On x86, each of these is a lock xadd instruction that forces cache-line synchronization across cores. This overhead occurs on every cache operation, even when the event is not sampled.

 Since setEventTracker() is called once during initialization (before workers start) and never again, atomic shared pointer semantics are unnecessary. This diff replaces the atomic_shared_ptr with a "cached raw pointer" pattern:

 - Ownership: std::unique_ptr<EventTracker> in CacheBase holds the sole owning reference, set once during init.
 - Navy layer: Engine, EnginePair, Driver, and AbstractCache now take and store non-owning EventTracker* pointers, since lifetime is managed by CacheBase. These are set during NVM cache initialization and invalidated by
 ~CacheAllocator() which destroys NVM cache before ~CacheBase() destroys the EventTracker.

Reviewed By: rlyerly

Differential Revision: D95646744

fbshipit-source-id: 39d2d352e3bf72bc669c9cdf5dfd3fd165178625

Author: pbhandar2

cachelib/allocator/Cache.cpp

@@ -630,10 +630,6 @@ void CacheBase::updateAggregatedPoolStats(const std::string& statPrefix) const {
 }
 
 void CacheBase::setEventTracker(EventTracker::Config&& config) {
-  eventTracker_.store(std::make_shared<EventTracker>(std::move(config)));
-}
-
-std::shared_ptr<EventTracker> CacheBase::getEventTracker() const {
-  return eventTracker_.load();
+  eventTracker_ = std::make_unique<EventTracker>(std::move(config));
 }
 } // namespace facebook::cachelib

cachelib/allocator/Cache.h

@@ -16,9 +16,10 @@
 
 #pragma once
 
-#include <folly/concurrency/AtomicSharedPtr.h>
+#include <folly/lang/Hint.h>
 #include <gtest/gtest_prod.h>
 
+#include <memory>
 #include <mutex>
 #include <unordered_map>
 
@@ -230,7 +231,9 @@ class CacheBase {
   // Whether to aggregate pool stats to reduce ODS counter inflation
   bool aggregatePoolStats_{false};
 
-  std::shared_ptr<EventTracker> getEventTracker() const;
+  FOLLY_ALWAYS_INLINE EventTracker* getEventTracker() const {
+    return eventTracker_.get();
+  }
   virtual void setEventTracker(EventTracker::Config&& config);
 
  protected:
@@ -375,7 +378,7 @@ class CacheBase {
       poolResizeStrategies_;
   std::shared_ptr<PoolOptimizeStrategy> poolOptimizeStrategy_;
 
-  folly::atomic_shared_ptr<EventTracker> eventTracker_;
+  std::unique_ptr<EventTracker> eventTracker_;
 
   // Enable aggregating pool stats
   void enableAggregatePoolStats() { aggregatePoolStats_ = true; }

cachelib/allocator/CacheAllocator.h

@@ -1246,7 +1246,7 @@ class CacheAllocator : public CacheBase {
   folly::F14FastMap<std::string, uint64_t> getEventTrackerStatsMap()
       const override {
     folly::F14FastMap<std::string, uint64_t> eventTrackerStats;
-    if (auto eventTracker = getEventTracker()) {
+    if (auto* eventTracker = getEventTracker()) {
       eventTracker->getStats(eventTrackerStats);
     }
     return eventTrackerStats;
@@ -1261,8 +1261,8 @@ class CacheAllocator : public CacheBase {
 
     // If NVM cache is enabled, also set the event tracker there
     if (nvmCache_ && nvmCache_->isEnabled()) {
-      if (auto eventTracker = getEventTracker()) {
-        nvmCache_->setEventTracker(eventTracker);
+      if (auto* et = getEventTracker()) {
+        nvmCache_->setEventTracker(et);
       }
     }
   }
@@ -1840,7 +1840,7 @@ class CacheAllocator : public CacheBase {
                    Key key,
                    AllocatorApiResult result,
                    EventRecordParams params = {}) const {
-    if (auto eventTracker = getEventTracker()) {
+    if (auto* eventTracker = getEventTracker()) {
       if (eventTracker->sampleKey(key)) {
         EventInfo eventInfo;
         eventInfo.eventTimestamp = util::getCurrentTimeSec();
@@ -2719,9 +2719,9 @@ void CacheAllocator<CacheTrait>::initNvmCache(bool dramCacheAttached) {
                                           config_.itemDestructor, persistParam);
 
   // Set EventTracker dynamically after NvmCache creation
-  if (auto eventTracker = getEventTracker()) {
+  if (auto* et = getEventTracker()) {
     XLOG(INFO) << "Setting event tracker in NVM cache engines.";
-    nvmCache_->setEventTracker(eventTracker);
+    nvmCache_->setEventTracker(et);
   }
   if (!config_.cacheDir.empty()) {
     nvmCacheState_.clearPrevState();

cachelib/allocator/nvmcache/CacheApiWrapper.h

@@ -143,7 +143,7 @@ class CacheAPIWrapperForNvm {
     return cache.getLegacyEventTracker();
   }
 
-  static std::shared_ptr<EventTracker> getEventTracker(C& cache) {
+  static EventTracker* getEventTracker(C& cache) {
     return cache.getEventTracker();
   }
 

cachelib/allocator/nvmcache/NvmCache.h

@@ -274,9 +274,9 @@ class NvmCache {
   }
 
   // Set the EventTracker for all underlying NVM engines
-  void setEventTracker(std::shared_ptr<EventTracker> tracker) {
+  void setEventTracker(EventTracker* tracker) {
     if (navyCache_) {
-      navyCache_->setEventTracker(std::move(tracker));
+      navyCache_->setEventTracker(tracker);
     }
   }
 

cachelib/navy/AbstractCache.h

@@ -51,8 +51,8 @@ class AbstractCache {
  public:
   virtual ~AbstractCache() = default;
 
-  // Set the EventTracker for all underlying engines
-  virtual void setEventTracker(std::shared_ptr<EventTracker> tracker) = 0;
+  // Set the EventTracker for all underlying engines (non-owning pointer)
+  virtual void setEventTracker(EventTracker* tracker) = 0;
 
   // Return true if item is considered a "large item". This is meant to be
   // a very fast check to verify a key & value pair will be considered as

cachelib/navy/driver/Driver.cpp

@@ -395,7 +395,7 @@ void Driver::updateEvictionStats(HashedKey key,
   enginePairs_[selectEnginePair(key)].updateEvictionStats(key, value, lifetime);
 }
 
-void Driver::setEventTracker(std::shared_ptr<EventTracker> tracker) {
+void Driver::setEventTracker(EventTracker* tracker) {
   for (auto& enginePair : enginePairs_) {
     enginePair.setEventTracker(tracker);
   }

cachelib/navy/driver/Driver.h

@@ -177,7 +177,7 @@ class Driver final : public AbstractCache {
                            uint32_t lifetime) override;
 
   // Set the EventTracker for all underlying engines
-  void setEventTracker(std::shared_ptr<EventTracker> tracker) override;
+  void setEventTracker(EventTracker* tracker) override;
 
  private:
   struct ValidConfigTag {};

cachelib/navy/engine/Engine.h

@@ -16,8 +16,6 @@
 
 #pragma once
 
-#include <folly/concurrency/AtomicSharedPtr.h>
-
 #include "cachelib/common/EventTracker.h"
 #include "cachelib/navy/AbstractCache.h"
 #include "cachelib/navy/common/Hash.h"
@@ -30,17 +28,12 @@ class Engine {
  public:
   virtual ~Engine() = default;
 
-  // Set the EventTracker for this engine.
-  // Thread-safe: can be called at runtime while other threads read.
-  void setEventTracker(std::shared_ptr<EventTracker> tracker) {
-    eventTracker_.store(std::move(tracker));
-  }
+  // Set the EventTracker for this engine (non-owning pointer).
+  // Must be called before concurrent access begins.
+  void setEventTracker(EventTracker* tracker) { eventTracker_ = tracker; }
 
   // Get the EventTracker for this engine.
-  // Thread-safe: returns a copy of the shared_ptr atomically.
-  std::shared_ptr<EventTracker> getEventTracker() const {
-    return eventTracker_.load();
-  }
+  EventTracker* getEventTracker() const { return eventTracker_; }
 
   // return the size of usable space
   virtual uint64_t getSize() const = 0;
@@ -104,7 +97,8 @@ class Engine {
   virtual void updateEvictionStats(uint32_t lifetime) = 0;
 
  protected:
-  folly::atomic_shared_ptr<EventTracker> eventTracker_;
+  // Non-owning pointer; lifetime managed by CacheBase.
+  EventTracker* eventTracker_{nullptr};
 };
 } // namespace navy
 } // namespace cachelib

cachelib/navy/engine/EnginePair.cpp

@@ -353,10 +353,10 @@ void EnginePair::updateEvictionStats(HashedKey hk,
   }
 }
 
-void EnginePair::setEventTracker(std::shared_ptr<EventTracker> tracker) {
+void EnginePair::setEventTracker(EventTracker* tracker) {
   // Not setting in BigHash for now since we do not log anything there yet.
   if (largeItemCache_) {
-    largeItemCache_->setEventTracker(std::move(tracker));
+    largeItemCache_->setEventTracker(tracker);
   }
 }