Add flag to control number of stack frames collected by profiler

runtime/vm/profiler.cc

Index: runtime/vm/profiler.cc
diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index b967123466e3ff9856cd092bebba57885d70df56..f78e18e50f0b251c6fa29d98830842aa76b2056d 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -19,15 +19,6 @@
 namespace dart {
 
 
-// Notes on stack frame walking:
-//
-// The sampling profiler will collect up to Sample::kNumStackFrames stack frames
-// The stack frame walking code uses the frame pointer to traverse the stack.
-// If the VM is compiled without frame pointers (which is the default on
-// recent GCC versions with optimizing enabled) the stack walking code may
-// fail (sometimes leading to a crash).
-//
-
 #if defined(USING_SIMULATOR) || defined(TARGET_OS_WINDOWS) || \
     defined(TARGET_OS_MACOS) || defined(TARGET_OS_ANDROID)
   DEFINE_FLAG(bool, profile, false, "Enable Sampling Profiler");
@@ -39,26 +30,30 @@ DEFINE_FLAG(charp, profile_dir, NULL,
             "Enable writing profile data into specified directory.");
 DEFINE_FLAG(int, profile_period, 1000,
             "Time between profiler samples in microseconds. Minimum 250.");
+DEFINE_FLAG(int, profile_frames, 8,
+            "Maximum number stack frames walked. Minimum 1.");

[siva, 2014/01/16 23:34:23]

maybe profile_frames_depth

[Cutch, 2014/01/17 16:02:25]

profile_depth.

 
 bool Profiler::initialized_ = false;
-Monitor* Profiler::monitor_ = NULL;
 SampleBuffer* Profiler::sample_buffer_ = NULL;
 
 void Profiler::InitOnce() {
   const int kMinimumProfilePeriod = 250;
+  const int kMinimumFrames = 1;

[siva, 2014/01/16 23:34:23]

We probably also want a max (not letting people arbitrarly collect huge stack
frames)

[Cutch, 2014/01/17 16:02:25]

Max of 128.

   if (!FLAG_profile) {
     return;
   }
   ASSERT(!initialized_);
-  initialized_ = true;
-  monitor_ = new Monitor();
-  sample_buffer_ = new SampleBuffer();
-  NativeSymbolResolver::InitOnce();
-  ThreadInterrupter::InitOnce();
   if (FLAG_profile_period < kMinimumProfilePeriod) {
     FLAG_profile_period = kMinimumProfilePeriod;
   }
+  if (FLAG_profile_frames < kMinimumFrames) {
+    FLAG_profile_frames = kMinimumFrames;
+  }
+  sample_buffer_ = new SampleBuffer();
+  NativeSymbolResolver::InitOnce();
+  ThreadInterrupter::InitOnce();
   ThreadInterrupter::SetInterruptPeriod(FLAG_profile_period);
+  initialized_ = true;
 }
 
 
@@ -78,7 +73,6 @@ void Profiler::InitProfilingForIsolate(Isolate* isolate, bool shared_buffer) {
   }
   ASSERT(isolate != NULL);
   ASSERT(sample_buffer_ != NULL);
-  MonitorLocker ml(monitor_);
   {
     MutexLocker profiler_data_lock(isolate->profiler_data_mutex());
     SampleBuffer* sample_buffer = sample_buffer_;
@@ -103,7 +97,6 @@ void Profiler::ShutdownProfilingForIsolate(Isolate* isolate) {
   }
   // We do not have a current isolate.
   ASSERT(Isolate::Current() == NULL);
-  MonitorLocker ml(monitor_);
   {
     MutexLocker profiler_data_lock(isolate->profiler_data_mutex());
     IsolateProfilerData* profiler_data = isolate->profiler_data();
@@ -540,16 +533,18 @@ intptr_t Profiler::ProcessSamples(Isolate* isolate,
                                   SampleBuffer* sample_buffer) {
   int64_t start = OS::GetCurrentTimeMillis();
   intptr_t samples = 0;
+  Sample* sample = Sample::Allocate();
   for (intptr_t i = 0; i < sample_buffer->capacity(); i++) {
-    Sample sample = sample_buffer->GetSample(i);
-    if (sample.isolate != isolate) {
+    sample_buffer->CopySample(i, sample);
+    if (sample->isolate != isolate) {
       continue;
     }
-    if (sample.timestamp == 0) {
+    if (sample->timestamp == 0) {
       continue;
     }
-    samples += ProcessSample(isolate, code_region_table, &sample);
+    samples += ProcessSample(isolate, code_region_table, sample);
   }
+  free(sample);
   int64_t end = OS::GetCurrentTimeMillis();
   if (FLAG_trace_profiled_isolates) {
     int64_t delta = end - start;
@@ -577,7 +572,7 @@ intptr_t Profiler::ProcessSample(Isolate* isolate,
   // i points to the leaf (exclusive) PC sample. Do not tick the address.
   code_region_table->AddTick(sample->pcs[i], true, false);
   // Give all frames an inclusive tick and tick the address.
-  for (; i < Sample::kNumStackFrames; i++) {
+  for (; i < FLAG_profile_frames; i++) {
     if (sample->pcs[i] == 0) {
       break;
     }
@@ -645,22 +640,49 @@ IsolateProfilerData::~IsolateProfilerData() {
 }
 
 
+void Sample::CopyInto(Sample* dst) const {
+  ASSERT(dst != NULL);
+  dst->timestamp = timestamp;
+  dst->tid = tid;
+  dst->isolate = isolate;
+  dst->type = type;
+  dst->vm_tags = vm_tags;
+  dst->runtime_tags = runtime_tags;
+  for (intptr_t i = 0; i < FLAG_profile_frames; i++) {
+    dst->pcs[i] = pcs[i];
+  }
+}
+
+
+Sample* Sample::Allocate() {
+  return reinterpret_cast<Sample*>(malloc(InstanceSize()));
+}
+
+
+intptr_t Sample::InstanceSize() {
+  ASSERT(FLAG_profile_frames >= 1);
+  return sizeof(Sample) + (sizeof(intptr_t) * FLAG_profile_frames);  // NOLINT.

[siva, 2014/01/16 23:34:23]

Why did you choose this to be a function instead of just a static variable
instance_size_; that could be initialized in Profiler::InitOnce.

Not sure if all compilers will nicely constant fold the expression.

[Cutch, 2014/01/17 16:02:25]

Done.

+}
+
+
 void Sample::Init(SampleType type, Isolate* isolate, int64_t timestamp,
                   ThreadId tid) {
   this->timestamp = timestamp;
   this->tid = tid;
   this->isolate = isolate;
-  for (intptr_t i = 0; i < kNumStackFrames; i++) {
-    pcs[i] = 0;
-  }
   this->type = type;
   vm_tags = 0;
   runtime_tags = 0;
+  for (int i = 0; i < FLAG_profile_frames; i++) {
+    pcs[i] = 0;
+  }
 }
 
+
 SampleBuffer::SampleBuffer(intptr_t capacity) {
   capacity_ = capacity;
-  samples_ = reinterpret_cast<Sample*>(calloc(capacity, sizeof(Sample)));
+  samples_ = reinterpret_cast<Sample*>(
+      calloc(capacity, Sample::InstanceSize()));
   cursor_ = 0;
 }
 
@@ -680,7 +702,21 @@ Sample* SampleBuffer::ReserveSample() {
   uintptr_t cursor = AtomicOperations::FetchAndIncrement(&cursor_);
   // Map back into sample buffer range.
   cursor = cursor % capacity_;
-  return &samples_[cursor];
+  return At(cursor);
+}
+
+
+void SampleBuffer::CopySample(intptr_t i, Sample* sample) const {
+  At(i)->CopyInto(sample);
+}
+
+
+Sample* SampleBuffer::At(intptr_t idx) const {
+  ASSERT(idx >= 0);
+  ASSERT(idx < capacity_);
+  intptr_t offset = idx * Sample::InstanceSize();
+  uint8_t* samples = reinterpret_cast<uint8_t*>(samples_);
+  return reinterpret_cast<Sample*>(samples + offset);
 }
 
 
@@ -698,13 +734,18 @@ ProfilerSampleStackWalker::ProfilerSampleStackWalker(Sample* sample,
     original_sp_(sp),
     lower_bound_(stack_lower) {
   ASSERT(sample_ != NULL);
-  // Zero out the PCs before (re)using the sample.
-  for (int i = 0; i < Sample::kNumStackFrames; i++) {
-    sample_->pcs[i] = 0;
-  }
 }
 
 
+// Notes on stack frame walking:
+//
+// The sampling profiler will collect up to Sample::kNumStackFrames stack frames
+// The stack frame walking code uses the frame pointer to traverse the stack.
+// If the VM is compiled without frame pointers (which is the default on
+// recent GCC versions with optimizing enabled) the stack walking code may
+// fail (sometimes leading to a crash).
+//
+
 int ProfilerSampleStackWalker::walk() {
   const intptr_t kMaxStep = 0x1000;  // 4K.
   uword* pc = reinterpret_cast<uword*>(original_pc_);
@@ -730,7 +771,7 @@ int ProfilerSampleStackWalker::walk() {
     lower_bound_ = original_sp_;
   }
   int i = 0;
-  for (; i < Sample::kNumStackFrames; i++) {
+  for (; i < FLAG_profile_frames; i++) {
     sample_->pcs[i] = reinterpret_cast<uintptr_t>(pc);
     if (!ValidFramePointer(fp)) {
       return i + 1;