common/perf_counters: enabling 'find()' by logger name

Multiple style and performance tweaks. The important one:
Modifying reset() to directly locate the named logger.
As most of the code here predates C++14, "transparent
comparators" were not used. Instead, we looped through
the list of loggers.
With the updated C++ we are using today, the comparator
can be fixed to allow a find() based on the name.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc
index 2eeaa80aae8ea6ae46c78aeed7ff2786568dafff..f448359fce7d3c8706ae25c9948fa6e3fd927a79 100644 (file)
--- a/src/common/perf_counters.cc
+++ b/src/common/perf_counters.cc
@@ -24,6 +24,8 @@ using std::ostringstream;
 using std::make_pair;
 using std::pair;
 
+using namespace std::literals;
+
 namespace TOPNSPC::common {
 PerfCountersCollectionImpl::PerfCountersCollectionImpl()
 {
@@ -37,81 +39,56 @@ PerfCountersCollectionImpl::~PerfCountersCollectionImpl()
 void PerfCountersCollectionImpl::add(PerfCounters *l)
 {
   // make sure the name is unique
-  perf_counters_set_t::iterator i;
-  i = m_loggers.find(l);
-  while (i != m_loggers.end()) {
-    ostringstream ss;
-    ss << l->get_name() << "-" << (void*)l;
-    l->set_name(ss.str());
-    i = m_loggers.find(l);
+  while (m_loggers.contains(l)) {
+    l->set_name(fmt::format("{}-{:p}", l->get_name(), (void *)l));
   }
 
   m_loggers.insert(l);
 
-  for (unsigned int i = 0; i < l->m_data.size(); ++i) {
-    PerfCounters::perf_counter_data_any_d &data = l->m_data[i];
-
-    std::string path = l->get_name();
-    path += ".";
-    path += data.name;
-
-    by_path[path] = {&data, l};
+  const auto rc_name = l->get_name();
+  for (auto& dt: l->m_data) {
+    const auto path = rc_name + "." + dt.name;
+    by_path[path] = {&dt, l};
   }
 }
 
 void PerfCountersCollectionImpl::remove(PerfCounters *l)
 {
-  for (unsigned int i = 0; i < l->m_data.size(); ++i) {
-    PerfCounters::perf_counter_data_any_d &data = l->m_data[i];
-
-    std::string path = l->get_name();
-    path += ".";
-    path += data.name;
-
+  const auto rc_name = l->get_name();
+  for (const auto& dt: l->m_data) {
+    const auto path = rc_name + "." + dt.name;
     by_path.erase(path);
   }
 
-  perf_counters_set_t::iterator i = m_loggers.find(l);
-  ceph_assert(i != m_loggers.end());
-  m_loggers.erase(i);
+  [[maybe_unused]] const auto rm_cnt = m_loggers.erase(l);
+  ceph_assert(rm_cnt == 1);
 }
 
 void PerfCountersCollectionImpl::clear()
 {
-  perf_counters_set_t::iterator i = m_loggers.begin();
-  perf_counters_set_t::iterator i_end = m_loggers.end();
-  for (; i != i_end; ) {
-    delete *i;
-    m_loggers.erase(i++);
+  for (auto& l : m_loggers) {
+    delete l;
   }
-
+  m_loggers.clear();
   by_path.clear();
 }
 
-bool PerfCountersCollectionImpl::reset(const std::string &name)
+bool PerfCountersCollectionImpl::reset(std::string_view name)
 {
-  bool result = false;
-  perf_counters_set_t::iterator i = m_loggers.begin();
-  perf_counters_set_t::iterator i_end = m_loggers.end();
-
-  if (!strcmp(name.c_str(), "all"))  {
-    while (i != i_end) {
-      (*i)->reset();
-      ++i;
-    }
-    result = true;
-  } else {
-    while (i != i_end) {
-      if (!name.compare((*i)->get_name())) {
-       (*i)->reset();
-       result = true;
-       break;
-      }
-      ++i;
+  if (name == "all"sv) {
+    for (auto& dt: m_loggers) {
+      dt->reset();
     }
+    return true;
+  }
+
+  auto dt = m_loggers.find(name);
+  if (dt == m_loggers.end()) {
+    return false;
   }
 
-  return result;
+  (*dt)->reset();
+  return true;
 }
 
 

diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h
index 0d0fe86a0920313ead589b10e227e979b2911bef..f5b0dfec617ddb3aa22deca18899130518eab55a 100644 (file)
--- a/src/common/perf_counters.h
+++ b/src/common/perf_counters.h
@@ -305,10 +305,16 @@ private:
   friend class PerfCountersCollectionImpl;
 };
 
-class SortPerfCountersByName {
-public:
+struct SortPerfCountersByName {
+  using is_transparent = void;
   bool operator()(const PerfCounters* lhs, const PerfCounters* rhs) const {
-    return (lhs->get_name() < rhs->get_name());
+    return lhs->get_name() < rhs->get_name();
+  }
+  bool operator()(std::string_view lhs, const PerfCounters* rhs) const {
+    return lhs < rhs->get_name();
+  }
+  bool operator()(const PerfCounters* lhs, std::string_view rhs) const {
+    return lhs->get_name() < rhs;
   }
 };
 
@@ -325,7 +331,8 @@ public:
   void add(PerfCounters *l);
   void remove(PerfCounters *l);
   void clear();
-  bool reset(const std::string &name);
+  // a parameter of "all" resets all counters
+  bool reset(std::string_view name);
 
   void dump_formatted(ceph::Formatter *f, bool schema, bool dump_labeled,
                       const std::string &logger = "",