From 184d0a75f92cbaebbb8e0eab162f48f49bea14b6 Mon Sep 17 00:00:00 2001
From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Tue, 15 May 2018 22:14:15 +0200
Subject: [PATCH] osd: avoid unncecessary ref-counting in OSD::enqueue_op.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

```
-    4,30%     0,32%  msgr-worker-0  ceph-osd  [.] OSD::enqueue_op
   - 3,98% OSD::enqueue_op
      + 2,40% OSD::ShardedOpWQ::_enqueue
      + 0,42% OpRequest::mark_flag_point
        0,37% operator new
        0,36% PerfCounters::tinc
      + 0,21% Mutex::Unlock
```

Annotate:
```
OSD::enqueue_op  /home/perf/src/ceph-rzarzynski3/build/bin/ceph-osd
       │93           if( px != 0 ) intrusive_ptr_add_ref( px );
       │       test   %rbx,%rbx
       │95   _ZN13PGOpQueueableC4E5spg_t():
       │175    explicit PGOpQueueable(spg_t pg) : pgid(pg) {}
  0,70 │       mov    %cl,0x18(%rax)
       │177  _ZN8PGOpItemC4E5spg_tN5boost13intrusive_ptrI9OpRequestEE():
       │204    PGOpItem(spg_t pg, OpRequestRef op) : PGOpQueueable(pg), op(op) {
       │       mov    %rdx,(%rax)
       │206  _ZN5boost13intrusive_ptrI9OpRequestEC4ERKS2_():
       │     ↓ je     166
       │94   _ZNSt13__atomic_baseIiEppEv():
 48,25 │       lock   addl   $0x1,0x30(%rbx)
       │166:   mov    0x158(%rbp),%ecx
       │       mov    0x15c(%rbp),%edx
       │299  _ZN17ShardedThreadPool9ShardedWQI11OpQueueItemE5queueEOS1_():
       │669        tp->set_wq(this);
       │670      }
       │671      ~ShardedWQ() override {}
       │
       │673      void queue(T&& item) {
       │674        _enqueue(std::move(item));
  2,80 │       mov    %r12,%rsi
```

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---
 src/osd/OSD.cc        | 29 +++++++++++++++--------------
 src/osd/OSD.h         |  2 +-
 src/osd/OpQueueItem.h |  2 +-
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 57f6d9fa06312..d383d28ca45da 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -6705,7 +6705,7 @@ void OSD::dispatch_session_waiting(SessionRef session, OSDMapRef osdmap)
     } else {
       pgid = m->get_spg();
     }
-    enqueue_op(pgid, op, m->get_map_epoch());
+    enqueue_op(pgid, std::move(op), m->get_map_epoch());
   }
 
   if (session->waiting_on_map.empty()) {
@@ -6791,7 +6791,7 @@ void OSD::ms_fast_dispatch(Message *m)
     // queue it directly
     enqueue_op(
       static_cast<MOSDFastDispatchOp*>(m)->get_spg(),
-      op,
+      std::move(op),
       static_cast<MOSDFastDispatchOp*>(m)->get_map_epoch());
   } else {
     // legacy client, and this is an MOSDOp (the *only* fast dispatch
@@ -9279,27 +9279,28 @@ bool OSD::op_is_discardable(const MOSDOp *op)
   return false;
 }
 
-void OSD::enqueue_op(spg_t pg, OpRequestRef& op, epoch_t epoch)
+void OSD::enqueue_op(spg_t pg, OpRequestRef&& op, epoch_t epoch)
 {
-  utime_t latency = ceph_clock_now() - op->get_req()->get_recv_stamp();
-  dout(15) << "enqueue_op " << op << " prio " << op->get_req()->get_priority()
-	   << " cost " << op->get_req()->get_cost()
+  const utime_t stamp = op->get_req()->get_recv_stamp();
+  const utime_t latency = ceph_clock_now() - stamp;
+  const unsigned priority = op->get_req()->get_priority();
+  const int cost = op->get_req()->get_cost();
+  const uint64_t owner = op->get_req()->get_source().num();
+
+  dout(15) << "enqueue_op " << op << " prio " << priority
+	   << " cost " << cost
 	   << " latency " << latency
 	   << " epoch " << epoch
 	   << " " << *(op->get_req()) << dendl;
   op->osd_trace.event("enqueue op");
-  op->osd_trace.keyval("priority", op->get_req()->get_priority());
-  op->osd_trace.keyval("cost", op->get_req()->get_cost());
+  op->osd_trace.keyval("priority", priority);
+  op->osd_trace.keyval("cost", cost);
   op->mark_queued_for_pg();
   logger->tinc(l_osd_op_before_queue_op_lat, latency);
   op_shardedwq.queue(
     OpQueueItem(
-      unique_ptr<OpQueueItem::OpQueueable>(new PGOpItem(pg, op)),
-      op->get_req()->get_cost(),
-      op->get_req()->get_priority(),
-      op->get_req()->get_recv_stamp(),
-      op->get_req()->get_source().num(),
-      epoch));
+      unique_ptr<OpQueueItem::OpQueueable>(new PGOpItem(pg, std::move(op))),
+      cost, priority, stamp, owner, epoch));
 }
 
 void OSD::enqueue_peering_evt(spg_t pgid, PGPeeringEventRef evt)
diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index cace749e139b4..15df17d34db9a 100644
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1779,7 +1779,7 @@ protected:
   } op_shardedwq;
 
 
-  void enqueue_op(spg_t pg, OpRequestRef& op, epoch_t epoch);
+  void enqueue_op(spg_t pg, OpRequestRef&& op, epoch_t epoch);
   void dequeue_op(
     PGRef pg, OpRequestRef op,
     ThreadPool::TPHandle &handle);
diff --git a/src/osd/OpQueueItem.h b/src/osd/OpQueueItem.h
index 749ef0706127a..558c5c885c8b7 100644
--- a/src/osd/OpQueueItem.h
+++ b/src/osd/OpQueueItem.h
@@ -201,7 +201,7 @@ public:
 class PGOpItem : public PGOpQueueable {
   OpRequestRef op;
 public:
-  PGOpItem(spg_t pg, OpRequestRef op) : PGOpQueueable(pg), op(op) {}
+  PGOpItem(spg_t pg, OpRequestRef op) : PGOpQueueable(pg), op(std::move(op)) {}
   op_type_t get_op_type() const override final {
     return op_type_t::client_op;
   }
-- 
2.39.5