From 41af4c8c4be2315f599a430ec781bce4efa6b9ed Mon Sep 17 00:00:00 2001
From: Benoit Girard <b56girard@gmail.com>
Date: Fri, 18 May 2012 17:03:10 -0400
Subject: [PATCH] Bug 750989 - Pause Profiler during saving. r=mstange,jrmuizel

--HG--
extra : rebase_source : e1a6623d2a0de80423efb76c5b35e7734b119635
---
 tools/profiler/TableTicker.cpp   | 11 +++++++++++
 tools/profiler/platform-linux.cc | 22 ++++++++--------------
 tools/profiler/platform-macos.cc |  6 ++++--
 tools/profiler/platform-win32.cc |  4 +++-
 tools/profiler/platform.h        |  5 +++++
 5 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/tools/profiler/TableTicker.cpp b/tools/profiler/TableTicker.cpp
index f5ae320964a..17258b5f8b0 100644
--- a/tools/profiler/TableTicker.cpp
+++ b/tools/profiler/TableTicker.cpp
@@ -381,8 +381,14 @@ public:
     }
 #endif
 
+    // Pause the profiler during saving.
+    // This will prevent us from recording sampling
+    // regarding profile saving. This will also
+    // prevent bugs caused by the circular buffer not
+    // being thread safe. Bug 750989.
     std::ofstream stream;
     stream.open(buff);
+    t->SetPaused(true);
     if (stream.is_open()) {
       stream << *(t->GetPrimaryThreadProfile());
       stream << "h-" << GetSharedLibraryInfoString() << std::endl;
@@ -391,6 +397,7 @@ public:
     } else {
       LOG("Fail to open profile log file.");
     }
+    t->SetPaused(false);
 
     return NS_OK;
   }
@@ -422,8 +429,10 @@ JSObject* TableTicker::ToJSObject(JSContext *aCx)
   b.DefineProperty(profile, "threads", threads);
 
   // For now we only have one thread
+  SetPaused(true);
   JSObject* threadSamples = GetPrimaryThreadProfile()->ToJSObject(aCx);
   b.ArrayPush(threads, threadSamples);
+  SetPaused(false);
 
   return profile;
 }
@@ -719,7 +728,9 @@ char* mozilla_sampler_get_profile()
   }
 
   std::stringstream profile;
+  t->SetPaused(true);
   profile << *(t->GetPrimaryThreadProfile());
+  t->SetPaused(false);
 
   std::string profileString = profile.str();
   char *rtn = (char*)malloc( (profileString.length() + 1) * sizeof(char) );
diff --git a/tools/profiler/platform-linux.cc b/tools/profiler/platform-linux.cc
index 9f3dd1930e2..ad1810710dd 100644
--- a/tools/profiler/platform-linux.cc
+++ b/tools/profiler/platform-linux.cc
@@ -139,28 +139,21 @@ class Sampler::PlatformData : public Malloced {
     while (sampler_->IsActive()) {
       sampler_->HandleSaveRequest();
 
+      if (!sampler_->IsPaused()) {
 #ifdef XP_MACOSX
-      pthread_kill(signal_receiver_, SIGPROF);
+        pthread_kill(signal_receiver_, SIGPROF);
 #else
-      // Glibc doesn't provide a wrapper for tgkill(2).
-      tgkill(vm_tgid_, vm_tid_, SIGPROF);
+        // Glibc doesn't provide a wrapper for tgkill(2).
+        tgkill(vm_tgid_, vm_tid_, SIGPROF);
 #endif
+      }
+
       // Convert ms to us and subtract 100 us to compensate delays
       // occuring during signal delivery.
-
       // TODO measure and confirm this.
       const useconds_t interval = sampler_->interval_ * 1000 - 100;
       //int result = usleep(interval);
       usleep(interval);
-      // sometimes usleep is defined as returning void
-      int result = 0;
-#ifdef DEBUG
-      if (result != 0 && errno != EINTR) {
-        LOG("SignalSender usleep error");
-        ASSERT(result == 0 || errno == EINTR);
-      }
-#endif
-      mozilla::unused << result;
     }
   }
 
@@ -189,7 +182,8 @@ static void* SenderEntry(void* arg) {
 Sampler::Sampler(int interval, bool profiling)
     : interval_(interval),
       profiling_(profiling),
-      active_(false) {
+      active_(false),
+      paused_(false) {
   data_ = new PlatformData(this);
 }
 
diff --git a/tools/profiler/platform-macos.cc b/tools/profiler/platform-macos.cc
index 79073ab45aa..254563ddfff 100644
--- a/tools/profiler/platform-macos.cc
+++ b/tools/profiler/platform-macos.cc
@@ -223,7 +223,8 @@ class SamplerThread : public Thread {
   // Implement Thread::Run().
   virtual void Run() {
     while (SamplerRegistry::sampler->IsActive()) {
-      SampleContext(SamplerRegistry::sampler);
+      if (!SamplerRegistry::sampler->IsPaused())
+        SampleContext(SamplerRegistry::sampler);
       OS::Sleep(interval_);
     }
   }
@@ -295,7 +296,8 @@ Sampler::Sampler(int interval, bool profiling)
     : // isolate_(isolate),
       interval_(interval),
       profiling_(profiling),
-      active_(false) /*,
+      active_(false),
+      paused_(false) /*,
       samples_taken_(0)*/ {
   data_ = new PlatformData;
 }
diff --git a/tools/profiler/platform-win32.cc b/tools/profiler/platform-win32.cc
index 952e2441974..c4578b9ee2b 100644
--- a/tools/profiler/platform-win32.cc
+++ b/tools/profiler/platform-win32.cc
@@ -64,7 +64,8 @@ class SamplerThread : public Thread {
   // Implement Thread::Run().
   virtual void Run() {
     while (sampler_->IsActive()) {
-      SampleContext(sampler_);
+      if (!sampler_->IsPaused())
+        SampleContext(sampler_);
       OS::Sleep(interval_);
     }
   }
@@ -121,6 +122,7 @@ Sampler::Sampler(int interval, bool profiling)
     : interval_(interval),
       profiling_(profiling),
       active_(false),
+      paused_(false),
       data_(new PlatformData) {
 }
 
diff --git a/tools/profiler/platform.h b/tools/profiler/platform.h
index 21a3334ecfa..61a50ead81d 100644
--- a/tools/profiler/platform.h
+++ b/tools/profiler/platform.h
@@ -211,6 +211,10 @@ class Sampler {
   // Whether the sampler is running (that is, consumes resources).
   bool IsActive() const { return active_; }
 
+  // Low overhead way to stop the sampler from ticking
+  bool IsPaused() const { return paused_; }
+  void SetPaused(bool value) { NoBarrier_Store(&paused_, value); }
+
   class PlatformData;
 
   PlatformData* platform_data() { return data_; }
@@ -229,6 +233,7 @@ class Sampler {
 
   const int interval_;
   const bool profiling_;
+  Atomic32 paused_;
   Atomic32 active_;
   PlatformData* data_;  // Platform specific data.
 };