From a7204018770cdbb433e258c062111289e6215e10 Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrewkr@fb.com>
Date: Wed, 13 Jun 2018 13:03:09 -0700
Subject: [PATCH] Avoid acquiring SyncPoint mutex when it is disabled (#3991)

Summary:
In `db_stress` profile the vast majority of CPU time is spent acquiring the `SyncPoint` mutex. I mistakenly assumed #3939 had fixed this mutex contention problem by disabling `SyncPoint` processing. But actually the lock was still being acquired just to check whether processing is enabled. We can avoid that overhead by using an atomic to track whether it's enabled.
Closes https://github.com/facebook/rocksdb/pull/3991

Differential Revision: D8393825

Pulled By: ajkr

fbshipit-source-id: 5bc4e3c722ee7304e7a9c2439998c456b05a6897
---
 util/sync_point_impl.cc | 2 +-
 util/sync_point_impl.h  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/sync_point_impl.cc b/util/sync_point_impl.cc
index ab4cc5ae5..248c381a3 100644
--- a/util/sync_point_impl.cc
+++ b/util/sync_point_impl.cc
@@ -89,11 +89,11 @@ void SyncPoint::Data::ClearAllCallBacks() {
 }
 
 void SyncPoint::Data::Process(const std::string& point, void* cb_arg) {
-  std::unique_lock<std::mutex> lock(mutex_);
   if (!enabled_) {
     return;
   }
 
+  std::unique_lock<std::mutex> lock(mutex_);
   auto thread_id = std::this_thread::get_id();
 
   auto marker_iter = markers_.find(point);
diff --git a/util/sync_point_impl.h b/util/sync_point_impl.h
index 8c7bd7a2d..3c7e70491 100644
--- a/util/sync_point_impl.h
+++ b/util/sync_point_impl.h
@@ -6,6 +6,7 @@
 #include "util/sync_point.h"
 
 #include <assert.h>
+#include <atomic>
 #include <condition_variable>
 #include <functional>
 #include <mutex>
@@ -22,6 +23,7 @@
 #ifndef NDEBUG
 namespace rocksdb {
 struct SyncPoint::Data {
+  Data() : enabled_(false) {}
   // Enable proper deletion by subclasses
   virtual ~Data() {}
   // successor/predecessor map loaded from LoadDependency
@@ -35,7 +37,7 @@ struct SyncPoint::Data {
   std::condition_variable cv_;
   // sync points that have been passed through
   std::unordered_set<std::string> cleared_points_;
-  bool enabled_ = false;
+  std::atomic<bool> enabled_;
   int num_callbacks_running_ = 0;
 
   void LoadDependency(const std::vector<SyncPointPair>& dependencies);
@@ -51,11 +53,9 @@ struct SyncPoint::Data {
   void ClearCallBack(const std::string& point);
   void ClearAllCallBacks();
   void EnableProcessing() {
-    std::lock_guard<std::mutex> lock(mutex_);
     enabled_ = true;
   }
   void DisableProcessing() {
-    std::lock_guard<std::mutex> lock(mutex_);
     enabled_ = false;
   }
   void ClearTrace() {