let ServiceData::trimRegexCache avoid exclusive lock

Summary: Currently, `ServiceData::trimRegexCache` always takes an exclusive lock and invokes `RegexMatchCache::purge`, even if there would be nothing to purge (i.e., all regexes in the cache are fresh / non-stale). It is possible to avoid tail risk of taking an exclusive lock by prefacing with `RegexMatchCache::hasItemsToPurge`, done under an upgrade lock. Reviewed By: mdas7 Differential Revision: D66730361 fbshipit-source-id: 6a8368ba701ef35c1f347e8a61669fb9392825b0
facebook · Dec 4, 2024 · 9fd84b4 · 9fd84b4
1 parent dea6ec5
commit 9fd84b4
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 6 deletions.
diff --git a/fb303/CallbackValuesMap-inl.h b/fb303/CallbackValuesMap-inl.h
@@ -124,6 +124,12 @@ void CallbackValuesMap<T>::clear() {
   detail::cachedClearStrings(*wlock);
 }
 
+template <typename T>
+void CallbackValuesMap<T>::trimRegexCache(
+    const folly::RegexMatchCache::time_point expiry) {
+  detail::cachedTrimStale(callbackMap_, expiry);
+}
+
 template <typename T>
 std::shared_ptr<typename CallbackValuesMap<T>::CallbackEntry>
 CallbackValuesMap<T>::getCallback(folly::StringPiece name) {

diff --git a/fb303/CallbackValuesMap.h b/fb303/CallbackValuesMap.h
@@ -92,9 +92,7 @@ class CallbackValuesMap {
    */
   void clear();
 
-  void trimRegexCache(folly::RegexMatchCache::time_point expiry) {
-    callbackMap_.wlock()->matches.purge(expiry);
-  }
+  void trimRegexCache(folly::RegexMatchCache::time_point expiry);
 
   class CallbackEntry {
    public:

diff --git a/fb303/ServiceData.cpp b/fb303/ServiceData.cpp
@@ -511,7 +511,7 @@ map<string, int64_t> ServiceData::getRegexCountersOptimized(
 void ServiceData::trimRegexCache(const std::chrono::seconds maxstale) {
   const auto now = folly::RegexMatchCache::clock::now();
   const auto expiry = now - maxstale;
-  counters_.wlock()->matches.purge(expiry);
+  detail::cachedTrimStale(counters_, expiry);
   quantileMap_.trimRegexCache(expiry);
   dynamicCounters_.trimRegexCache(expiry);
 }

diff --git a/fb303/detail/QuantileStatMap.h b/fb303/detail/QuantileStatMap.h
@@ -116,8 +116,8 @@ class BasicQuantileStatMap {
     countersWLock->bases.clear();
   }
 
-  void trimRegexCache(folly::RegexMatchCache::time_point expiry) {
-    counters_.wlock()->matches.purge(expiry);
+  void trimRegexCache(const folly::RegexMatchCache::time_point expiry) {
+    detail::cachedTrimStale(counters_, expiry);
   }
 
  private:

diff --git a/fb303/detail/RegexUtil.h b/fb303/detail/RegexUtil.h
@@ -84,4 +84,13 @@ void cachedFindMatches(
   cachedFindMatchesCopyUnderSharedLock(out, r->matches, regex, now);
 }
 
+template <typename SyncMap>
+void cachedTrimStale(
+    SyncMap& map,
+    folly::RegexMatchCache::time_point const expiry) {
+  if (auto ulock = map.ulock(); ulock->matches.hasItemsToPurge(expiry)) {
+    ulock.moveFromUpgradeToWrite()->matches.purge(expiry);
+  }
+}
+
 } // namespace facebook::fb303::detail