Project

General

Profile

Actions

Bug #21845

closed

Objecter::_send_op unnecessarily constructs costly hobject_t

Added by Jason Dillaman over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Jason Dillaman
Category:
Performance/Resource Usage
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
luminous
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(RADOS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

With zero backoffs, just constructing an hobject_t ("hobject_t hoid = op->target.get_hobj();") results in an approximate 10% performance degradation on RBD 4K IO random read tests. Moving the object construction to within the backoff clause where it's used skips the penalty for the cases where there is no backoff from the OSDs.

12.2.1 original fio results:

   bw (  KiB/s): min=119040, max=154472, per=100.00%, avg=146800.00, stdev=9222.50, samples=30
   iops        : min=29760, max=38618, avg=36700.00, stdev=2305.62, samples=30

12.2.1 fio results after skipping the unnecessary construction:

   bw (  KiB/s): min=158688, max=165488, per=100.00%, avg=162691.57, stdev=1706.77, samples=30
   iops        : min=39672, max=41372, avg=40672.87, stdev=426.71, samples=30

Related issues 1 (0 open1 closed)

Copied to RADOS - Backport #21921: luminous: Objecter::_send_op unnecessarily constructs costly hobject_t ResolvedShinobu KinjoActions
Actions #1

Updated by Jason Dillaman over 6 years ago

  • Status changed from New to In Progress
  • Assignee set to Jason Dillaman
Actions #2

Updated by Jason Dillaman over 6 years ago

  • Status changed from In Progress to Fix Under Review
  • Backport set to luminous
Actions #3

Updated by Haomai Wang over 6 years ago

I really don't agree that this will cause 10% performance degraded... the construct should be nanoseconds level..

Actions #4

Updated by Haomai Wang over 6 years ago

of course, it's a good cleanup

Actions #5

Updated by Jason Dillaman over 6 years ago

multiple runs of "perf record" didn't lie -- and neither did the fact that moving it increased performance by ~10% under repeated testing.

Actions #6

Updated by Jason Dillaman over 6 years ago

... I should also note that in unrelated "perf record" sessions for the "debug ms = 0/1" performance depredations, you could see a decent hit just in "hobject_t::operator<<" where it default constructs itself to test if it's the minimum [1]

[1] https://github.com/ceph/ceph/blob/edbd6edb0471d5e61815b29a1c4a50e21f3b8d12/src/common/hobject.cc#L239

Actions #7

Updated by Haomai Wang over 6 years ago

we have a lots of "xx == hobject_t()" judgements in the codes...

Actions #8

Updated by Jason Dillaman over 6 years ago

Indeed -- in the OSDs. I only benchmarked the librbd clients under high IOPS workloads and that call is only executed in "operator<<" from a client's perspective.

Actions #9

Updated by Sage Weil over 6 years ago

  • Status changed from Fix Under Review to Pending Backport
Actions #10

Updated by Nathan Cutler over 6 years ago

  • Copied to Backport #21921: luminous: Objecter::_send_op unnecessarily constructs costly hobject_t added
Actions #11

Updated by Nathan Cutler over 6 years ago

  • Status changed from Pending Backport to Resolved
Actions

Also available in: Atom PDF