Use CounterVisitor for NvmAdmissionPolicy

Summary:
In NvmCache we use counter visitor to collect stats. This allows the function to be ignorant of the implementation of the cache.

In this diff, migrate NvmAdmissionPolicy to CounterVisitor so that it can be migrated later.

Reviewed By: therealgymmy

Differential Revision: D40882985

fbshipit-source-id: 2d83e59ec1721f04b4b2968ed29745cb8a35a7d4

Author: haowu14

cachelib/allocator/CacheAllocator-inl.h

@@ -3660,11 +3660,11 @@ std::unordered_map<std::string, double>
 CacheAllocator<CacheTrait>::getNvmCacheStatsMap() const {
   auto ret = nvmCache_ ? nvmCache_->getStatsMap()
                        : std::unordered_map<std::string, double>{};
+  util::CounterVisitor visitor = [&ret](folly::StringPiece k, double v) {
+    ret[k.str()] = v;
+  };
   if (nvmAdmissionPolicy_) {
-    auto policyStats = nvmAdmissionPolicy_->getCounters();
-    for (const auto& kv : policyStats) {
-      ret[kv.first] = kv.second;
-    }
+    nvmAdmissionPolicy_->getCounters(visitor);
   }
   return ret;
 }

cachelib/allocator/NvmAdmissionPolicy.h

@@ -81,19 +81,29 @@ class NvmAdmissionPolicy {
     return decision;
   }
 
-  // The method that exposes stats.
+  // Deprecated. Please use getCounters(visitor)
   virtual std::unordered_map<std::string, double> getCounters() final {
-    auto ctrs = getCountersImpl();
-    ctrs["ap.called"] = overallCount_.get();
-    ctrs["ap.accepted"] = accepted_.get();
-    ctrs["ap.rejected"] = rejected_.get();
+    std::unordered_map<std::string, double> ctrs;
+    util::CounterVisitor visitor = [&ctrs](folly::StringPiece k, double v) {
+      auto keyStr = k.str();
+      ctrs.insert({std::move(keyStr), v});
+    };
+    getCounters(visitor);
+    return ctrs;
+  }
 
-    auto visitorUs = [&ctrs](folly::StringPiece name, double count) {
-      ctrs[name.toString()] = count / 1000;
+  // The method that exposes stats.
+  virtual void getCounters(const util::CounterVisitor& visitor) final {
+    getCountersImpl(visitor);
+    visitor("ap.called", overallCount_.get());
+    visitor("ap.accepted", accepted_.get());
+    visitor("ap.rejected", rejected_.get());
+
+    auto visitorUs = [&visitor](folly::StringPiece name, double count) {
+      visitor(name.toString(), count / 1000);
     };
     overallLatency_.visitQuantileEstimator(visitorUs, "ap.latency_us");
-    ctrs["ap.ttlRejected"] = ttlRejected_.get();
-    return ctrs;
+    visitor("ap.ttlRejected", ttlRejected_.get());
   }
 
   // Track access for an item.
@@ -130,9 +140,7 @@ class NvmAdmissionPolicy {
   // Implementation specific statistics.
   // Please include a prefix/postfix with the name of implementation to avoid
   // collision with base level stats.
-  virtual std::unordered_map<std::string, double> getCountersImpl() {
-    return {};
-  }
+  virtual void getCountersImpl(const util::CounterVisitor&) {}
 
  private:
   util::PercentileStats overallLatency_;
@@ -184,13 +192,11 @@ class RejectFirstAP final : public NvmAdmissionPolicy<Cache> {
     return trackAndCheckIfSeenBefore(key);
   }
 
-  std::unordered_map<std::string, double> getCountersImpl() final override {
-    std::unordered_map<std::string, double> ctrs;
-    ctrs["ap.reject_first_keys_tracked"] = tracker_.numKeysTracked();
-    ctrs["ap.reject_first_tracking_window_secs"] =
-        tracker_.trackingWindowDurationSecs();
-    ctrs["ap.reject_first_admits_by_dram_hit"] = admitsByDramHits_.get();
-    return ctrs;
+  void getCountersImpl(const util::CounterVisitor& visitor) final override {
+    visitor("ap.reject_first_keys_tracked", tracker_.numKeysTracked());
+    visitor("ap.reject_first_tracking_window_secs",
+            tracker_.trackingWindowDurationSecs());
+    visitor("ap.reject_first_admits_by_dram_hit", admitsByDramHits_.get());
   }
 
  private:

cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp

@@ -76,10 +76,8 @@ struct MockNvmAdmissionPolicy : public NvmAdmissionPolicy<T> {
   virtual bool acceptImpl(const Item&, folly::Range<ChainedItemIter>) override {
     return true;
   }
-  virtual std::unordered_map<std::string, double> getCountersImpl() override {
-    std::unordered_map<std::string, double> ret;
-    ret["nvm_mock_policy"] = 1;
-    return ret;
+  virtual void getCountersImpl(const util::CounterVisitor& visitor) override {
+    visitor("nvm_mock_policy", 1);
   }
 };
 } // namespace

cachelib/allocator/tests/NvmAdmissionPolicyTest.cpp

@@ -26,7 +26,6 @@
 namespace facebook {
 namespace cachelib {
 namespace tests {
-
 class NvmAdmissionPolicyTest : public testing::Test {
  public:
   // Expose the admission policy for testing.
@@ -50,11 +49,10 @@ class MockFailedNvmAdmissionPolicy : public NvmAdmissionPolicy<Cache> {
   MockFailedNvmAdmissionPolicy() = default;
 
  protected:
-  virtual std::unordered_map<std::string, double> getCountersImpl() override {
-    std::unordered_map<std::string, double> ret;
-    ret["nvm_mock_failed_policy"] = 1;
-    return ret;
+  virtual void getCountersImpl(const util::CounterVisitor& v) override {
+    v("nvm_mock_failed_policy", 1);
   }
+
   virtual bool acceptImpl(const Item& it,
                           folly::Range<ChainedItemIter>) override {
     switch (it.getKey().size() % 3) {

cachelib/cachebench/cache/Cache.h

@@ -74,9 +74,7 @@ class RetentionAP final : public NvmAdmissionPolicy<Cache> {
     return true;
   }
 
-  std::unordered_map<std::string, double> getCountersImpl() final override {
-    return {};
-  }
+  virtual void getCountersImpl(const util::CounterVisitor&) final override {}
 
  private:
   const uint32_t retentionThreshold_{0};