Get rid of UnsafeCurrent in Sampler

src/profiler/sampler.cc

Index: src/profiler/sampler.cc
diff --git a/src/profiler/sampler.cc b/src/profiler/sampler.cc
index a34042453c902d25a2ced72494a47fe4896e9263..a4cd4ac4832f1c218cd5040835c6bc842da1bf13 100644
--- a/src/profiler/sampler.cc
+++ b/src/profiler/sampler.cc
@@ -42,6 +42,7 @@
 
 #endif
 
+#include "src/atomic-utils.h"
 #include "src/base/platform/platform.h"
 #include "src/flags.h"
 #include "src/frames-inl.h"
@@ -236,6 +237,38 @@ bool IsNoFrameRegion(Address address) {
   return false;
 }
 
+typedef List<Sampler*> SamplerList;
+
+#if defined(USE_SIGNALS)
+class AtomicGuard {
+ public:
+  AtomicGuard(bool is_block, AtomicValue<int>* atomic,

[alph, 2016/04/07 22:38:16]

nit: why did you swap the arguments?
is_blocking should go second. also you can make it default to true.

[lpy, 2016/04/07 23:19:19]

Done.

+              int expected = 0, int desired = 1)

[alph, 2016/04/07 22:38:17]

just drop these. if at some point in the future they will be needed (what I
doubt), we can reintroduce them.

[lpy, 2016/04/07 23:19:19]

Done.

+      : atomic_(atomic),
+        expected_(expected),
+        is_success_(false) {
+    do {
+      is_success_ = atomic_->TrySetValue(expected, desired);
+      if (is_success_) break;
+    } while (is_block);

[alph, 2016/04/07 22:38:17]

still, why don't you want to change it to:
do { } while (is_block && !is_success_);

[lpy, 2016/04/07 23:19:19]

Done.

+  }
+
+  bool is_success() { return is_success_; }
+
+  ~AtomicGuard() {
+    if (is_success_) {
+      atomic_->SetValue(expected_);
+    }
+    atomic_ = NULL;
+  }
+
+ private:
+  AtomicValue<int>* atomic_;
+  int expected_;
+  bool is_success_;
+};
+#endif  // USE_SIGNALS
+
 }  // namespace
 
 #if defined(USE_SIGNALS)
@@ -374,6 +407,10 @@ class SignalHandler : public AllStatic {
     return signal_handler_installed_;
   }
 
+#if !V8_OS_NACL
+  static void CollectSample(void* context, Sampler* sampler);
+#endif
+
  private:
   static void Install() {
 #if !V8_OS_NACL
@@ -418,23 +455,19 @@ bool SignalHandler::signal_handler_installed_ = false;
 
 // As Native Client does not support signal handling, profiling is disabled.
 #if !V8_OS_NACL
-void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
-                                         void* context) {
-  USE(info);
-  if (signal != SIGPROF) return;
-  Isolate* isolate = Isolate::UnsafeCurrent();
-  if (isolate == NULL || !isolate->IsInUse()) {
-    // We require a fully initialized and entered isolate.
+void SignalHandler::CollectSample(void* context, Sampler* sampler) {
+  if (sampler == NULL || (!sampler->IsProfiling() && !sampler->IsRegistered()))
     return;
-  }
+  Isolate* isolate = sampler->isolate();
+
+  // We require a fully initialized and entered isolate.
+  if (isolate == NULL || !isolate->IsInUse()) return;
+
   if (v8::Locker::IsActive() &&
       !isolate->thread_manager()->IsLockedByCurrentThread()) {
     return;
   }
 
-  Sampler* sampler = isolate->logger()->sampler();
-  if (sampler == NULL) return;
-
   v8::RegisterState state;
 
 #if defined(USE_SIMULATOR)
@@ -582,7 +615,7 @@ void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
 }
 #endif  // V8_OS_NACL
 
-#endif
+#endif  // USE_SIGNALS
 
 
 class SamplerThread : public base::Thread {
@@ -607,19 +640,48 @@ class SamplerThread : public base::Thread {
     }
 
     DCHECK(sampler->IsActive());
-    DCHECK(!instance_->active_samplers_.Contains(sampler));
     DCHECK(instance_->interval_ == sampler->interval());
+
+#if defined(USE_SIGNALS)
+    AddSampler(sampler);
+#else
+    DCHECK(!instance_->active_samplers_.Contains(sampler));
     instance_->active_samplers_.Add(sampler);
+#endif  // USE_SIGNALS
 
     if (need_to_start) instance_->StartSynchronously();
   }
 
-  static void RemoveActiveSampler(Sampler* sampler) {
+  static void RemoveSampler(Sampler* sampler) {
     SamplerThread* instance_to_remove = NULL;
     {
       base::LockGuard<base::Mutex> lock_guard(mutex_);
 
-      DCHECK(sampler->IsActive());
+      DCHECK(sampler->IsActive() || sampler->IsRegistered());
+#if defined(USE_SIGNALS)
+      {
+        AtomicGuard atomic_guard(true, &sampler_list_access_counter_);
+        // Remove sampler from map.
+        pthread_t thread_id = sampler->platform_data()->vm_tid();
+        uint32_t profiled_thread_id =
+            sampler->platform_data()->profiled_thread_id().ToInteger();
+        HashMap::Entry* entry =
+            thread_id_to_samplers_.Lookup(reinterpret_cast<void*>(thread_id),
+                                          profiled_thread_id);
+        DCHECK(entry != NULL);
+        SamplerList* samplers = reinterpret_cast<SamplerList*>(entry->value);
+        samplers->RemoveElement(sampler);
+        if (samplers->is_empty()) {
+          thread_id_to_samplers_.Remove(reinterpret_cast<void*>(thread_id),
+                                        profiled_thread_id);
+          delete samplers;
+        }
+        if (thread_id_to_samplers_.occupancy() == 0) {
+          instance_to_remove = instance_;
+          instance_ = NULL;
+        }
+      }
+#else
       bool removed = instance_->active_samplers_.RemoveElement(sampler);
       DCHECK(removed);
       USE(removed);
@@ -630,6 +692,7 @@ class SamplerThread : public base::Thread {
         instance_to_remove = instance_;
         instance_ = NULL;
       }
+#endif  // USE_SIGNALS
     }
 
     if (!instance_to_remove) return;
@@ -637,11 +700,30 @@ class SamplerThread : public base::Thread {
     delete instance_to_remove;
   }
 
+  // Unlike AddActiveSampler, this method only adds a sampler,
+  // but won't start the sampler thread.
+  static void RegisterSampler(Sampler* sampler) {
+    base::LockGuard<base::Mutex> lock_guard(mutex_);
+#if defined(USE_SIGNALS)
+    AddSampler(sampler);
+#endif  // USE_SIGNALS
+  }
+
   // Implement Thread::Run().
   virtual void Run() {
     while (true) {
       {
         base::LockGuard<base::Mutex> lock_guard(mutex_);
+#if defined(USE_SIGNALS)
+        if (thread_id_to_samplers_.occupancy() == 0) break;
+        if (SignalHandler::Installed()) {
+          for (HashMap::Entry *p = thread_id_to_samplers_.Start(); p != NULL;
+               p = thread_id_to_samplers_.Next(p)) {
+            pthread_t thread_id = reinterpret_cast<pthread_t>(p->key);
+            pthread_kill(thread_id, SIGPROF);
+          }
+        }
+#else
         if (active_samplers_.is_empty()) break;
         // When CPU profiling is enabled both JavaScript and C++ code is
         // profiled. We must not suspend.
@@ -650,6 +732,7 @@ class SamplerThread : public base::Thread {
           if (!sampler->IsProfiling()) continue;
           sampler->DoSample();
         }
+#endif  // USE_SIGNALS
       }
       base::OS::Sleep(base::TimeDelta::FromMilliseconds(interval_));
     }
@@ -661,7 +744,33 @@ class SamplerThread : public base::Thread {
   static SamplerThread* instance_;
 
   const int interval_;
-  List<Sampler*> active_samplers_;
+
+#if defined(USE_SIGNALS)
+  friend class SignalHandler;
+  static HashMap thread_id_to_samplers_;
+  static AtomicValue<int> sampler_list_access_counter_;
+  static void AddSampler(Sampler* sampler) {
+    AtomicGuard atomic_guard(true, &sampler_list_access_counter_);
+    // Add sampler into map if needed.
+    pthread_t thread_id = sampler->platform_data()->vm_tid();
+    HashMap::Entry *entry =
+        thread_id_to_samplers_.LookupOrInsert(
+            reinterpret_cast<void*>(thread_id),
+            sampler->platform_data()->profiled_thread_id().ToInteger());
+    if (entry->value == NULL) {
+      SamplerList* samplers = new SamplerList();
+      samplers->Add(sampler);
+      entry->value = samplers;
+    } else {
+      SamplerList* samplers = reinterpret_cast<SamplerList*>(entry->value);
+      if (!samplers->Contains(sampler)) {
+        samplers->Add(sampler);
+      }
+    }
+  }
+#else
+  SamplerList active_samplers_;
+#endif  // USE_SIGNALS
 
   DISALLOW_COPY_AND_ASSIGN(SamplerThread);
 };
@@ -669,6 +778,38 @@ class SamplerThread : public base::Thread {
 
 base::Mutex* SamplerThread::mutex_ = NULL;
 SamplerThread* SamplerThread::instance_ = NULL;
+#if defined(USE_SIGNALS)
+HashMap SamplerThread::thread_id_to_samplers_(HashMap::PointersMatch);
+AtomicValue<int> SamplerThread::sampler_list_access_counter_(0);
+
+// As Native Client does not support signal handling, profiling is disabled.
+#if !V8_OS_NACL
+void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
+                                         void* context) {
+  USE(info);
+  if (signal != SIGPROF) return;
+  AtomicGuard atomic_guard(false, &SamplerThread::sampler_list_access_counter_);
+  if (!atomic_guard.is_success()) return;
+  pthread_t thread_id = pthread_self();
+  HashMap::Entry* entry = NULL;
+  for (HashMap::Entry *p = SamplerThread::thread_id_to_samplers_.Start();
+       p != NULL; p = SamplerThread::thread_id_to_samplers_.Next(p)) {
+    if (pthread_equal(thread_id, reinterpret_cast<pthread_t>(p->key)) != 0) {
+      entry = p;
+      break;
+    }
+  }
+  if (entry == NULL) {

[alph, 2016/04/07 22:38:16]

nit: drop {}

[lpy, 2016/04/07 23:19:19]

Done.

+    return;
+  }
+  SamplerList* samplers = reinterpret_cast<SamplerList*>(entry->value);
+  for (int i = 0; i < samplers->length(); ++i) {
+    Sampler* sampler = samplers->at(i);
+    CollectSample(context, sampler);
+  }
+}
+#endif  // !V8_OS_NACL
+#endif  // USE_SIGNALs
 
 
 //
@@ -789,6 +930,7 @@ Sampler::Sampler(Isolate* isolate, int interval)
       profiling_(false),
       has_processing_thread_(false),
       active_(false),
+      registered_(false),
       is_counting_samples_(false),
       js_sample_count_(0),
       external_sample_count_(0) {
@@ -797,6 +939,9 @@ Sampler::Sampler(Isolate* isolate, int interval)
 
 Sampler::~Sampler() {
   DCHECK(!IsActive());
+  if (IsRegistered()) {
+    SamplerThread::RemoveSampler(this);
+  }
   delete data_;
 }
 
@@ -809,8 +954,9 @@ void Sampler::Start() {
 
 void Sampler::Stop() {
   DCHECK(IsActive());
-  SamplerThread::RemoveActiveSampler(this);
+  SamplerThread::RemoveSampler(this);
   SetActive(false);
+  SetRegistered(false);
 }
 
 
@@ -850,6 +996,10 @@ void Sampler::SampleStack(const v8::RegisterState& state) {
 
 void Sampler::DoSample() {
   if (!SignalHandler::Installed()) return;
+  if (!IsActive() && !IsRegistered()) {
+    SamplerThread::RegisterSampler(this);
+    SetRegistered(true);
+  }
   pthread_kill(platform_data()->vm_tid(), SIGPROF);
 }