Improve CodeRegionTable build time by 30x

runtime/vm/profiler.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "platform/utils.h"
 
 #include "vm/allocation.h"
 #include "vm/atomic.h"
 #include "vm/code_patcher.h"
 #include "vm/isolate.h"
@@ skipping 130 matching lines @@
     return;
   }
   if (!FLAG_profile) {
     return;
   }
   ASSERT(initialized_);
   ThreadInterrupter::Unregister();
 }
 
 
-void Profiler::RecordTickInterruptCallback(const InterruptedThreadState& state,
-                                           void* data) {
-  Isolate* isolate = reinterpret_cast<Isolate*>(data);
-  if (isolate == NULL) {
-    return;
-  }
-  IsolateProfilerData* profiler_data = isolate->profiler_data();
-  if (profiler_data == NULL) {
-    return;
-  }
-  SampleBuffer* sample_buffer = profiler_data->sample_buffer();
-  if (sample_buffer == NULL) {
-    return;
-  }
-  Sample* sample = sample_buffer->ReserveSample();
-  sample->Init(Sample::kIsolateSample, isolate, OS::GetCurrentTimeMicros(),
-               state.tid);
-}
-
-
-void Profiler::RecordSampleInterruptCallback(
-    const InterruptedThreadState& state,
-    void* data) {
-  Isolate* isolate = reinterpret_cast<Isolate*>(data);
-  if (isolate == NULL) {
-    return;
-  }
-  IsolateProfilerData* profiler_data = isolate->profiler_data();
-  if (profiler_data == NULL) {
-    return;
-  }
-  SampleBuffer* sample_buffer = profiler_data->sample_buffer();
-  if (sample_buffer == NULL) {
-    return;
-  }
-  Sample* sample = sample_buffer->ReserveSample();
-  sample->Init(Sample::kIsolateSample, isolate, OS::GetCurrentTimeMicros(),
-               state.tid);
-  uintptr_t stack_lower = 0;
-  uintptr_t stack_upper = 0;
-  isolate->GetStackBounds(&stack_lower, &stack_upper);
-  if ((stack_lower == 0) || (stack_upper == 0)) {
-    stack_lower = 0;
-    stack_upper = 0;
-  }
-  ProfilerSampleStackWalker stackWalker(sample, stack_lower, stack_upper,
-                                        state.pc, state.fp, state.sp);
-  stackWalker.walk();
-}
-
-
 struct AddressEntry {
   uintptr_t pc;
   uintptr_t ticks;
 };
 
 
 // A region of code. Each region is a kind of code (Dart, Collected, or Native).
 class CodeRegion : public ZoneAllocated {
  public:
   enum Kind {
     kDartCode,
     kCollectedCode,
     kNativeCode
   };
 
   CodeRegion(Kind kind, uintptr_t start, uintptr_t end) :
       kind_(kind),
       start_(start),
       end_(end),
       inclusive_ticks_(0),
       exclusive_ticks_(0),
       name_(NULL),
       address_table_(new ZoneGrowableArray<AddressEntry>()) {
+    ASSERT(start_ < end_);
   }
 
   ~CodeRegion() {
   }
 
   uintptr_t start() const { return start_; }
   void set_start(uintptr_t start) {
     start_ = start;
   }
 
   uintptr_t end() const { return end_; }
   void set_end(uintptr_t end) {
     end_ = end;
   }
 
   void AdjustExtent(uintptr_t start, uintptr_t end) {
     if (start < start_) {
       start_ = start;
     }
     if (end > end_) {
       end_ = end;
     }
+    ASSERT(start_ < end_);
   }
 
   bool contains(uintptr_t pc) const {
     return (pc >= start_) && (pc < end_);
   }
 
+  bool overlaps(const CodeRegion* other) const {
+    ASSERT(other != NULL);
+    bool a = other->contains(start_);
+    bool b = other->contains(end_ - 1);
+    bool c = contains(other->start());
+    bool d = contains(other->end() - 1);
+    return a || b || c || d;

[siva, 2014/02/12 00:58:52]

There is no short circuiting of calls to contains this way, why not just do:

return other->contains(start_) ||
       other->contains(end_ - 1) ||
       contains(other->start()) ||
       contains(other->end() - 1);

[Cutch, 2014/02/13 23:13:19]

Done.

+  }
+
   intptr_t inclusive_ticks() const { return inclusive_ticks_; }
   void set_inclusive_ticks(intptr_t inclusive_ticks) {
     inclusive_ticks_ = inclusive_ticks;
   }
 
   intptr_t exclusive_ticks() const { return exclusive_ticks_; }
   void set_exclusive_ticks(intptr_t exclusive_ticks) {
     exclusive_ticks_ = exclusive_ticks;
   }
 
@@ skipping 24 matching lines @@
   }
 
   void AddTick(bool exclusive) {
     if (exclusive) {
       exclusive_ticks_++;
     } else {
       inclusive_ticks_++;
     }
   }
 
-  void DebugPrint() {
-    printf("%s [%" Px ", %" Px ") %s\n", name_, start(), end(),
-           KindToCString(kind_));
+  void DebugPrint() const {
+    printf("%s [%" Px ", %" Px ") %s\n", KindToCString(kind_), start(), end(),
+           name_);
   }
 
   void AddTickAtAddress(uintptr_t pc) {
     const intptr_t length = address_table_->length();
     intptr_t i = 0;
     for (; i < length; i++) {
       AddressEntry& entry = (*address_table_)[i];
       if (entry.pc == pc) {
         entry.ticks++;
         return;
@@ skipping 77 matching lines @@
   uintptr_t end_;
   intptr_t inclusive_ticks_;
   intptr_t exclusive_ticks_;
   const char* name_;
   ZoneGrowableArray<AddressEntry>* address_table_;
 
   DISALLOW_COPY_AND_ASSIGN(CodeRegion);
 };
 
 
+class ScopeStopwatch {
+ public:
+  explicit ScopeStopwatch(const char* name) : name_(name) {
+    start_ = OS::GetCurrentTimeMillis();
+  }
+
+  intptr_t GetElapsed() {

[siva, 2014/02/12 00:58:52]

const {

[Cutch, 2014/02/13 23:13:19]

Done.

+    intptr_t end = OS::GetCurrentTimeMillis();
+    ASSERT(end >= start_);
+    return end - start_;
+  }
+
+  ~ScopeStopwatch() {
+    if (FLAG_trace_profiled_isolates) {
+      intptr_t elapsed = GetElapsed();
+      OS::Print("%s took %" Pd " millis.\n", name_, elapsed);
+    }
+  }
+
+ private:
+  const char* name_;
+  intptr_t start_;

[siva, 2014/02/12 00:58:52]

DISALLOW stuff..... or make this a ValueObject. Probably it should be a
ValueObject.

[Cutch, 2014/02/13 23:13:19]

Done.

+};
+
+
 // All code regions. Code region tables are built on demand when a profile
 // is requested (through the service or on isolate shutdown).
 class ProfilerCodeRegionTable : public ValueObject {
  public:
   explicit ProfilerCodeRegionTable(Isolate* isolate) :
       heap_(isolate->heap()),
       code_region_table_(new ZoneGrowableArray<CodeRegion*>(64)) {
   }
 
   ~ProfilerCodeRegionTable() {
@@ skipping 13 matching lines @@
       (*code_region_table_)[index]->AddTickAtAddress(pc);
     }
   }
 
   intptr_t Length() const { return code_region_table_->length(); }
 
   CodeRegion* At(intptr_t idx) {
     return (*code_region_table_)[idx];
   }
 
+#if defined(DEBUG)
+  void Verify() {
+    VerifyOrder();
+    VerifyOverlap();
+  }
+#endif
+
  private:
+  intptr_t FindUpperBound(uintptr_t pc) {

[siva, 2014/02/12 00:58:52]

we usually use uword for address types.

[Cutch, 2014/02/13 23:13:19]

Done here and elsewhere.

+    intptr_t count = code_region_table_->length();
+    intptr_t first = 0;
+    while (count > 0) {
+      intptr_t it = first;
+      intptr_t step = count / 2;
+      it += step;
+      const CodeRegion* code_region = (*code_region_table_)[it];
+      if (pc >= code_region->end()) {
+        first = ++it;
+        count -= (step + 1);
+      } else {
+        count = step;
+      }
+    }
+    return first;
+  }
+
+
+  intptr_t FindLowerBound(uintptr_t pc) {
+    intptr_t count = code_region_table_->length();
+    intptr_t first = 0;
+    while (count > 0) {
+      intptr_t it = first;
+      intptr_t step = count / 2;
+      it += step;
+      const CodeRegion* code_region = (*code_region_table_)[it];
+      if (code_region->start() < pc) {
+        first = ++it;
+        count -= (step + 1);
+      } else {
+        count = step;
+      }
+    }
+    return first;
+  }

[siva, 2014/02/12 00:58:52]

Would it make sense to combine FindUpperBound and FindLowerBound into a single:
intptr_t FindRegion(uword pc,
                    bool (*comparator)(uword pc, uword region_pc));
and pass in approriate comparators for the upper and lower cases.

[Cutch, 2014/02/13 23:13:19]

Done.

+
+
   intptr_t FindIndex(uintptr_t pc) {
-    const intptr_t length = code_region_table_->length();
-    for (intptr_t i = 0; i < length; i++) {
-      const CodeRegion* code_region = (*code_region_table_)[i];
+    intptr_t index = FindLowerBound(pc);
+    const CodeRegion* code_region = NULL;
+    if (index > 0) {
+      // We may have overshot by 1. Check previous entry.

[siva, 2014/02/12 00:58:52]

Not sure how this happens?

[Cutch, 2014/02/13 23:13:19]

It doesn't anymore.

+      code_region = (*code_region_table_)[index - 1];
       if (code_region->contains(pc)) {
-        return i;
+        // Found at index - 1.
+        return index - 1;
       }
     }
+    if (index == code_region_table_->length()) {
+      // Not present.
+      return -1;
+    }
+    code_region = (*code_region_table_)[index];
+    if (code_region->contains(pc)) {
+      // Found at index.
+      return index;
+    }
     return -1;
   }
 
   CodeRegion* CreateCodeRegion(uintptr_t pc) {
     Code& code = Code::Handle(Code::LookupCode(pc));
     if (!code.IsNull()) {
       return new CodeRegion(CodeRegion::kDartCode, code.EntryPoint(),
                             code.EntryPoint() + code.Size());
     }
     if (heap_->CodeContains(pc)) {
       const intptr_t kDartCodeAlignment = 0x10;
       const intptr_t kDartCodeAlignmentMask = ~(kDartCodeAlignment - 1);
-      return new CodeRegion(CodeRegion::kCollectedCode,
-                            (pc & kDartCodeAlignmentMask),
+      return new CodeRegion(CodeRegion::kCollectedCode, pc,
                             (pc & kDartCodeAlignmentMask) + kDartCodeAlignment);
     }
     uintptr_t native_start = 0;
     char* native_name = NativeSymbolResolver::LookupSymbolName(pc,
                                                                &native_start);
     if (native_name == NULL) {
       return new CodeRegion(CodeRegion::kNativeCode, pc, pc + 1);
     }
     ASSERT(pc >= native_start);
     CodeRegion* code_region =
         new CodeRegion(CodeRegion::kNativeCode, native_start, pc + 1);
     code_region->SetName(native_name);
     free(native_name);
     return code_region;
   }
 
+  void HandleOverlap(CodeRegion* region, CodeRegion* code_region,
+                     uintptr_t start, uintptr_t end) {
+    // We should never see overlapping Dart code regions.
+    ASSERT(region->kind() != CodeRegion::kDartCode);
+    // When code regions overlap, they should be of the same kind.
+    ASSERT(region->kind() == code_region->kind());
+    region->AdjustExtent(start, end);
+  }
+
   intptr_t InsertCodeRegion(CodeRegion* code_region) {
-    const intptr_t length = code_region_table_->length();
     const uintptr_t start = code_region->start();
     const uintptr_t end = code_region->end();
-    intptr_t i = 0;
-    for (; i < length; i++) {
-      CodeRegion* region = (*code_region_table_)[i];
-      if (region->contains(start) || region->contains(end - 1)) {
-        // We should only see overlapping native code regions.
-        ASSERT(region->kind() == CodeRegion::kNativeCode);
-        // When code regions overlap, they should be of the same kind.
-        ASSERT(region->kind() == code_region->kind());
-        // Overlapping code region.
-        region->AdjustExtent(start, end);
-        return i;
-      } else if (start >= region->end()) {
-        // Insert here.
-        break;
+    const intptr_t length = code_region_table_->length();
+    if (length == 0) {
+      code_region_table_->Add(code_region);
+      return length;
+    }
+    // Determine the correct place to insert or merge code_region into table.
+    intptr_t lo = FindLowerBound(start);
+    intptr_t hi = FindUpperBound(end - 1);
+    if ((lo == length) && (hi == length)) {
+      lo = length - 1;
+    }
+    if (lo == length) {
+      CodeRegion* region = (*code_region_table_)[hi];
+      if (region->overlaps(code_region)) {
+        HandleOverlap(region, code_region, start, end);
+        return hi;
+      }
+      code_region_table_->Add(code_region);
+      return length;
+    } else if (hi == length) {
+      CodeRegion* region = (*code_region_table_)[lo];
+      if (region->overlaps(code_region)) {
+        HandleOverlap(region, code_region, start, end);
+        return lo;
+      }
+      code_region_table_->Add(code_region);
+      return length;
+    } else if (lo == hi) {
+      CodeRegion* region = (*code_region_table_)[lo];
+      if (region->overlaps(code_region)) {
+        HandleOverlap(region, code_region, start, end);
+        return lo;
+      }
+      code_region_table_->InsertAt(lo, code_region);
+      return lo;
+    } else {
+      CodeRegion* region = (*code_region_table_)[lo];
+      if (region->overlaps(code_region)) {
+        HandleOverlap(region, code_region, start, end);
+        return lo;
+      }
+      region = (*code_region_table_)[hi];
+      if (region->overlaps(code_region)) {
+        HandleOverlap(region, code_region, start, end);
+        return hi;
+      }
+      code_region_table_->InsertAt(hi, code_region);
+      return hi;
+    }
+    UNREACHABLE();
+  }
+
+#if defined(DEBUG)
+  void VerifyOrder() {
+    const intptr_t length = code_region_table_->length();
+    if (length == 0) {
+      return;
+    }
+    uintptr_t last = (*code_region_table_)[0]->end();
+    for (intptr_t i = 1; i < length; i++) {
+      CodeRegion* a = (*code_region_table_)[i];
+      ASSERT(last <= a->start());
+      last = a->end();
+    }
+  }
+
+  void VerifyOverlap() {
+    const intptr_t length = code_region_table_->length();
+    for (intptr_t i = 0; i < length; i++) {
+      CodeRegion* a = (*code_region_table_)[i];
+      for (intptr_t j = i+1; j < length; j++) {
+        CodeRegion* b = (*code_region_table_)[j];
+        ASSERT(!a->contains(b->start()) &&
+               !a->contains(b->end() - 1) &&
+               !b->contains(a->start()) &&
+               !b->contains(a->end() - 1));
       }
     }
-    if (i != length) {
-      code_region_table_->InsertAt(i, code_region);
-      return i;
-    }
-    code_region_table_->Add(code_region);
-    return code_region_table_->length() - 1;
   }
+#endif
 
   Heap* heap_;
   ZoneGrowableArray<CodeRegion*>* code_region_table_;
 };
 
 
+class CodeRegionTableBuilder : public SampleVisitor {
+ public:
+  CodeRegionTableBuilder(Isolate* isolate,
+                         ProfilerCodeRegionTable* code_region_table)
+      : SampleVisitor(isolate), code_region_table_(code_region_table) {
+    frames_ = 0;
+  }
+
+  void VisitSample(Sample* sample) {
+    code_region_table_->AddTick(sample->At(0), true, false);
+    // Give all frames an inclusive tick and tick the address.
+    for (intptr_t i = 0; i < FLAG_profile_depth; i++) {
+      if (sample->At(i) == 0) {
+        break;
+      }
+      frames_++;
+      code_region_table_->AddTick(sample->At(i), false, true);
+    }
+  }
+
+  intptr_t frames() const { return frames_; }
+ private:
+  intptr_t frames_;
+  ProfilerCodeRegionTable* code_region_table_;
+};
+
+
 void Profiler::PrintToJSONStream(Isolate* isolate, JSONStream* stream,
                                  bool full) {
   ASSERT(isolate == Isolate::Current());
   // Disable profile interrupts while processing the buffer.
   EndExecution(isolate);
   MutexLocker profiler_data_lock(isolate->profiler_data_mutex());
   IsolateProfilerData* profiler_data = isolate->profiler_data();
   if (profiler_data == NULL) {
     JSONObject error(stream);
     error.AddProperty("type", "Error");
     error.AddProperty("text", "Isolate does not have profiling enabled.");
     return;
   }
   SampleBuffer* sample_buffer = profiler_data->sample_buffer();
   ASSERT(sample_buffer != NULL);
   {
     StackZone zone(isolate);
     {
       // Build code region table.
       ProfilerCodeRegionTable code_region_table(isolate);
-      intptr_t samples =
-          ProcessSamples(isolate, &code_region_table, sample_buffer);
+      CodeRegionTableBuilder builder(isolate, &code_region_table);
       {
+        ScopeStopwatch sw("CodeTableBuild");
+        sample_buffer->VisitSamples(&builder);
+      }
+#if defined(DEBUG)
+      code_region_table.Verify();
+#endif
+      // Number of samples we processed.
+      intptr_t samples = builder.visited();
+      intptr_t frames = builder.frames();
+      if (FLAG_trace_profiled_isolates) {
+        OS::Print("%" Pd " frames produced %" Pd " code objects.\n",
+                  frames, code_region_table.Length());
+      }
+      {
+        ScopeStopwatch sw("CodeTableStream");
         // Serialize to JSON.
         JSONObject obj(stream);
         obj.AddProperty("type", "Profile");
         obj.AddProperty("samples", samples);
         JSONArray codes(&obj, "codes");
         for (intptr_t i = 0; i < code_region_table.Length(); i++) {
           CodeRegion* region = code_region_table.At(i);
           ASSERT(region != NULL);
-          region->PrintToJSONArray(&codes, full);
+          region->PrintToJSONArray(&codes, false);
         }
       }
     }
   }
   // Enable profile interrupts.
   BeginExecution(isolate);
 }
 
 
-intptr_t Profiler::ProcessSamples(Isolate* isolate,
-                                  ProfilerCodeRegionTable* code_region_table,
-                                  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_buffer->CopySample(i, sample);
-    if (sample->isolate() != isolate) {
-      continue;
-    }
-    if (sample->timestamp() == 0) {
-      continue;
-    }
-    samples += ProcessSample(isolate, code_region_table, sample);
-  }
-  free(sample);
-  int64_t end = OS::GetCurrentTimeMillis();
-  if (FLAG_trace_profiled_isolates) {
-    int64_t delta = end - start;
-    OS::Print("Processed %" Pd " samples from %s in %" Pd64 " milliseconds.\n",
-        samples,
-        isolate->name(),
-        delta);
-  }
-  return samples;
-}
-
-
-intptr_t Profiler::ProcessSample(Isolate* isolate,
-                                 ProfilerCodeRegionTable* code_region_table,
-                                Sample* sample) {
-  if (sample->type() != Sample::kIsolateSample) {
-    return 0;
-  }
-  if (sample->At(0) == 0) {
-    // No frames in this sample.
-    return 0;
-  }
-  // i points to the leaf (exclusive) PC sample. Do not tick the address.
-  code_region_table->AddTick(sample->At(0), true, false);
-  // Give all frames an inclusive tick and tick the address.
-  for (intptr_t i = 0; i < FLAG_profile_depth; i++) {
-    if (sample->At(i) == 0) {
-      break;
-    }
-    code_region_table->AddTick(sample->At(i), false, true);
-  }
-  return 1;
-}
-
-
 void Profiler::WriteProfile(Isolate* isolate) {
   if (isolate == NULL) {
     return;
   }
   if (!FLAG_profile) {
     return;
   }
   ASSERT(initialized_);
   if (FLAG_profile_dir == NULL) {
     return;
@@ skipping 90 matching lines @@
     dst->pcs_[i] = pcs_[i];
   }
 }
 
 
 Sample* Sample::Allocate() {
   return reinterpret_cast<Sample*>(malloc(instance_size()));
 }
 
 
+SampleVisitor::SampleVisitor(Isolate* isolate)
+    : isolate_(isolate), visited_(0) {
+}

[siva, 2014/02/12 00:58:52]

Why not move this to the header file itself, seems very small.

[Cutch, 2014/02/13 23:13:19]

Done.

+
+
 SampleBuffer::SampleBuffer(intptr_t capacity) {
   capacity_ = capacity;
   samples_ = reinterpret_cast<Sample*>(
       calloc(capacity, Sample::instance_size()));
   cursor_ = 0;
 }
 
 
 SampleBuffer::~SampleBuffer() {
   if (samples_ != NULL) {
@@ skipping 21 matching lines @@
 
 Sample* SampleBuffer::At(intptr_t idx) const {
   ASSERT(idx >= 0);
   ASSERT(idx < capacity_);
   intptr_t offset = idx * Sample::instance_size();
   uint8_t* samples = reinterpret_cast<uint8_t*>(samples_);
   return reinterpret_cast<Sample*>(samples + offset);
 }
 
 
-ProfilerSampleStackWalker::ProfilerSampleStackWalker(Sample* sample,
-                                                     uintptr_t stack_lower,
-                                                     uintptr_t stack_upper,
-                                                     uintptr_t pc,
-                                                     uintptr_t fp,
-                                                     uintptr_t sp) :
-    sample_(sample),
-    stack_lower_(stack_lower),
-    stack_upper_(stack_upper),
-    original_pc_(pc),
-    original_fp_(fp),
-    original_sp_(sp),
-    lower_bound_(stack_lower) {
-  ASSERT(sample_ != NULL);
+void SampleBuffer::VisitSamples(SampleVisitor* visitor) {
+  ASSERT(visitor != NULL);
+  Sample* sample = Sample::Allocate();
+  const intptr_t length = capacity();
+  for (intptr_t i = 0; i < length; i++) {
+    CopySample(i, sample);
+    if (sample->isolate() != visitor->isolate()) {
+      // Another isolate.
+      continue;
+    }
+    if (sample->timestamp() == 0) {
+      // Empty.
+      continue;
+    }
+    if (sample->At(0) == 0) {
+      // No frames.
+      continue;
+    }
+    visitor->IncrementVisited();
+    visitor->VisitSample(sample);
+  }
+  free(sample);
 }
 
 
 // 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).
 //
+class ProfilerSampleStackWalker : public ValueObject {
+ public:
+  ProfilerSampleStackWalker(Sample* sample,
+                            uintptr_t stack_lower,
+                            uintptr_t stack_upper,
+                            uintptr_t pc,
+                            uintptr_t fp,
+                            uintptr_t sp)
+      : sample_(sample),
+        stack_lower_(stack_lower),
+        stack_upper_(stack_upper),
+        original_pc_(pc),
+        original_fp_(fp),
+        original_sp_(sp),
+        lower_bound_(stack_lower) {
+    ASSERT(sample_ != NULL);
+  }
 
-int ProfilerSampleStackWalker::walk() {
-  const intptr_t kMaxStep = 0x1000;  // 4K.
-  const bool kWalkStack = true;  // Walk the stack.
-  // Always store the exclusive PC.
-  sample_->SetAt(0, original_pc_);
-  if (!kWalkStack) {
-    // Not walking the stack, only took exclusive sample.
-    return 1;
+  int walk() {
+    const intptr_t kMaxStep = 0x1000;  // 4K.
+    const bool kWalkStack = true;  // Walk the stack.
+    // Always store the exclusive PC.
+    sample_->SetAt(0, original_pc_);
+    if (!kWalkStack) {
+      // Not walking the stack, only took exclusive sample.
+      return 1;
+    }
+    uword* pc = reinterpret_cast<uword*>(original_pc_);
+    uword* fp = reinterpret_cast<uword*>(original_fp_);
+    uword* previous_fp = fp;
+    if (original_sp_ > original_fp_) {
+      // Stack pointer should not be above frame pointer.
+      return 1;
+    }
+    intptr_t gap = original_fp_ - original_sp_;
+    if (gap >= kMaxStep) {
+      // Gap between frame pointer and stack pointer is
+      // too large.
+      return 1;
+    }
+    if (original_sp_ < lower_bound_) {
+      // The stack pointer gives us a better lower bound than
+      // the isolates stack limit.
+      lower_bound_ = original_sp_;
+    }
+    int i = 0;
+    for (; i < FLAG_profile_depth; i++) {
+      sample_->SetAt(i, reinterpret_cast<uintptr_t>(pc));
+      if (!ValidFramePointer(fp)) {
+        return i + 1;
+      }
+      pc = CallerPC(fp);
+      previous_fp = fp;
+      fp = CallerFP(fp);
+      intptr_t step = fp - previous_fp;
+      if ((step >= kMaxStep) || (fp <= previous_fp) || !ValidFramePointer(fp)) {
+        // Frame pointer step is too large.
+        // Frame pointer did not move to a higher address.
+        // Frame pointer is outside of isolate stack bounds.
+        return i + 1;
+      }
+      // Move the lower bound up.
+      lower_bound_ = reinterpret_cast<uintptr_t>(fp);
+    }
+    return i;
   }
-  uword* pc = reinterpret_cast<uword*>(original_pc_);
-  uword* fp = reinterpret_cast<uword*>(original_fp_);
-  uword* previous_fp = fp;
-  if (original_sp_ > original_fp_) {
-    // Stack pointer should not be above frame pointer.
-    return 1;
+
+ private:
+  uword* CallerPC(uword* fp) {

[siva, 2014/02/12 00:58:52]

const {

[Cutch, 2014/02/13 23:13:19]

Done.

+    ASSERT(fp != NULL);
+    return reinterpret_cast<uword*>(*(fp + kSavedCallerPcSlotFromFp));
   }
-  intptr_t gap = original_fp_ - original_sp_;
-  if (gap >= kMaxStep) {
-    // Gap between frame pointer and stack pointer is
-    // too large.
-    return 1;
+
+  uword* CallerFP(uword* fp) {

[siva, 2014/02/12 00:58:52]

const {

[Cutch, 2014/02/13 23:13:19]

Done.

+    ASSERT(fp != NULL);
+    return reinterpret_cast<uword*>(*(fp + kSavedCallerFpSlotFromFp));
   }
-  if (original_sp_ < lower_bound_) {
-    // The stack pointer gives us a better lower bound than
-    // the isolates stack limit.
-    lower_bound_ = original_sp_;
+
+  bool ValidFramePointer(uword* fp) {

[siva, 2014/02/12 00:58:52]

const {

[Cutch, 2014/02/13 23:13:19]

Done.

+    if (fp == NULL) {
+      return false;
+    }
+    uintptr_t cursor = reinterpret_cast<uintptr_t>(fp);
+    cursor += sizeof(fp);
+    bool r = cursor >= lower_bound_ && cursor < stack_upper_;
+    return r;
   }
-  int i = 0;
-  for (; i < FLAG_profile_depth; i++) {
-    sample_->SetAt(i, reinterpret_cast<uintptr_t>(pc));
-    if (!ValidFramePointer(fp)) {
-      return i + 1;
-    }
-    pc = CallerPC(fp);
-    previous_fp = fp;
-    fp = CallerFP(fp);
-    intptr_t step = fp - previous_fp;
-    if ((step >= kMaxStep) || (fp <= previous_fp) || !ValidFramePointer(fp)) {
-      // Frame pointer step is too large.
-      // Frame pointer did not move to a higher address.
-      // Frame pointer is outside of isolate stack bounds.
-      return i + 1;
-    }
-    // Move the lower bound up.
-    lower_bound_ = reinterpret_cast<uintptr_t>(fp);
+
+  Sample* sample_;
+  const uintptr_t stack_lower_;
+  const uintptr_t stack_upper_;
+  const uintptr_t original_pc_;
+  const uintptr_t original_fp_;
+  const uintptr_t original_sp_;
+  uintptr_t lower_bound_;
+};
+
+void Profiler::RecordSampleInterruptCallback(
+    const InterruptedThreadState& state,
+    void* data) {
+  Isolate* isolate = reinterpret_cast<Isolate*>(data);
+  if (isolate == NULL) {
+    return;
   }
-  return i;
+  IsolateProfilerData* profiler_data = isolate->profiler_data();
+  if (profiler_data == NULL) {
+    return;
+  }
+  SampleBuffer* sample_buffer = profiler_data->sample_buffer();
+  if (sample_buffer == NULL) {
+    return;
+  }
+  Sample* sample = sample_buffer->ReserveSample();
+  sample->Init(Sample::kIsolateSample, isolate, OS::GetCurrentTimeMicros(),
+               state.tid);
+  uintptr_t stack_lower = 0;
+  uintptr_t stack_upper = 0;
+  isolate->GetStackBounds(&stack_lower, &stack_upper);
+  if ((stack_lower == 0) || (stack_upper == 0)) {
+    stack_lower = 0;
+    stack_upper = 0;
+  }
+  ProfilerSampleStackWalker stackWalker(sample, stack_lower, stack_upper,
+                                        state.pc, state.fp, state.sp);
+  stackWalker.walk();
 }
 
 
-uword* ProfilerSampleStackWalker::CallerPC(uword* fp) {
-  ASSERT(fp != NULL);
-  return reinterpret_cast<uword*>(*(fp + kSavedCallerPcSlotFromFp));
-}
-
-
-uword* ProfilerSampleStackWalker::CallerFP(uword* fp) {
-  ASSERT(fp != NULL);
-  return reinterpret_cast<uword*>(*(fp + kSavedCallerFpSlotFromFp));
-}
-
-
-bool ProfilerSampleStackWalker::ValidFramePointer(uword* fp) {
-  if (fp == NULL) {
-    return false;
-  }
-  uintptr_t cursor = reinterpret_cast<uintptr_t>(fp);
-  cursor += sizeof(fp);
-  bool r = cursor >= lower_bound_ && cursor < stack_upper_;
-  return r;
-}
-
-
 }  // namespace dart