[StatsPlugin] Use lock-free list for global stats plugins list

yashykt · Nov 5, 2024 · c1e4af6 · c1e4af6
1 parent 6c05780
commit c1e4af6
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 17 deletions.
diff --git a/src/core/telemetry/metrics.cc b/src/core/telemetry/metrics.cc
@@ -120,42 +120,45 @@ void GlobalStatsPluginRegistry::StatsPluginGroup::AddServerCallTracers(
   }
 }
 
-NoDestruct<Mutex> GlobalStatsPluginRegistry::mutex_;
-NoDestruct<std::vector<std::shared_ptr<StatsPlugin>>>
+std::atomic<GlobalStatsPluginRegistry::GlobalStatsPluginNode*>
     GlobalStatsPluginRegistry::plugins_;
 
 void GlobalStatsPluginRegistry::RegisterStatsPlugin(
     std::shared_ptr<StatsPlugin> plugin) {
-  MutexLock lock(&*mutex_);
-  plugins_->push_back(std::move(plugin));
+  GlobalStatsPluginNode* node = new GlobalStatsPluginNode();
+  node->plugin = std::move(plugin);
+  node->next = plugins_.load(std::memory_order_relaxed);
+  while (!plugins_.compare_exchange_weak(
+      node->next, node, std::memory_order_acq_rel, std::memory_order_relaxed)) {
+  }
 }
 
 GlobalStatsPluginRegistry::StatsPluginGroup
 GlobalStatsPluginRegistry::GetStatsPluginsForChannel(
     const experimental::StatsPluginChannelScope& scope) {
-  MutexLock lock(&*mutex_);
   StatsPluginGroup group;
-  for (const auto& plugin : *plugins_) {
+  for (GlobalStatsPluginNode* node = plugins_.load(std::memory_order_acquire);
+       node != nullptr; node = node->next) {
     bool is_enabled = false;
     std::shared_ptr<StatsPlugin::ScopeConfig> config;
-    std::tie(is_enabled, config) = plugin->IsEnabledForChannel(scope);
+    std::tie(is_enabled, config) = node->plugin->IsEnabledForChannel(scope);
     if (is_enabled) {
-      group.AddStatsPlugin(plugin, std::move(config));
+      group.AddStatsPlugin(node->plugin, std::move(config));
     }
   }
   return group;
 }
 
 GlobalStatsPluginRegistry::StatsPluginGroup
 GlobalStatsPluginRegistry::GetStatsPluginsForServer(const ChannelArgs& args) {
-  MutexLock lock(&*mutex_);
   StatsPluginGroup group;
-  for (const auto& plugin : *plugins_) {
+  for (GlobalStatsPluginNode* node = plugins_.load(std::memory_order_acquire);
+       node != nullptr; node = node->next) {
     bool is_enabled = false;
     std::shared_ptr<StatsPlugin::ScopeConfig> config;
-    std::tie(is_enabled, config) = plugin->IsEnabledForServer(args);
+    std::tie(is_enabled, config) = node->plugin->IsEnabledForServer(args);
     if (is_enabled) {
-      group.AddStatsPlugin(plugin, std::move(config));
+      group.AddStatsPlugin(node->plugin, std::move(config));
     }
   }
   return group;

diff --git a/src/core/telemetry/metrics.h b/src/core/telemetry/metrics.h
@@ -508,13 +508,15 @@ class GlobalStatsPluginRegistry {
   static StatsPluginGroup GetStatsPluginsForServer(const ChannelArgs& args);
 
  private:
+  struct GlobalStatsPluginNode {
+    std::shared_ptr<StatsPlugin> plugin;
+    GlobalStatsPluginNode* next = nullptr;
+  };
   friend class GlobalStatsPluginRegistryTestPeer;
 
   GlobalStatsPluginRegistry() = default;
 
-  static NoDestruct<Mutex> mutex_;
-  static NoDestruct<std::vector<std::shared_ptr<StatsPlugin>>> plugins_
-      ABSL_GUARDED_BY(mutex_);
+  static std::atomic<GlobalStatsPluginNode*> plugins_;
 };
 
 // A metric callback that is registered with a stats plugin group.

diff --git a/test/core/test_util/fake_stats_plugin.h b/test/core/test_util/fake_stats_plugin.h
@@ -696,8 +696,14 @@ class GlobalInstrumentsRegistryTestPeer {
 class GlobalStatsPluginRegistryTestPeer {
  public:
   static void ResetGlobalStatsPluginRegistry() {
-    MutexLock lock(&*GlobalStatsPluginRegistry::mutex_);
-    GlobalStatsPluginRegistry::plugins_->clear();
+    GlobalStatsPluginRegistry::GlobalStatsPluginNode* node =
+        GlobalStatsPluginRegistry::plugins_.exchange(nullptr,
+                                                     std::memory_order_acq_rel);
+    while (node != nullptr) {
+      GlobalStatsPluginRegistry::GlobalStatsPluginNode* next = node->next;
+      delete node;
+      node = next;
+    }
   }
 };