Merge pull request ceph#55583 from ronen-fr/wip-rf-hobject

osd: improve hobject_t::to_str() performance

Reviewed-by: Matan Breizman <[email protected]>
Reviewed-by: Samuel Just <[email protected]>

Author: ronen-fr

src/common/fmt_common.h

@@ -7,6 +7,7 @@
 /**
  * \file default fmtlib formatters for specifically-tagged types
  */
+#include <fmt/compile.h>
 #include <fmt/format.h>
 
 /**

src/common/hobject.cc

@@ -2,6 +2,8 @@
 // vim: ts=8 sw=2 smarttab
 
 #include <charconv>
+#include <fmt/compile.h>
+#include <fmt/core.h>
 
 #include "hobject.h"
 #include "common/Formatter.h"
@@ -14,23 +16,25 @@ using std::string;
 using ceph::bufferlist;
 using ceph::Formatter;
 
-static void append_escaped(const string &in, string *out)
+namespace {
+void escape_special_chars(const string& in, string* out)
 {
-  for (string::const_iterator i = in.begin(); i != in.end(); ++i) {
-    if (*i == '%') {
+  for (auto c : in) {
+    if (c == '%') {
       out->push_back('%');
       out->push_back('p');
-    } else if (*i == '.') {
+    } else if (c == '.') {
       out->push_back('%');
       out->push_back('e');
-    } else if (*i == '_') {
+    } else if (c == '_') {
       out->push_back('%');
       out->push_back('u');
     } else {
-      out->push_back(*i);
+      out->push_back(c);
     }
   }
 }
+}  // namespace
 
 set<string> hobject_t::get_prefixes(
   uint32_t bits,
@@ -80,33 +84,25 @@ set<string> hobject_t::get_prefixes(
 
 string hobject_t::to_str() const
 {
-  string out;
-
-  char snap_with_hash[1000];
-  char *t = snap_with_hash;
-  const char *end = t + sizeof(snap_with_hash);
-
   uint64_t poolid(pool);
-  t += snprintf(t, end - t, "%.*llX", 16, (long long unsigned)poolid);
-
   uint32_t revhash(get_nibblewise_key_u32());
-  t += snprintf(t, end - t, ".%.*X", 8, revhash);
 
-  if (snap == CEPH_NOSNAP)
-    t += snprintf(t, end - t, ".head");
-  else if (snap == CEPH_SNAPDIR)
-    t += snprintf(t, end - t, ".snapdir");
-  else
-    t += snprintf(t, end - t, ".%llx", (long long unsigned)snap);
-
-  out.append(snap_with_hash, t);
+  string out;
+  if (snap == CEPH_NOSNAP) {
+    out = fmt::format(FMT_COMPILE("{:016X}.{:08X}.head."), poolid, revhash);
+  } else if (snap == CEPH_SNAPDIR) {
+    out = fmt::format(FMT_COMPILE("{:016X}.{:08X}.snapdir."), poolid, revhash);
+  } else {
+    out = fmt::format(
+	FMT_COMPILE("{:016X}.{:08X}.{:X}."), poolid, revhash,
+	(unsigned long long)snap);
+  }
 
+  escape_special_chars(oid.name, &out);
   out.push_back('.');
-  append_escaped(oid.name, &out);
-  out.push_back('.');
-  append_escaped(get_key(), &out);
+  escape_special_chars(get_key(), &out);
   out.push_back('.');
-  append_escaped(nspace, &out);
+  escape_special_chars(nspace, &out);
 
   return out;
 }

src/common/hobject.h

@@ -15,6 +15,9 @@
 #ifndef __CEPH_OS_HOBJECT_H
 #define __CEPH_OS_HOBJECT_H
 
+#include <fmt/compile.h>
+#include <fmt/format.h>
+
 #if FMT_VERSION >= 90000
 #include <fmt/ostream.h>
 #endif
@@ -318,7 +321,7 @@ struct hobject_t {
   void dump(ceph::Formatter *f) const;
   static void generate_test_instances(std::list<hobject_t*>& o);
   friend int cmp(const hobject_t& l, const hobject_t& r);
-  auto operator<=>(const hobject_t &rhs) const noexcept {
+  constexpr auto operator<=>(const hobject_t &rhs) const noexcept {
     auto cmp = max <=> rhs.max;
     if (cmp != 0) return cmp;
     cmp = pool <=> rhs.pool;
@@ -335,10 +338,11 @@ struct hobject_t {
     if (cmp != 0) return cmp;
     return snap <=> rhs.snap;
   }
-  bool operator==(const hobject_t& rhs) const noexcept {
+  constexpr bool operator==(const hobject_t& rhs) const noexcept {
     return operator<=>(rhs) == 0;
   }
   friend struct ghobject_t;
+  friend struct test_hobject_fmt_t;
 };
 WRITE_CLASS_ENCODER(hobject_t)
 
@@ -351,6 +355,54 @@ template<> struct hash<hobject_t> {
 };
 } // namespace std
 
+namespace fmt {
+template <>
+struct formatter<hobject_t> {
+
+  template <typename FormatContext>
+  static inline auto
+  append_sanitized(FormatContext& ctx, const std::string& in, int sep = 0)
+  {
+    for (const auto i : in) {
+      if (i == '%' || i == ':' || i == '/' || i < 32 || i >= 127) {
+	fmt::format_to(
+	    ctx.out(), FMT_COMPILE("%{:02x}"), static_cast<unsigned char>(i));
+      } else {
+	fmt::format_to(ctx.out(), FMT_COMPILE("{:c}"), i);
+      }
+    }
+    if (sep) {
+      fmt::format_to(
+	  ctx.out(), FMT_COMPILE("{:c}"), sep);
+    }
+    return ctx.out();
+  }
+
+  constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); }
+
+  template <typename FormatContext>
+  auto format(const hobject_t& ho, FormatContext& ctx)
+  {
+    if (ho == hobject_t{}) {
+      return fmt::format_to(ctx.out(), "MIN");
+    }
+
+    if (ho.is_max()) {
+      return fmt::format_to(ctx.out(), "MAX");
+    }
+
+    fmt::format_to(
+	ctx.out(), FMT_COMPILE("{}:{:08x}:"), static_cast<uint64_t>(ho.pool),
+	ho.get_bitwise_key_u32());
+    append_sanitized(ctx, ho.nspace, ':');
+    append_sanitized(ctx, ho.get_key(), ':');
+    append_sanitized(ctx, ho.oid.name);
+    return fmt::format_to(ctx.out(), FMT_COMPILE(":{}"), ho.snap);
+  }
+};
+}  // namespace fmt
+
+
 std::ostream& operator<<(std::ostream& out, const hobject_t& o);
 
 template <typename T>
@@ -495,7 +547,7 @@ struct ghobject_t {
   void dump(ceph::Formatter *f) const;
   static void generate_test_instances(std::list<ghobject_t*>& o);
   friend int cmp(const ghobject_t& l, const ghobject_t& r);
-  auto operator<=>(const ghobject_t&) const = default;
+  constexpr auto operator<=>(const ghobject_t&) const = default;
   bool operator==(const ghobject_t&) const = default;
 };
 WRITE_CLASS_ENCODER(ghobject_t)

src/common/hobject_fmt.h

@@ -4,7 +4,7 @@
 #include <algorithm>
 #include <boost/type_index.hpp>
 #include <fmt/ranges.h>
-#include "common/hobject_fmt.h"
+#include "common/hobject.h"
 #include "crimson/osd/backfill_state.h"
 #include "osd/osd_types_fmt.h"
 

src/crimson/osd/backfill_state.cc

@@ -14,7 +14,7 @@
 #include <fmt/format.h>
 #include <fmt/ostream.h>
 
-#include "common/hobject_fmt.h"
+#include "common/hobject.h"
 
 #include "messages/MOSDOp.h"
 #include "messages/MOSDOpReply.h"

src/crimson/osd/pg.cc

@@ -18,7 +18,6 @@
 
 #include "common/fmt_common.h"
 #include "common/hobject.h"
-#include "common/hobject_fmt.h"
 #include "crimson/common/log.h"
 #include "osd/osd_types_fmt.h"
 #include "scrub_validator.h"

src/crimson/osd/scrub/scrub_machine.h

@@ -23,6 +23,9 @@
 #include <string>
 #include <string_view>
 
+#include <fmt/compile.h>
+#include <fmt/format.h>
+
 #include "include/rados.h"
 #include "include/unordered_map.h"
 #include "common/Formatter.h"
@@ -112,10 +115,10 @@ struct file_object_t {
 struct snapid_t {
   uint64_t val;
   // cppcheck-suppress noExplicitConstructor
-  snapid_t(uint64_t v=0) : val(v) {}
+  constexpr snapid_t(uint64_t v=0) : val(v) {}
   snapid_t operator+=(snapid_t o) { val += o.val; return *this; }
   snapid_t operator++() { ++val; return *this; }
-  operator uint64_t() const { return val; }
+  constexpr operator uint64_t() const { return val; }
 };
 
 inline void encode(snapid_t i, ceph::buffer::list &bl) {
@@ -153,6 +156,25 @@ inline std::ostream& operator<<(std::ostream& out, const snapid_t& s) {
     return out << std::hex << s.val << std::dec;
 }
 
+namespace fmt {
+template <>
+struct formatter<snapid_t> {
+
+  constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); }
+
+  template <typename FormatContext>
+  auto format(const snapid_t& snp, FormatContext& ctx) const
+  {
+    if (snp == CEPH_NOSNAP) {
+      return fmt::format_to(ctx.out(), "head");
+    }
+    if (snp == CEPH_SNAPDIR) {
+      return fmt::format_to(ctx.out(), "snapdir");
+    }
+    return fmt::format_to(ctx.out(), FMT_COMPILE("{:x}"), snp.val);
+  }
+};
+} // namespace fmt
 
 struct sobject_t {
   object_t oid;

src/include/object.h

@@ -513,7 +513,7 @@ struct shard_id_t {
   shard_id_t() : id(0) {}
   constexpr explicit shard_id_t(int8_t _id) : id(_id) {}
 
-  operator int8_t() const { return id; }
+  constexpr operator int8_t() const { return id; }
 
   const static shard_id_t NO_SHARD;