Skip to content

Metrics: Revert added locking and annotations #13567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
moonchen merged 1 commit into from
Aug 19, 2026
+15 −19
Merged

Metrics: Revert added locking and annotations #13567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions include/tsutil/Metrics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include "swoc/MemSpan.h"

#include "tsutil/Assert.h"
#include "tsutil/TsMutex.h"

namespace ts
{
Expand Down Expand Up @@ -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]);
Expand All @@ -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;
Expand All @@ -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};
}

Expand All @@ -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)));
}
};
Expand Down
16 changes: 7 additions & 9 deletions src/tsutil/Metrics.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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();

Comment on lines 95 to 99
Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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);
Expand Down