From cabe82959fb6a5fd177f14ab752c855ac38264bd Mon Sep 17 00:00:00 2001 From: "kasperl@chromium.org" Date: Mon, 18 Oct 2010 12:37:07 +0000 Subject: [PATCH] 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 --- src/log.cc | 5 +++-- src/platform-linux.cc | 11 +++++++---- src/platform-macos.cc | 31 ++++++++++++++++++++++--------- src/platform-win32.cc | 30 ++++++++++++++++++++++-------- src/platform.h | 13 ++++++++++--- test/cctest/test-log.cc | 2 +- 6 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/log.cc b/src/log.cc index 4230cba..d0e0c21 100644 --- 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 --- 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 --- 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
(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 --- 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(-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 --- 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 --- 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(); -- 2.7.4