[cpu-profiler] Cleanup and use std atomics in Sampler

There's no reason to use our self-baked atomics anymore. Also

- Changes two boolean values to use a boolean instead of an int
- Uses a unique ptr for data_
- Removes has_processing_thread_ which is not used
- Moves most initialization inline into the class
- Removes SetUp/TearDown which weren't needed

Change-Id: I8f50133636961502d56351abd2fb17196603a01a
Reviewed-on: https://chromium-review.googlesource.com/c/1422918
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58950}

src/libsampler/sampler.cc

@@ -307,24 +307,18 @@ class Sampler::PlatformData {
 #if defined(USE_SIGNALS)
 class SignalHandler {
  public:
-  static void SetUp() { if (!mutex_) mutex_ = new base::Mutex(); }
-  static void TearDown() {
-    delete mutex_;
-    mutex_ = nullptr;
-  }
-
   static void IncreaseSamplerCount() {
-    base::MutexGuard lock_guard(mutex_);
+    base::MutexGuard lock_guard(mutex_.Pointer());
     if (++client_count_ == 1) Install();
   }
 
   static void DecreaseSamplerCount() {
-    base::MutexGuard lock_guard(mutex_);
+    base::MutexGuard lock_guard(mutex_.Pointer());
     if (--client_count_ == 0) Restore();
   }
 
   static bool Installed() {
-    base::MutexGuard lock_guard(mutex_);
+    base::MutexGuard lock_guard(mutex_.Pointer());
     return signal_handler_installed_;
   }
 
@@ -353,13 +347,13 @@ class SignalHandler {
   static void HandleProfilerSignal(int signal, siginfo_t* info, void* context);
 
   // Protects the process wide state below.
-  static base::Mutex* mutex_;
+  static base::LazyMutex mutex_;
   static int client_count_;
   static bool signal_handler_installed_;
   static struct sigaction old_signal_handler_;
 };
 
-base::Mutex* SignalHandler::mutex_ = nullptr;
+base::LazyMutex SignalHandler::mutex_ = LAZY_MUTEX_INITIALIZER;
 int SignalHandler::client_count_ = 0;
 struct sigaction SignalHandler::old_signal_handler_;
 bool SignalHandler::signal_handler_installed_ = false;
@@ -518,31 +512,8 @@ void SignalHandler::FillRegisterState(void* context, RegisterState* state) {
 
 #endif  // USE_SIGNALS
 
-
-void Sampler::SetUp() {
-#if defined(USE_SIGNALS)
-  SignalHandler::SetUp();
-#endif
-}
-
-
-void Sampler::TearDown() {
-#if defined(USE_SIGNALS)
-  SignalHandler::TearDown();
-#endif
-}
-
 Sampler::Sampler(Isolate* isolate)
-    : is_counting_samples_(false),
-      js_sample_count_(0),
-      external_sample_count_(0),
-      isolate_(isolate),
-      profiling_(false),
-      has_processing_thread_(false),
-      active_(false),
-      registered_(false) {
-  data_ = new PlatformData;
-}
+    : isolate_(isolate), data_(base::make_unique<PlatformData>()) {}
 
 void Sampler::UnregisterIfRegistered() {
 #if defined(USE_SIGNALS)
@@ -556,7 +527,6 @@ void Sampler::UnregisterIfRegistered() {
 Sampler::~Sampler() {
   DCHECK(!IsActive());
   DCHECK(!IsRegistered());
-  delete data_;
 }
 
 void Sampler::Start() {
@@ -579,7 +549,7 @@ void Sampler::Stop() {
 
 
 void Sampler::IncreaseProfilingDepth() {
-  base::Relaxed_AtomicIncrement(&profiling_, 1);
+  profiling_++;
 #if defined(USE_SIGNALS)
   SignalHandler::IncreaseSamplerCount();
 #endif
@@ -590,7 +560,7 @@ void Sampler::DecreaseProfilingDepth() {
 #if defined(USE_SIGNALS)
   SignalHandler::DecreaseSamplerCount();
 #endif
-  base::Relaxed_AtomicIncrement(&profiling_, -1);
+  profiling_--;
 }
 
 

src/libsampler/sampler.h

@@ -5,10 +5,10 @@
 #ifndef V8_LIBSAMPLER_SAMPLER_H_
 #define V8_LIBSAMPLER_SAMPLER_H_
 
+#include <atomic>
 #include <unordered_map>
 
 #include "include/v8.h"
-#include "src/base/atomicops.h"
 #include "src/base/lazy-instance.h"
 #include "src/base/macros.h"
 
@@ -31,10 +31,6 @@ class Sampler {
   static const int kMaxFramesCountLog2 = 8;
   static const unsigned kMaxFramesCount = (1u << kMaxFramesCountLog2) - 1;
 
-  // Initializes the Sampler support. Called once at VM startup.
-  static void SetUp();
-  static void TearDown();
-
   // Initialize sampler.
   explicit Sampler(Isolate* isolate);
   virtual ~Sampler();
@@ -52,19 +48,20 @@ class Sampler {
 
   // Whether the sampling thread should use this Sampler for CPU profiling?
   bool IsProfiling() const {
-    return base::Relaxed_Load(&profiling_) > 0 &&
-           !base::Relaxed_Load(&has_processing_thread_);
+    return profiling_.load(std::memory_order_relaxed) > 0;
   }
   void IncreaseProfilingDepth();
   void DecreaseProfilingDepth();
 
   // Whether the sampler is running (that is, consumes resources).
-  bool IsActive() const { return base::Relaxed_Load(&active_) != 0; }
+  bool IsActive() const { return active_.load(std::memory_order_relaxed); }
 
   // CpuProfiler collects samples by calling DoSample directly
   // without calling Start. To keep it working, we register the sampler
   // with the CpuProfiler.
-  bool IsRegistered() const { return base::Relaxed_Load(&registered_) != 0; }
+  bool IsRegistered() const {
+    return registered_.load(std::memory_order_relaxed);
+  }
 
   // The sampler must be unregistered with the SamplerManager before ~Sampler()
   // is called. If this doesn't happen, the signal handler might interrupt
@@ -74,10 +71,6 @@ class Sampler {
 
   void DoSample();
 
-  void SetHasProcessingThread(bool value) {
-    base::Relaxed_Store(&has_processing_thread_, value);
-  }
-
   // Used in tests to make sure that stack sampling is performed.
   unsigned js_sample_count() const { return js_sample_count_; }
   unsigned external_sample_count() const { return external_sample_count_; }
@@ -88,23 +81,26 @@ class Sampler {
   }
 
   class PlatformData;
-  PlatformData* platform_data() const { return data_; }
+  PlatformData* platform_data() const { return data_.get(); }
 
  protected:
   // Counts stack samples taken in various VM states.
-  bool is_counting_samples_;
-  unsigned js_sample_count_;
-  unsigned external_sample_count_;
+  bool is_counting_samples_ = 0;
+  unsigned js_sample_count_ = 0;
+  unsigned external_sample_count_ = 0;
 
-  void SetActive(bool value) { base::Relaxed_Store(&active_, value); }
-  void SetRegistered(bool value) { base::Relaxed_Store(&registered_, value); }
+  void SetActive(bool value) {
+    active_.store(value, std::memory_order_relaxed);
+  }
+  void SetRegistered(bool value) {
+    registered_.store(value, std::memory_order_relaxed);
+  }
 
   Isolate* isolate_;
-  base::Atomic32 profiling_;
-  base::Atomic32 has_processing_thread_;
-  base::Atomic32 active_;
-  base::Atomic32 registered_;
-  PlatformData* data_;  // Platform specific data.
+  std::atomic<std::int32_t> profiling_{0};
+  std::atomic_bool active_{false};
+  std::atomic_bool registered_{false};
+  std::unique_ptr<PlatformData> data_;  // Platform specific data.
   DISALLOW_IMPLICIT_CONSTRUCTORS(Sampler);
 };
 

src/v8.cc

@@ -56,7 +56,6 @@ void V8::TearDown() {
   Bootstrapper::TearDownExtensions();
   ElementsAccessor::TearDown();
   RegisteredExtension::UnregisterAll();
-  sampler::Sampler::TearDown();
   FlagList::ResetAllFlags();  // Frees memory held by string arguments.
 }
 
@@ -90,7 +89,6 @@ void V8::InitializeOncePerProcessImpl() {
 #if defined(USE_SIMULATOR)
   Simulator::InitializeOncePerProcess();
 #endif
-  sampler::Sampler::SetUp();
   CpuFeatures::Probe(false);
   ElementsAccessor::InitializeOncePerProcess();
   Bootstrapper::InitializeOncePerProcess();

test/cctest/libsampler/test-sampler.cc

@@ -73,23 +73,19 @@ static void RunSampler(v8::Local<v8::Context> env,
                        v8::Local<v8::Value> argv[], int argc,
                        unsigned min_js_samples = 0,
                        unsigned min_external_samples = 0) {
-  Sampler::SetUp();
-  TestSampler* sampler = new TestSampler(env->GetIsolate());
-  TestSamplingThread* thread = new TestSamplingThread(sampler);
-  sampler->IncreaseProfilingDepth();
-  sampler->Start();
-  sampler->StartCountingSamples();
-  thread->StartSynchronously();
+  TestSampler sampler(env->GetIsolate());
+  TestSamplingThread thread(&sampler);
+  sampler.IncreaseProfilingDepth();
+  sampler.Start();
+  sampler.StartCountingSamples();
+  thread.StartSynchronously();
   do {
     function->Call(env, env->Global(), argc, argv).ToLocalChecked();
-  } while (sampler->js_sample_count() < min_js_samples ||
-           sampler->external_sample_count() < min_external_samples);
-  sampler->Stop();
-  sampler->DecreaseProfilingDepth();
-  thread->Join();
-  delete thread;
-  delete sampler;
-  Sampler::TearDown();
+  } while (sampler.js_sample_count() < min_js_samples ||
+           sampler.external_sample_count() < min_external_samples);
+  sampler.Stop();
+  sampler.DecreaseProfilingDepth();
+  thread.Join();
 }
 
 }  // namespace