From ac68624c602e969093551cec7bbca88c658e9848 Mon Sep 17 00:00:00 2001
From: Mo Chen <mochen@apache.org>
Date: Tue, 18 Aug 2026 17:41:13 -0500
Subject: [PATCH] Metrics: Revert added locking and annotations

The locking that the thread safety annotations required on the
ts::Metrics::Storage read paths introduces too much of a performance
regression.  Revert it for now, until a better solution can be found.

The thread safety analysis infrastructure and its use in
SSLOriginSessionCache are unaffected.
---
 include/tsutil/Metrics.h | 18 ++++++++----------
 src/tsutil/Metrics.cc    | 16 +++++++---------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/include/tsutil/Metrics.h b/include/tsutil/Metrics.h
index 4f47fd5485f..630f079b4d4 100644
--- a/include/tsutil/Metrics.h
+++ b/include/tsutil/Metrics.h
@@ -38,7 +38,6 @@
 #include "swoc/MemSpan.h"
 
 #include "tsutil/Assert.h"
-#include "tsutil/TsMutex.h"
 
 namespace ts
 {
@@ -305,17 +304,17 @@ class Metrics
 
   class Storage
   {
-    BlobStorage _blobs   TS_GUARDED_BY(_mutex);
-    uint16_t _cur_blob   TS_GUARDED_BY(_mutex) = 0;
-    uint16_t _cur_off    TS_GUARDED_BY(_mutex) = 0;
-    LookupTable _lookups TS_GUARDED_BY(_mutex);
-    mutable ts::mutex    _mutex;
+    BlobStorage        _blobs;
+    uint16_t           _cur_blob = 0;
+    uint16_t           _cur_off  = 0;
+    LookupTable        _lookups;
+    mutable std::mutex _mutex;
 
   public:
     Storage(const Storage &)            = delete;
     Storage &operator=(const Storage &) = delete;
 
-    Storage() TS_NO_THREAD_SAFETY_ANALYSIS // single-threaded construction; not yet shared
+    Storage()
     {
       _blobs[0] = std::make_unique<NamesAndAtomics>();
       release_assert(_blobs[0]);
@@ -326,7 +325,7 @@ class Metrics
     ~Storage() {}
 
     IdType           create(const std::string_view name, const MetricType type = MetricType::COUNTER);
-    void             addBlob() TS_REQUIRES(_mutex);
+    void             addBlob();
     IdType           lookup(const std::string_view name) const;
     AtomicType      *lookup(const std::string_view name, IdType *out_id, MetricType *out_type = nullptr) const;
     AtomicType      *lookup(Metrics::IdType id, std::string_view *out_name = nullptr, MetricType *out_type = nullptr) const;
@@ -338,7 +337,7 @@ class Metrics
     std::pair<int16_t, int16_t>
     current() const
     {
-      ts::lock_guard lock(_mutex);
+      std::lock_guard lock(_mutex);
       return {_cur_blob, _cur_off};
     }
 
@@ -347,7 +346,6 @@ class Metrics
     {
       auto [blob, entry] = _splitID(id);
 
-      ts::lock_guard lock(_mutex);
       return (id >= 0 && ((blob < _cur_blob && entry < MAX_SIZE) || (blob == _cur_blob && entry <= _cur_off)));
     }
   };
diff --git a/src/tsutil/Metrics.cc b/src/tsutil/Metrics.cc
index 92bcb3bf6b0..ae96c1dbbac 100644
--- a/src/tsutil/Metrics.cc
+++ b/src/tsutil/Metrics.cc
@@ -57,8 +57,8 @@ Metrics::Storage::addBlob() // The mutex must be held before calling this!
 Metrics::IdType
 Metrics::Storage::create(std::string_view name, const MetricType type)
 {
-  ts::lock_guard lock(_mutex);
-  auto           it = _lookups.find(name);
+  std::lock_guard lock(_mutex);
+  auto            it = _lookups.find(name);
 
   if (it != _lookups.end()) {
     return it->second;
@@ -81,8 +81,8 @@ Metrics::Storage::create(std::string_view name, const MetricType type)
 Metrics::IdType
 Metrics::Storage::lookup(const std::string_view name) const
 {
-  ts::lock_guard lock(_mutex);
-  auto           it = _lookups.find(name);
+  std::lock_guard lock(_mutex);
+  auto            it = _lookups.find(name);
 
   if (it != _lookups.end()) {
     return it->second;
@@ -94,7 +94,6 @@ Metrics::Storage::lookup(const std::string_view name) const
 Metrics::AtomicType *
 Metrics::Storage::lookup(Metrics::IdType id, std::string_view *out_name, Metrics::MetricType *out_type) const
 {
-  ts::lock_guard lock(_mutex);
   auto [blob_ix, offset]         = _splitID(id);
   Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get();
 
@@ -141,7 +140,6 @@ Metrics::Storage::lookup(const std::string_view name, Metrics::IdType *out_id, M
 std::string_view
 Metrics::Storage::name(Metrics::IdType id) const
 {
-  ts::lock_guard lock(_mutex);
   auto [blob_ix, offset]         = _splitID(id);
   Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get();
 
@@ -166,7 +164,7 @@ Metrics::SpanType
 Metrics::Storage::createSpan(size_t size, Metrics::MetricType type, Metrics::IdType *id)
 {
   release_assert(size <= MAX_SIZE);
-  ts::lock_guard lock(_mutex);
+  std::lock_guard lock(_mutex);
 
   if (_cur_off + size > MAX_SIZE) {
     addBlob();
@@ -189,7 +187,6 @@ Metrics::Storage::createSpan(size_t size, Metrics::MetricType type, Metrics::IdT
 bool
 Metrics::Storage::rename(Metrics::IdType id, std::string_view name)
 {
-  ts::lock_guard lock(_mutex);
   auto [blob_ix, offset]         = _splitID(id);
   Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get();
 
@@ -198,7 +195,8 @@ Metrics::Storage::rename(Metrics::IdType id, std::string_view name)
     return false;
   }
 
-  std::string &cur = std::get<0>(std::get<0>(*blob)[offset]);
+  std::string    &cur = std::get<0>(std::get<0>(*blob)[offset]);
+  std::lock_guard lock(_mutex);
 
   if (cur.length() > 0) {
     _lookups.erase(cur);