Merge PR ceph#60216 into main

* refs/pull/60216/head:
	common/options: pass name as rvalue reference
	common/config: use libfmt to build strings
	common/config: use emplace_back() instead of push_back()
	common/HeartbeatMap: pass name as rvalue reference
	common/config_obs_mgr: use the erase() return value
	common/SloppyCRCMap: use the erase() return value
	common: disable `boost::intrusive::constant_time_size`

Reviewed-by: Patrick Donnelly <[email protected]>

Author: batrick

src/common/HeartbeatMap.cc

@@ -43,11 +43,11 @@ HeartbeatMap::~HeartbeatMap()
   ceph_assert(m_workers.empty());
 }
 
-heartbeat_handle_d *HeartbeatMap::add_worker(const string& name, pthread_t thread_id)
+heartbeat_handle_d *HeartbeatMap::add_worker(string&& name, pthread_t thread_id)
 {
   std::unique_lock locker{m_rwlock};
   ldout(m_cct, 10) << "add_worker '" << name << "'" << dendl;
-  heartbeat_handle_d *h = new heartbeat_handle_d(name);
+  heartbeat_handle_d *h = new heartbeat_handle_d(std::move(name));
   ANNOTATE_BENIGN_RACE_SIZED(&h->timeout, sizeof(h->timeout),
                              "heartbeat_handle_d timeout");
   ANNOTATE_BENIGN_RACE_SIZED(&h->suicide_timeout, sizeof(h->suicide_timeout),

src/common/HeartbeatMap.h

@@ -48,15 +48,15 @@ struct heartbeat_handle_d {
   ceph::timespan suicide_grace = ceph::timespan::zero();
   std::list<heartbeat_handle_d*>::iterator list_item;
 
-  explicit heartbeat_handle_d(const std::string& n)
-    : name(n)
+  explicit heartbeat_handle_d(std::string&& n)
+    : name(std::move(n))
   { }
 };
 
 class HeartbeatMap {
  public:
   // register/unregister
-  heartbeat_handle_d *add_worker(const std::string& name, pthread_t thread_id);
+  heartbeat_handle_d *add_worker(std::string&& name, pthread_t thread_id);
   void remove_worker(const heartbeat_handle_d *h);
 
   // reset the timeout so that it expects another touch within grace amount of time

src/common/LRUSet.h

@@ -43,6 +43,7 @@ class LRUSet {
   // lru
   boost::intrusive::list<
     Node,
+    boost::intrusive::constant_time_size<false>,
     boost::intrusive::member_hook<Node,
 				  boost::intrusive::list_member_hook<>,
 				  &Node::lru_item>

src/common/SloppyCRCMap.cc

@@ -73,7 +73,7 @@ void SloppyCRCMap::truncate(uint64_t offset)
   offset -= offset % block_size;
   std::map<uint64_t,uint32_t>::iterator p = crc_map.lower_bound(offset);
   while (p != crc_map.end())
-    crc_map.erase(p++);
+    p = crc_map.erase(p);
 }
 
 void SloppyCRCMap::zero(uint64_t offset, uint64_t len)

src/common/TrackedOp.h

@@ -243,6 +243,7 @@ class TrackedOp : public boost::intrusive::list_base_hook<> {
 public:
   typedef boost::intrusive::list<
   TrackedOp,
+  boost::intrusive::constant_time_size<false>,
   boost::intrusive::member_hook<
     TrackedOp,
     boost::intrusive::list_member_hook<>,

src/common/config.cc

@@ -24,6 +24,8 @@
 #include "common/hostname.h"
 #include "common/dout.h"
 
+#include <fmt/core.h>
+
 /* Don't use standard Ceph logging in this file.
  * We can't use logging until it's initialized, and a lot of the necessary
  * initialization happens here.
@@ -131,14 +133,11 @@ md_config_t::md_config_t(ConfigValues& values,
   // Define the debug_* options as well.
   subsys_options.reserve(values.subsys.get_num());
   for (unsigned i = 0; i < values.subsys.get_num(); ++i) {
-    string name = string("debug_") + values.subsys.get_name(i);
-    subsys_options.push_back(
-      Option(name, Option::TYPE_STR, Option::LEVEL_ADVANCED));
+    subsys_options.emplace_back(
+      fmt::format("debug_{}", values.subsys.get_name(i)), Option::TYPE_STR, Option::LEVEL_ADVANCED);
     Option& opt = subsys_options.back();
-    opt.set_default(stringify(values.subsys.get_log_level(i)) + "/" +
-		    stringify(values.subsys.get_gather_level(i)));
-    string desc = string("Debug level for ") + values.subsys.get_name(i);
-    opt.set_description(desc.c_str());
+    opt.set_default(fmt::format("{}/{}", values.subsys.get_log_level(i), values.subsys.get_gather_level(i)));
+    opt.set_description(fmt::format("Debug level for {}", values.subsys.get_name(i)).c_str());
     opt.set_flag(Option::FLAG_RUNTIME);
     opt.set_long_description("The value takes the form 'N' or 'N/M' where N and M are values between 0 and 99.  N is the debug level to log (all values below this are included), and M is the level to gather and buffer in memory.  In the event of a crash, the most recent items <= M are dumped to the log file.");
     opt.set_subsys(i);
@@ -158,7 +157,7 @@ md_config_t::md_config_t(ConfigValues& values,
 	  } else {
 	    // normalize to M/N
 	    n = m;
-	    *value = stringify(m) + "/" + stringify(n);
+	    *value = fmt::format("{}/{}", m, n);
 	  }
 	} else {
 	  *error_message = "value must take the form N or N/M, where N and M are integers";
@@ -775,7 +774,7 @@ int md_config_t::parse_option(ConfigValues& values,
     option_name = opt.name;
     if (ceph_argparse_witharg(
 	  args, i, &val, err,
-	  string(string("--default-") + opt.name).c_str(), (char*)NULL)) {
+	  fmt::format("--default-{}", opt.name).c_str(), (char*)NULL)) {
       if (!err.str().empty()) {
         error_message = err.str();
 	ret = -EINVAL;
@@ -1268,7 +1267,7 @@ Option::value_t md_config_t::_expand_meta(
 		     << Option::to_str(*i->second) << "\n";
 	      }
 	    }
-	    return Option::value_t(std::string("$") + o->name);
+	    return Option::value_t(fmt::format("${}", o->name));
 	  } else {
 	    // recursively evaluate!
 	    string n;

src/common/config_obs_mgr.h

@@ -75,7 +75,7 @@ typename ObserverMgr<ConfigObs>::config_obs_wptr ObserverMgr<ConfigObs>::remove_
   for (auto o = observers.begin(); o != observers.end(); ) {
     if (*o->second == observer) {
       ptr = std::move(o->second);
-      observers.erase(o++);
+      o = observers.erase(o);
       found_obs = true;
     } else {
       ++o;

src/common/intrusive_lru.h

@@ -125,6 +125,7 @@ class intrusive_lru {
 
   using lru_list_t = boost::intrusive::list<
     base_t,
+    boost::intrusive::constant_time_size<false>,
     boost::intrusive::member_hook<
       base_t,
       boost::intrusive::list_member_hook<>,

src/common/options.h

@@ -207,8 +207,8 @@ struct Option {
   typedef std::function<int(std::string *, std::string *)> validator_fn_t;
   validator_fn_t validator;
 
-  Option(std::string const &name, type_t t, level_t l)
-    : name(name), type(t), level(l)
+  Option(std::string &&name, type_t t, level_t l)
+    : name(std::move(name)), type(t), level(l)
   {
     // While value_t is nullable (via std::monostate), we don't ever
     // want it set that way in an Option instance: within an instance,

src/mon/ConfigMap.cc

@@ -266,7 +266,7 @@ int ConfigMap::add_option(
     ldout(cct, 10) << __func__ << " unrecognized option '" << name << "'" << dendl;
     stray_options.push_back(
       std::unique_ptr<Option>(
-	new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
+	new Option(std::string{name}, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
     opt = stray_options.back().get();
   }