This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 12acc3a1b571746729dcce668b9f334374b23817 Author: Mo Chen <[email protected]> AuthorDate: Wed Aug 19 09:04:25 2026 -0500 Metrics: Revert added locking and annotations (#13567) 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. (cherry picked from commit 02013679bb8237bf942b639b40530d6fcd5ce7ca) --- 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 4f47fd5485..630f079b4d 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 @@ private: 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 @@ private: ~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 @@ private: 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 @@ private: { 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 92bcb3bf6b..ae96c1dbba 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);
