In instrumented mutex, take timing once for both of perf_context and statistics

Summary: Closes https://github.com/facebook/rocksdb/pull/3427

Differential Revision: D6827236

Pulled By: siying

fbshipit-source-id: d8a2cc525c90df625510565669f2659014259a8a

monitoring/instrumented_mutex.cc

@@ -10,25 +10,21 @@
 
 namespace rocksdb {
 namespace {
-bool ShouldReportToStats(Env* env, Statistics* stats) {
+Statistics* stats_for_report(Env* env, Statistics* stats) {
-  return env != nullptr && stats != nullptr &&
+  if (env != nullptr && stats != nullptr &&
-          stats->stats_level_ > kExceptTimeForMutex;
+      stats->stats_level_ > kExceptTimeForMutex) {
+    return stats;
+  } else {
+    return nullptr;
+  }
 }
 }  // namespace
 
 void InstrumentedMutex::Lock() {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_mutex_lock_nanos,
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+      db_mutex_lock_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-  uint64_t wait_time_micros = 0;
+      stats_for_report(env_, stats_), stats_code_);
-  if (ShouldReportToStats(env_, stats_)) {
+  LockInternal();
-    {
-      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
-      LockInternal();
-    }
-    RecordTick(stats_, stats_code_, wait_time_micros);
-  } else {
-    LockInternal();
-  }
 }
 
 void InstrumentedMutex::LockInternal() {
@@ -39,18 +35,10 @@ void InstrumentedMutex::LockInternal() {
 }
 
 void InstrumentedCondVar::Wait() {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_condition_wait_nanos,
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+      db_condition_wait_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-  uint64_t wait_time_micros = 0;
+      stats_for_report(env_, stats_), stats_code_);
-  if (ShouldReportToStats(env_, stats_)) {
+  WaitInternal();
-    {
-      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
-      WaitInternal();
-    }
-    RecordTick(stats_, stats_code_, wait_time_micros);
-  } else {
-    WaitInternal();
-  }
 }
 
 void InstrumentedCondVar::WaitInternal() {
@@ -61,20 +49,10 @@ void InstrumentedCondVar::WaitInternal() {
 }
 
 bool InstrumentedCondVar::TimedWait(uint64_t abs_time_us) {
-  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(db_condition_wait_nanos,
+  PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(
-                                         stats_code_ == DB_MUTEX_WAIT_MICROS);
+      db_condition_wait_nanos, stats_code_ == DB_MUTEX_WAIT_MICROS,
-  uint64_t wait_time_micros = 0;
+      stats_for_report(env_, stats_), stats_code_);
-  bool result = false;
+  return TimedWaitInternal(abs_time_us);
-  if (ShouldReportToStats(env_, stats_)) {
-    {
-      StopWatch sw(env_, nullptr, 0, &wait_time_micros);
-      result = TimedWaitInternal(abs_time_us);
-    }
-    RecordTick(stats_, stats_code_, wait_time_micros);
-  } else {
-    result = TimedWaitInternal(abs_time_us);
-  }
-  return result;
 }
 
 bool InstrumentedCondVar::TimedWaitInternal(uint64_t abs_time_us) {

monitoring/perf_context_imp.h

@@ -41,10 +41,12 @@ extern __thread PerfContext perf_context;
   PerfStepTimer perf_step_timer_##metric(&(perf_context.metric)); \
   perf_step_timer_##metric.Start();
 
-#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition)       \
+#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition, stats,      \
-  PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), true); \
+                                               ticker_type)                   \
-  if (condition) {                                                      \
+  PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), true, stats, \
-    perf_step_timer_##metric.Start();                                   \
+                                         ticker_type);                        \
+  if (condition) {                                                            \
+    perf_step_timer_##metric.Start();                                         \
   }
 
 // Update metric with time elapsed since last START. start time is reset

monitoring/perf_step_timer.h

@@ -12,19 +12,25 @@ namespace rocksdb {
 
 class PerfStepTimer {
  public:
-  explicit PerfStepTimer(uint64_t* metric, bool for_mutex = false)
+  explicit PerfStepTimer(uint64_t* metric, bool for_mutex = false,
-      : enabled_(perf_level >= PerfLevel::kEnableTime ||
+                         Statistics* statistics = nullptr,
-                 (!for_mutex && perf_level >= kEnableTimeExceptForMutex)),
+                         uint32_t ticker_type = 0)
-        env_(enabled_ ? Env::Default() : nullptr),
+      : perf_counter_enabled_(
+            perf_level >= PerfLevel::kEnableTime ||
+            (!for_mutex && perf_level >= kEnableTimeExceptForMutex)),
+        env_((perf_counter_enabled_ || statistics != nullptr) ? Env::Default()
+                                                              : nullptr),
         start_(0),
-        metric_(metric) {}
+        metric_(metric),
+        statistics_(statistics),
+        ticker_type_(ticker_type) {}
 
   ~PerfStepTimer() {
     Stop();
   }
 
   void Start() {
-    if (enabled_) {
+    if (perf_counter_enabled_ || statistics_ != nullptr) {
       start_ = env_->NowNanos();
     }
   }
@@ -39,16 +45,25 @@ class PerfStepTimer {
 
   void Stop() {
     if (start_) {
-      *metric_ += env_->NowNanos() - start_;
+      uint64_t duration = env_->NowNanos() - start_;
+      if (perf_counter_enabled_) {
+        *metric_ += duration;
+      }
+
+      if (statistics_ != nullptr) {
+        RecordTick(statistics_, ticker_type_, duration);
+      }
       start_ = 0;
     }
   }
 
  private:
-  const bool enabled_;
+  const bool perf_counter_enabled_;
   Env* const env_;
   uint64_t start_;
   uint64_t* metric_;
+  Statistics* statistics_;
+  uint32_t ticker_type_;
 };
 
 }  // namespace rocksdb