From ecc7f4baada4d220ba0e1bc89d14bff3d6f4bbb8 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Tue, 2 Oct 2012 10:51:00 +0000 Subject: [PATCH] Replacing circular queue by single buffer in CPU Profiler. BUG=None Review URL: https://codereview.chromium.org/10871039 Patch from Sergey Rogulenko . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12650 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/cpu-profiler-inl.h | 16 +++++++---- src/cpu-profiler.cc | 57 +++++++++++++++++----------------------- src/cpu-profiler.h | 22 ++++++++++------ src/platform-cygwin.cc | 6 ++--- src/platform-freebsd.cc | 6 ++--- src/platform-linux.cc | 6 ++--- src/platform-macos.cc | 6 ++--- src/platform-openbsd.cc | 6 ++--- src/platform-solaris.cc | 6 ++--- src/platform-win32.cc | 6 ++--- test/cctest/test-cpu-profiler.cc | 33 +++++++++++++---------- 11 files changed, 89 insertions(+), 81 deletions(-) diff --git a/src/cpu-profiler-inl.h b/src/cpu-profiler-inl.h index 4982197..1133b20 100644 --- a/src/cpu-profiler-inl.h +++ b/src/cpu-profiler-inl.h @@ -31,7 +31,6 @@ #include "cpu-profiler.h" #include -#include "circular-queue-inl.h" #include "profile-generator-inl.h" #include "unbound-queue-inl.h" @@ -56,11 +55,18 @@ void SharedFunctionInfoMoveEventRecord::UpdateCodeMap(CodeMap* code_map) { } -TickSample* ProfilerEventsProcessor::TickSampleEvent() { +TickSample* ProfilerEventsProcessor::StartTickSampleEvent() { + if (!ticks_buffer_is_empty_ || ticks_buffer_is_initialized_) return NULL; + ticks_buffer_is_initialized_ = true; generator_->Tick(); - TickSampleEventRecord* evt = - new(ticks_buffer_.Enqueue()) TickSampleEventRecord(enqueue_order_); - return &evt->sample; + ticks_buffer_ = TickSampleEventRecord(enqueue_order_); + return &ticks_buffer_.sample; +} + + +void ProfilerEventsProcessor::FinishTickSampleEvent() { + ASSERT(ticks_buffer_is_initialized_ && ticks_buffer_is_empty_); + ticks_buffer_is_empty_ = false; } diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.cc index 0caead0..8f72c17 100644 --- a/src/cpu-profiler.cc +++ b/src/cpu-profiler.cc @@ -39,9 +39,6 @@ namespace v8 { namespace internal { -static const int kEventsBufferSize = 256 * KB; -static const int kTickSamplesBufferChunkSize = 64 * KB; -static const int kTickSamplesBufferChunksCount = 16; static const int kProfilerStackSize = 64 * KB; @@ -53,9 +50,8 @@ ProfilerEventsProcessor::ProfilerEventsProcessor(ProfileGenerator* generator, sampler_(sampler), running_(true), period_in_useconds_(period_in_useconds), - ticks_buffer_(sizeof(TickSampleEventRecord), - kTickSamplesBufferChunkSize, - kTickSamplesBufferChunksCount), + ticks_buffer_is_empty_(true), + ticks_buffer_is_initialized_(false), enqueue_order_(0) { } @@ -210,9 +206,8 @@ bool ProfilerEventsProcessor::ProcessCodeEvent(unsigned* dequeue_order) { } -bool ProfilerEventsProcessor::ProcessTicks(int64_t stop_time, - unsigned dequeue_order) { - while (stop_time == -1 || OS::Ticks() < stop_time) { +bool ProfilerEventsProcessor::ProcessTicks(unsigned dequeue_order) { + while (true) { if (!ticks_from_vm_buffer_.IsEmpty() && ticks_from_vm_buffer_.Peek()->order == dequeue_order) { TickSampleEventRecord record; @@ -220,35 +215,28 @@ bool ProfilerEventsProcessor::ProcessTicks(int64_t stop_time, generator_->RecordTickSample(record.sample); } - const TickSampleEventRecord* rec = - TickSampleEventRecord::cast(ticks_buffer_.StartDequeue()); - if (rec == NULL) return !ticks_from_vm_buffer_.IsEmpty(); - // Make a local copy of tick sample record to ensure that it won't - // be modified as we are processing it. This is possible as the - // sampler writes w/o any sync to the queue, so if the processor - // will get far behind, a record may be modified right under its - // feet. - TickSampleEventRecord record = *rec; - if (record.order == dequeue_order) { + if (ticks_buffer_is_empty_) return !ticks_from_vm_buffer_.IsEmpty(); + if (ticks_buffer_.order == dequeue_order) { // A paranoid check to make sure that we don't get a memory overrun // in case of frames_count having a wild value. - if (record.sample.frames_count < 0 - || record.sample.frames_count > TickSample::kMaxFramesCount) - record.sample.frames_count = 0; - generator_->RecordTickSample(record.sample); - ticks_buffer_.FinishDequeue(); + if (ticks_buffer_.sample.frames_count < 0 + || ticks_buffer_.sample.frames_count > TickSample::kMaxFramesCount) { + ticks_buffer_.sample.frames_count = 0; + } + generator_->RecordTickSample(ticks_buffer_.sample); + ticks_buffer_is_empty_ = true; + ticks_buffer_is_initialized_ = false; } else { return true; } } - return false; } void ProfilerEventsProcessor::ProcessEventsQueue(int64_t stop_time, unsigned* dequeue_order) { while (OS::Ticks() < stop_time) { - if (ProcessTicks(stop_time, *dequeue_order)) { + if (ProcessTicks(*dequeue_order)) { // All ticks of the current dequeue_order are processed, // proceed to the next code event. ProcessCodeEvent(dequeue_order); @@ -268,11 +256,7 @@ void ProfilerEventsProcessor::Run() { ProcessEventsQueue(stop_time, &dequeue_order); } - // Process remaining tick events. - ticks_buffer_.FlushResidualRecords(); - // Perform processing until we have tick events, skip remaining code events. - while (ProcessTicks(-1, dequeue_order) && ProcessCodeEvent(&dequeue_order)) { - } + while (ProcessTicks(dequeue_order) && ProcessCodeEvent(&dequeue_order)) { } } @@ -327,15 +311,22 @@ CpuProfile* CpuProfiler::FindProfile(Object* security_token, unsigned uid) { } -TickSample* CpuProfiler::TickSampleEvent(Isolate* isolate) { +TickSample* CpuProfiler::StartTickSampleEvent(Isolate* isolate) { if (CpuProfiler::is_profiling(isolate)) { - return isolate->cpu_profiler()->processor_->TickSampleEvent(); + return isolate->cpu_profiler()->processor_->StartTickSampleEvent(); } else { return NULL; } } +void CpuProfiler::FinishTickSampleEvent(Isolate* isolate) { + if (CpuProfiler::is_profiling(isolate)) { + isolate->cpu_profiler()->processor_->FinishTickSampleEvent(); + } +} + + void CpuProfiler::DeleteAllProfiles() { Isolate* isolate = Isolate::Current(); ASSERT(isolate->cpu_profiler() != NULL); diff --git a/src/cpu-profiler.h b/src/cpu-profiler.h index d335ded..f4bc0c7 100644 --- a/src/cpu-profiler.h +++ b/src/cpu-profiler.h @@ -158,11 +158,12 @@ class ProfilerEventsProcessor : public Thread { // Puts current stack into tick sample events buffer. void AddCurrentStack(); - // Tick sample events are filled directly in the buffer of the circular - // queue (because the structure is of fixed width, but usually not all - // stack frame entries are filled.) This method returns a pointer to the - // next record of the buffer. - INLINE(TickSample* TickSampleEvent()); + // StartTickSampleEvent returns a pointer only if the ticks_buffer_ is empty, + // FinishTickSampleEvent marks the ticks_buffer_ as filled. + // Finish should be called only after successful Start (returning non-NULL + // pointer). + INLINE(TickSample* StartTickSampleEvent()); + INLINE(void FinishTickSampleEvent()); private: union CodeEventsContainer { @@ -174,7 +175,7 @@ class ProfilerEventsProcessor : public Thread { // Called from events processing thread (Run() method.) bool ProcessCodeEvent(unsigned* dequeue_order); - bool ProcessTicks(int64_t stop_time, unsigned dequeue_order); + bool ProcessTicks(unsigned dequeue_order); void ProcessEventsQueue(int64_t stop_time, unsigned* dequeue_order); INLINE(static bool FilterOutCodeCreateEvent(Logger::LogEventsAndTags tag)); @@ -185,7 +186,9 @@ class ProfilerEventsProcessor : public Thread { // Sampling period in microseconds. const int period_in_useconds_; UnboundQueue events_buffer_; - SamplingCircularQueue ticks_buffer_; + TickSampleEventRecord ticks_buffer_; + bool ticks_buffer_is_empty_; + bool ticks_buffer_is_initialized_; UnboundQueue ticks_from_vm_buffer_; unsigned enqueue_order_; }; @@ -224,7 +227,10 @@ class CpuProfiler { static bool HasDetachedProfiles(); // Invoked from stack sampler (thread or signal handler.) - static TickSample* TickSampleEvent(Isolate* isolate); + // Finish should be called only after successful Start (returning non-NULL + // pointer). + static TickSample* StartTickSampleEvent(Isolate* isolate); + static void FinishTickSampleEvent(Isolate* isolate); // Must be called via PROFILE macro, otherwise will crash when // profiling is not enabled. diff --git a/src/platform-cygwin.cc b/src/platform-cygwin.cc index 702b1d2..b39dfc0 100644 --- a/src/platform-cygwin.cc +++ b/src/platform-cygwin.cc @@ -692,9 +692,8 @@ class SamplerThread : public Thread { CONTEXT context; memset(&context, 0, sizeof(context)); - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(sampler->isolate()); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(sampler->isolate()); + if (sample == NULL) return; static const DWORD kSuspendFailed = static_cast(-1); if (SuspendThread(profiled_thread) == kSuspendFailed) return; @@ -714,6 +713,7 @@ class SamplerThread : public Thread { sampler->SampleStack(sample); sampler->Tick(sample); } + CpuProfiler::FinishTickSampleEvent(sampler->isolate()); ResumeThread(profiled_thread); } diff --git a/src/platform-freebsd.cc b/src/platform-freebsd.cc index 87c252c..6d1bccd 100644 --- a/src/platform-freebsd.cc +++ b/src/platform-freebsd.cc @@ -678,9 +678,8 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { Sampler* sampler = isolate->logger()->sampler(); if (sampler == NULL || !sampler->IsActive()) return; - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(isolate); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(isolate); + if (sample == NULL) return; // Extracting the sample from the context is extremely machine dependent. ucontext_t* ucontext = reinterpret_cast(context); @@ -701,6 +700,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { #endif sampler->SampleStack(sample); sampler->Tick(sample); + CpuProfiler::FinishTickSampleEvent(isolate); } diff --git a/src/platform-linux.cc b/src/platform-linux.cc index 308cdf8..fe366c9 100644 --- a/src/platform-linux.cc +++ b/src/platform-linux.cc @@ -1015,9 +1015,8 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { Sampler* sampler = isolate->logger()->sampler(); if (sampler == NULL || !sampler->IsActive()) return; - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(isolate); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(isolate); + if (sample == NULL) return; // Extracting the sample from the context is extremely machine dependent. ucontext_t* ucontext = reinterpret_cast(context); @@ -1052,6 +1051,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { #endif // V8_HOST_ARCH_* sampler->SampleStack(sample); sampler->Tick(sample); + CpuProfiler::FinishTickSampleEvent(isolate); } diff --git a/src/platform-macos.cc b/src/platform-macos.cc index 704fe12..a570868 100644 --- a/src/platform-macos.cc +++ b/src/platform-macos.cc @@ -819,9 +819,8 @@ class SamplerThread : public Thread { void SampleContext(Sampler* sampler) { thread_act_t profiled_thread = sampler->platform_data()->profiled_thread(); - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(sampler->isolate()); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(sampler->isolate()); + if (sample == NULL) return; if (KERN_SUCCESS != thread_suspend(profiled_thread)) return; @@ -858,6 +857,7 @@ class SamplerThread : public Thread { sampler->SampleStack(sample); sampler->Tick(sample); } + CpuProfiler::FinishTickSampleEvent(sampler->isolate()); thread_resume(profiled_thread); } diff --git a/src/platform-openbsd.cc b/src/platform-openbsd.cc index cf149cd..f96d9e3 100644 --- a/src/platform-openbsd.cc +++ b/src/platform-openbsd.cc @@ -731,9 +731,8 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { Sampler* sampler = isolate->logger()->sampler(); if (sampler == NULL || !sampler->IsActive()) return; - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(isolate); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(isolate); + if (sample == NULL) return; // Extracting the sample from the context is extremely machine dependent. sample->state = isolate->current_vm_state(); @@ -762,6 +761,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { #endif // __NetBSD__ sampler->SampleStack(sample); sampler->Tick(sample); + CpuProfiler::FinishTickSampleEvent(isolate); } diff --git a/src/platform-solaris.cc b/src/platform-solaris.cc index d6fa415..36d89e6 100644 --- a/src/platform-solaris.cc +++ b/src/platform-solaris.cc @@ -669,9 +669,8 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { Sampler* sampler = isolate->logger()->sampler(); if (sampler == NULL || !sampler->IsActive()) return; - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(isolate); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(isolate); + if (sample == NULL) return; // Extracting the sample from the context is extremely machine dependent. ucontext_t* ucontext = reinterpret_cast(context); @@ -684,6 +683,7 @@ static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { sampler->SampleStack(sample); sampler->Tick(sample); + CpuProfiler::FinishTickSampleEvent(isolate); } class Sampler::PlatformData : public Malloced { diff --git a/src/platform-win32.cc b/src/platform-win32.cc index 4ea4f92..65d31a9 100644 --- a/src/platform-win32.cc +++ b/src/platform-win32.cc @@ -2038,9 +2038,8 @@ class SamplerThread : public Thread { CONTEXT context; memset(&context, 0, sizeof(context)); - TickSample sample_obj; - TickSample* sample = CpuProfiler::TickSampleEvent(sampler->isolate()); - if (sample == NULL) sample = &sample_obj; + TickSample* sample = CpuProfiler::StartTickSampleEvent(sampler->isolate()); + if (sample == NULL) return; static const DWORD kSuspendFailed = static_cast(-1); if (SuspendThread(profiled_thread) == kSuspendFailed) return; @@ -2060,6 +2059,7 @@ class SamplerThread : public Thread { sampler->SampleStack(sample); sampler->Tick(sample); } + CpuProfiler::FinishTickSampleEvent(sampler->isolate()); ResumeThread(profiled_thread); } diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 20af070..589e6d8 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -5,6 +5,7 @@ #include "v8.h" #include "cpu-profiler-inl.h" #include "cctest.h" +#include "platform.h" #include "../include/v8-profiler.h" using i::CodeEntry; @@ -38,11 +39,13 @@ static inline i::Address ToAddress(int n) { return reinterpret_cast(n); } -static void EnqueueTickSampleEvent(ProfilerEventsProcessor* proc, - i::Address frame1, - i::Address frame2 = NULL, - i::Address frame3 = NULL) { - i::TickSample* sample = proc->TickSampleEvent(); +static void AddTickSampleEvent(ProfilerEventsProcessor* processor, + i::Address frame1, + i::Address frame2 = NULL, + i::Address frame3 = NULL) { + i::TickSample* sample; + i::OS::Sleep(20); + while ((sample = processor->StartTickSampleEvent()) == NULL) i::OS::Sleep(20); sample->pc = frame1; sample->tos = frame1; sample->frames_count = 0; @@ -54,6 +57,7 @@ static void EnqueueTickSampleEvent(ProfilerEventsProcessor* proc, sample->stack[1] = frame3; sample->frames_count = 2; } + processor->FinishTickSampleEvent(); } namespace { @@ -108,8 +112,8 @@ TEST(CodeEvents) { processor.CodeMoveEvent(ToAddress(0x1400), ToAddress(0x1500)); processor.CodeCreateEvent(i::Logger::STUB_TAG, 3, ToAddress(0x1600), 0x10); processor.CodeCreateEvent(i::Logger::STUB_TAG, 4, ToAddress(0x1605), 0x10); - // Enqueue a tick event to enable code events processing. - EnqueueTickSampleEvent(&processor, ToAddress(0x1000)); + // Add a tick event to enable code events processing. + AddTickSampleEvent(&processor, ToAddress(0x1000)); processor.Stop(); processor.Join(); @@ -154,12 +158,12 @@ TEST(TickEvents) { "ddd", ToAddress(0x1400), 0x80); - EnqueueTickSampleEvent(&processor, ToAddress(0x1210)); - EnqueueTickSampleEvent(&processor, ToAddress(0x1305), ToAddress(0x1220)); - EnqueueTickSampleEvent(&processor, - ToAddress(0x1404), - ToAddress(0x1305), - ToAddress(0x1230)); + AddTickSampleEvent(&processor, ToAddress(0x1210)); + AddTickSampleEvent(&processor, ToAddress(0x1305), ToAddress(0x1220)); + AddTickSampleEvent(&processor, + ToAddress(0x1404), + ToAddress(0x1305), + ToAddress(0x1230)); processor.Stop(); processor.Join(); @@ -240,13 +244,14 @@ TEST(Issue1398) { ToAddress(0x1200), 0x80); - i::TickSample* sample = processor.TickSampleEvent(); + i::TickSample* sample = processor.StartTickSampleEvent(); sample->pc = ToAddress(0x1200); sample->tos = 0; sample->frames_count = i::TickSample::kMaxFramesCount; for (int i = 0; i < sample->frames_count; ++i) { sample->stack[i] = ToAddress(0x1200); } + processor.FinishTickSampleEvent(); processor.Stop(); processor.Join(); -- 2.7.4