Try to simplify the semantics of the profiling code by making

sure to suspend the thread (if necessary) on mac/win32 before
reading the VM state.

Avoid dealing with signals delivered to non-VM threads on
linux no matter if we're profiling or not.
Review URL: http://codereview.chromium.org/3845006

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5641 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/log.cc b/src/log.cc
index 4230cba..d0e0c21 100644 (file)
--- a/src/log.cc
+++ b/src/log.cc
@@ -191,11 +191,12 @@ class Ticker: public Sampler {
 
   ~Ticker() { if (IsActive()) Stop(); }
 
-  void SampleStack(TickSample* sample) {
+  virtual void SampleStack(TickSample* sample) {
+    ASSERT(IsSynchronous());
     StackTracer::Trace(sample);
   }
 
-  void Tick(TickSample* sample) {
+  virtual void Tick(TickSample* sample) {
     if (profiler_) profiler_->Insert(sample);
     if (window_) window_->AddState(sample->state);
   }

diff --git a/src/platform-linux.cc b/src/platform-linux.cc
index f7d8609..8bb7abf 100644 (file)
--- a/src/platform-linux.cc
+++ b/src/platform-linux.cc
@@ -748,6 +748,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) {
   USE(info);
   if (signal != SIGPROF) return;
   if (active_sampler_ == NULL) return;
+  if (!IsVmThread()) return;
 
   TickSample sample_obj;
   TickSample* sample = CpuProfiler::TickSampleEvent();
@@ -755,6 +756,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) {
 
   // We always sample the VM state.
   sample->state = VMState::current_state();
+
   // If profiling, we extract the current pc and sp.
   if (active_sampler_->IsProfiling()) {
     // Extracting the sample from the context is extremely machine dependent.
@@ -783,9 +785,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) {
     // Implement this on MIPS.
     UNIMPLEMENTED();
 #endif
-    if (IsVmThread()) {
-      active_sampler_->SampleStack(sample);
-    }
+    active_sampler_->SampleStack(sample);
   }
 
   active_sampler_->Tick(sample);
@@ -806,7 +806,10 @@ class Sampler::PlatformData : public Malloced {
 
 
 Sampler::Sampler(int interval, bool profiling)
-    : interval_(interval), profiling_(profiling), active_(false) {
+    : interval_(interval),
+      profiling_(profiling),
+      synchronous_(profiling),
+      active_(false) {
   data_ = new PlatformData();
 }
 

diff --git a/src/platform-macos.cc b/src/platform-macos.cc
index 47193de..67da3c1 100644 (file)
--- a/src/platform-macos.cc
+++ b/src/platform-macos.cc
@@ -549,17 +549,24 @@ class Sampler::PlatformData : public Malloced {
 
   // Sampler thread handler.
   void Runner() {
-    // Loop until the sampler is disengaged, keeping the specified samling freq.
+    // Loop until the sampler is disengaged, keeping the specified
+    // sampling frequency.
     for ( ; sampler_->IsActive(); OS::Sleep(sampler_->interval_)) {
       TickSample sample_obj;
       TickSample* sample = CpuProfiler::TickSampleEvent();
       if (sample == NULL) sample = &sample_obj;
 
+      // If the sampler runs in sync with the JS thread, we try to
+      // suspend it. If we fail, we skip the current sample.
+      if (sampler_->IsSynchronous()) {
+        if (KERN_SUCCESS != thread_suspend(profiled_thread_)) continue;
+      }
+
       // We always sample the VM state.
       sample->state = VMState::current_state();
+
       // If profiling, we record the pc and sp of the profiled thread.
-      if (sampler_->IsProfiling()
-          && KERN_SUCCESS == thread_suspend(profiled_thread_)) {
+      if (sampler_->IsProfiling()) {
 #if V8_HOST_ARCH_X64
         thread_state_flavor_t flavor = x86_THREAD_STATE64;
         x86_thread_state64_t state;
@@ -591,11 +598,14 @@ class Sampler::PlatformData : public Malloced {
           sample->fp = reinterpret_cast<Address>(state.REGISTER_FIELD(bp));
           sampler_->SampleStack(sample);
         }
-        thread_resume(profiled_thread_);
       }
 
       // Invoke tick handler with program counter and stack pointer.
       sampler_->Tick(sample);
+
+      // If the sampler runs in sync with the JS thread, we have to
+      // remember to resume it.
+      if (sampler_->IsSynchronous()) thread_resume(profiled_thread_);
     }
   }
 };
@@ -613,7 +623,10 @@ static void* SamplerEntry(void* arg) {
 
 
 Sampler::Sampler(int interval, bool profiling)
-    : interval_(interval), profiling_(profiling), active_(false) {
+    : interval_(interval),
+      profiling_(profiling),
+      synchronous_(profiling),
+      active_(false) {
   data_ = new PlatformData(this);
 }
 
@@ -624,9 +637,9 @@ Sampler::~Sampler() {
 
 
 void Sampler::Start() {
-  // If we are profiling, we need to be able to access the calling
-  // thread.
-  if (IsProfiling()) {
+  // If we are starting a synchronous sampler, we need to be able to
+  // access the calling thread.
+  if (IsSynchronous()) {
     data_->profiled_thread_ = mach_thread_self();
   }
 
@@ -655,7 +668,7 @@ void Sampler::Stop() {
   pthread_join(data_->sampler_thread_, NULL);
 
   // Deallocate Mach port for thread.
-  if (IsProfiling()) {
+  if (IsSynchronous()) {
     mach_port_deallocate(data_->task_self_, data_->profiled_thread_);
   }
 }

diff --git a/src/platform-win32.cc b/src/platform-win32.cc
index 86314a8..964d7bf 100644 (file)
--- a/src/platform-win32.cc
+++ b/src/platform-win32.cc
@@ -1838,17 +1838,25 @@ class Sampler::PlatformData : public Malloced {
     // Context used for sampling the register state of the profiled thread.
     CONTEXT context;
     memset(&context, 0, sizeof(context));
-    // Loop until the sampler is disengaged, keeping the specified samling freq.
+    // Loop until the sampler is disengaged, keeping the specified
+    // sampling frequency.
     for ( ; sampler_->IsActive(); Sleep(sampler_->interval_)) {
       TickSample sample_obj;
       TickSample* sample = CpuProfiler::TickSampleEvent();
       if (sample == NULL) sample = &sample_obj;
 
+      // If the sampler runs in sync with the JS thread, we try to
+      // suspend it. If we fail, we skip the current sample.
+      if (sampler_->IsSynchronous()) {
+        static const DWORD kSuspendFailed = static_cast<DWORD>(-1);
+        if (SuspendThread(profiled_thread_) == kSuspendFailed) continue;
+      }
+
       // We always sample the VM state.
       sample->state = VMState::current_state();
+
       // If profiling, we record the pc and sp of the profiled thread.
-      if (sampler_->IsProfiling()
-          && SuspendThread(profiled_thread_) != (DWORD)-1) {
+      if (sampler_->IsProfiling()) {
         context.ContextFlags = CONTEXT_FULL;
         if (GetThreadContext(profiled_thread_, &context) != 0) {
 #if V8_HOST_ARCH_X64
@@ -1862,11 +1870,14 @@ class Sampler::PlatformData : public Malloced {
 #endif
           sampler_->SampleStack(sample);
         }
-        ResumeThread(profiled_thread_);
       }
 
       // Invoke tick handler with program counter and stack pointer.
       sampler_->Tick(sample);
+
+      // If the sampler runs in sync with the JS thread, we have to
+      // remember to resume it.
+      if (sampler_->IsSynchronous()) ResumeThread(profiled_thread_);
     }
   }
 };
@@ -1883,7 +1894,10 @@ static unsigned int __stdcall SamplerEntry(void* arg) {
 
 // Initialize a profile sampler.
 Sampler::Sampler(int interval, bool profiling)
-    : interval_(interval), profiling_(profiling), active_(false) {
+    : interval_(interval),
+      profiling_(profiling),
+      synchronous_(profiling),
+      active_(false) {
   data_ = new PlatformData(this);
 }
 
@@ -1895,9 +1909,9 @@ Sampler::~Sampler() {
 
 // Start profiling.
 void Sampler::Start() {
-  // If we are profiling, we need to be able to access the calling
-  // thread.
-  if (IsProfiling()) {
+  // If we are starting a synchronous sampler, we need to be able to
+  // access the calling thread.
+  if (IsSynchronous()) {
     // Get a handle to the calling thread. This is the thread that we are
     // going to profile. We need to make a copy of the handle because we are
     // going to use it in the sampler thread. Using GetThreadHandle() will

diff --git a/src/platform.h b/src/platform.h
index e9e7c22..12a0253 100644 (file)
--- a/src/platform.h
+++ b/src/platform.h
@@ -563,17 +563,24 @@ class Sampler {
   void Start();
   void Stop();
 
-  // Is the sampler used for profiling.
-  inline bool IsProfiling() { return profiling_; }
+  // Is the sampler used for profiling?
+  bool IsProfiling() const { return profiling_; }
+
+  // Is the sampler running in sync with the JS thread? On platforms
+  // where the sampler is implemented with a thread that wakes up
+  // every now and then, having a synchronous sampler implies
+  // suspending/resuming the JS thread.
+  bool IsSynchronous() const { return synchronous_; }
 
   // Whether the sampler is running (that is, consumes resources).
-  inline bool IsActive() { return active_; }
+  bool IsActive() const { return active_; }
 
   class PlatformData;
 
  private:
   const int interval_;
   const bool profiling_;
+  const bool synchronous_;
   bool active_;
   PlatformData* data_;  // Platform specific data.
   DISALLOW_IMPLICIT_CONSTRUCTORS(Sampler);

diff --git a/test/cctest/test-log.cc b/test/cctest/test-log.cc
index b364ae3..16d0f00 100644 (file)
--- a/test/cctest/test-log.cc
+++ b/test/cctest/test-log.cc
@@ -469,7 +469,7 @@ TEST(ProfMultipleThreads) {
   CHECK(!sampler.WasSampleStackCalled());
   nonJsThread.WaitForRunning();
   nonJsThread.SendSigProf();
-  CHECK(sampler.WaitForTick());
+  CHECK(!sampler.WaitForTick());
   CHECK(!sampler.WasSampleStackCalled());
   sampler.Stop();