crimson: drop extra copy in errorator's appliance.

Save few unneeded instructions on the read path. I expect
negligible impact on performance. The patch is rather for
easier reading of assembler.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h
index 36de9770078c37f84faa7339f9208463fb1e542b..00b0a15d6bfa914a3ed1e3cf02e5ae6a95874372 100644 (file)
--- a/src/crimson/common/errorator.h
+++ b/src/crimson/common/errorator.h
@@ -288,6 +288,8 @@ struct errorator {
   static_assert((... && contains_once_v<AllowedErrors>),
                 "no error type in errorator can be duplicated");
 
+  struct ready_future_marker{};
+
 private:
   // see the comment for `using future = _future` below.
   template <class>
@@ -377,6 +379,12 @@ private:
       : base_t(std::move(base)) {
     }
 
+    template <class... A>
+    [[gnu::always_inline]]
+    _future(ready_future_marker, A&&... a)
+      : base_t(::seastar::make_ready_future<ValuesT...>(std::forward<A>(a)...)) {
+    }
+
     template <template <class...> class ErroratedFuture,
               class = std::void_t<
                 typename ErroratedFuture<
@@ -625,6 +633,15 @@ public:
     using type = errorator<AllowedErrors...>;
   };
 
+  template <typename... T, typename... A>
+  static future<T...> make_ready_future(A&&... value) {
+    return future<T...>(ready_future_marker(), std::forward<A>(value)...);
+  }
+
+  static auto now() {
+    return make_ready_future<>();
+  }
+
 private:
   template <class T, class = std::void_t<T>>
   class futurize {

diff --git a/src/crimson/os/cyan_store.cc b/src/crimson/os/cyan_store.cc
index 604c2d9262ca3f915b64d881890be016acd430d6..f0a76bd490053a6785f4be75cfcb38e9ecae51a4 100644 (file)
--- a/src/crimson/os/cyan_store.cc
+++ b/src/crimson/os/cyan_store.cc
@@ -189,13 +189,13 @@ CyanStore::read_errorator::future<ceph::bufferlist> CyanStore::read(
     return crimson::ct_error::enoent::make();
   }
   if (offset >= o->get_size())
-    return seastar::make_ready_future<ceph::bufferlist>();
+    return read_errorator::make_ready_future<ceph::bufferlist>();
   size_t l = len;
   if (l == 0 && offset == 0)  // note: len == 0 means read the entire object
     l = o->get_size();
   else if (offset + l > o->get_size())
     l = o->get_size() - offset;
-  return seastar::make_ready_future<ceph::bufferlist>(o->read(offset, l));
+  return read_errorator::make_ready_future<ceph::bufferlist>(o->read(offset, l));
 }
 
 CyanStore::get_attr_errorator::future<ceph::bufferptr> CyanStore::get_attr(
@@ -211,7 +211,7 @@ CyanStore::get_attr_errorator::future<ceph::bufferptr> CyanStore::get_attr(
     return crimson::ct_error::enoent::make();
   }
   if (auto found = o->xattr.find(name); found != o->xattr.end()) {
-    return seastar::make_ready_future<ceph::bufferptr>(found->second);
+    return get_attr_errorator::make_ready_future<ceph::bufferptr>(found->second);
   } else {
     return crimson::ct_error::enodata::make();
   }

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 80d6c01198f8b30d6e4472d13df07fdd9e13c934..7124f49d5494de25d8ae73364a73e390adb87563 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -372,7 +372,7 @@ OpsExecuter::execute_osd_op(OSDOp& osd_op)
         [&osd_op](ceph::bufferlist&& bl) {
           osd_op.rval = bl.length();
           osd_op.outdata = std::move(bl);
-          return seastar::now();
+          return osd_op_errorator::now();
         });
     });
   case CEPH_OSD_OP_GETXATTR:
@@ -392,7 +392,7 @@ OpsExecuter::execute_osd_op(OSDOp& osd_op)
       return backend.writefull(os, osd_op, txn);
     });
   case CEPH_OSD_OP_SETALLOCHINT:
-    return seastar::now();
+    return osd_op_errorator::now();
   case CEPH_OSD_OP_SETXATTR:
     return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
       return backend.setxattr(os, osd_op, txn);
@@ -428,9 +428,11 @@ OpsExecuter::execute_osd_op(OSDOp& osd_op)
       return backend.omap_get_vals_by_keys(os, osd_op);
     });
   case CEPH_OSD_OP_OMAPSETVALS:
+#if 0
     if (!pg.get_pool().info.supports_omap()) {
       return crimson::ct_error::operation_not_supported::make();
     }
+#endif
     return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
       return backend.omap_set_vals(os, osd_op, txn);
     });
@@ -439,7 +441,7 @@ OpsExecuter::execute_osd_op(OSDOp& osd_op)
   case CEPH_OSD_OP_WATCH:
     return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
       logger().warn("CEPH_OSD_OP_WATCH is not implemented yet; ignoring");
-      return seastar::now();
+      return osd_op_errorator::now();
     });
 
   default:

diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc
index 319fd756da5774d46bb6ba0e8acb938dfa262f79..edb20dd0b99084371505fbf141c7a45dcafd02da 100644 (file)
--- a/src/crimson/osd/pg_backend.cc
+++ b/src/crimson/osd/pg_backend.cc
@@ -121,12 +121,12 @@ PGBackend::_load_os(const hobject_t& oid)
       // decode existing OI_ATTR's value
       ceph::bufferlist bl;
       bl.push_back(std::move(bp));
-      return seastar::make_ready_future<cached_os_t>(
+      return get_os_errorator::make_ready_future<cached_os_t>(
         os_cache.insert(oid,
           std::make_unique<ObjectState>(object_info_t{bl}, true /* exists */)));
     }, crimson::errorator<crimson::ct_error::enoent,
                           crimson::ct_error::enodata>::all_same_way([oid, this] {
-      return seastar::make_ready_future<cached_os_t>(
+      return get_os_errorator::make_ready_future<cached_os_t>(
         os_cache.insert(oid,
           std::make_unique<ObjectState>(object_info_t{oid}, false)));
     }));
@@ -146,7 +146,7 @@ PGBackend::_load_ss(const hobject_t& oid)
       // decode existing SS_ATTR's value
       ceph::bufferlist bl;
       bl.push_back(std::move(bp));
-      return seastar::make_ready_future<cached_ss_t>(
+      return get_os_errorator::make_ready_future<cached_ss_t>(
         ss_cache.insert(oid, std::make_unique<SnapSet>(bl)));
     }, crimson::errorator<crimson::ct_error::enoent,
                           crimson::ct_error::enodata>::all_same_way([oid, this] {
@@ -154,7 +154,7 @@ PGBackend::_load_ss(const hobject_t& oid)
       //   `get_attr_errorator::all_same_way(...)`.
       // however, this way is more explicit and resilient to unexpected
       // changes in the alias definition.
-      return seastar::make_ready_future<cached_ss_t>(
+      return get_os_errorator::make_ready_future<cached_ss_t>(
         ss_cache.insert(oid, std::make_unique<SnapSet>()));
     }));
 }
@@ -240,12 +240,12 @@ PGBackend::read(const object_info_t& oi,
   }
   if (offset >= size) {
     // read size was trimmed to zero and it is expected to do nothing,
-    return seastar::make_ready_future<bufferlist>();
+    return read_errorator::make_ready_future<bufferlist>();
   }
   return _read(oi.soid, offset, length, flags).safe_then(
     [&oi](auto&& bl) -> read_errorator::future<ceph::bufferlist> {
       if (const bool is_fine = _read_verify_data(oi, bl); is_fine) {
-        return seastar::make_ready_future<bufferlist>(std::move(bl));
+        return read_errorator::make_ready_future<bufferlist>(std::move(bl));
       } else {
         return crimson::ct_error::object_corrupted::make();
       }
@@ -264,7 +264,7 @@ PGBackend::stat_errorator::future<> PGBackend::stat(
     logger().debug("stat object does not exist");
     return crimson::ct_error::enoent::make();
   }
-  return seastar::now();
+  return stat_errorator::now();
   // TODO: ctx->delta_stats.num_rd++;
 }
 
@@ -483,7 +483,7 @@ PGBackend::get_attr_errorator::future<> PGBackend::getxattr(
     osd_op.outdata.clear();
     osd_op.outdata.push_back(std::move(val));
     osd_op.op.xattr.value_len = osd_op.outdata.length();
-    return seastar::now();
+    return get_attr_errorator::now();
     //ctx->delta_stats.num_rd_kb += shift_round_up(osd_op.outdata.length(), 10);
   });
   //ctx->delta_stats.num_rd++;