From cd71a2792c7d13d68ae8e48ca5ef46147b591fbe Mon Sep 17 00:00:00 2001
From: "mikhail.naganov@gmail.com"
 <mikhail.naganov@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Fri, 6 Mar 2009 13:07:57 +0000
Subject: [PATCH] Get rid or heap allocation in stack sampler to avoid
 deadlocks.

Review URL: http://codereview.chromium.org/40219

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1440 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/log.cc                   | 32 +++++++++++---------------------
 src/log.h                    |  2 --
 src/platform.h               | 23 +++--------------------
 test/cctest/test-log-ia32.cc |  4 ++--
 4 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/src/log.cc b/src/log.cc
index 34631cd..805bb51 100644
--- a/src/log.cc
+++ b/src/log.cc
@@ -138,39 +138,31 @@ bool Profiler::paused_ = false;
 //
 void StackTracer::Trace(TickSample* sample) {
   if (sample->state == GC) {
-    sample->InitStack(0);
+    sample->frames_count = 0;
     return;
   }
 
   // If c_entry_fp is available, this means that we are inside a C++
-  // function and sample->fp value isn't reliable due to FPO
+  // function and sample->fp value isn't reliable due to FPO.
   if (Top::c_entry_fp(Top::GetCurrentThread()) != NULL) {
     SafeStackTraceFrameIterator it(
         reinterpret_cast<Address>(sample->sp),
         reinterpret_cast<Address>(low_stack_bound_));
-    // Pass 1: Calculate depth
-    int depth = 0;
-    while (!it.done() && depth <= kMaxStackFrames) {
-      ++depth;
+    int i = 0;
+    while (!it.done() && i < TickSample::kMaxFramesCount) {
+      sample->stack[i++] = it.frame()->pc();
       it.Advance();
     }
-    // Pass 2: Save stack
-    sample->InitStack(depth);
-    if (depth > 0) {
-      it.Reset();
-      for (int i = 0; i < depth && !it.done(); ++i, it.Advance()) {
-        sample->stack[i] = it.frame()->pc();
-      }
-    }
+    sample->frames_count = i;
   } else if (sample->sp < sample->fp && sample->fp < low_stack_bound_) {
-    // The check assumes that stack grows from lower addresses
-    sample->InitStack(1);
+    // The check assumes that stack grows from lower addresses.
     sample->stack[0] = Memory::Address_at(
         (Address)(sample->fp + StandardFrameConstants::kCallerPCOffset));
+    sample->frames_count = 1;
   } else {
     // FP seems to be in some intermediate state,
     // better discard this sample
-    sample->InitStack(0);
+    sample->frames_count = 0;
   }
 }
 
@@ -945,10 +937,8 @@ void Logger::TickEvent(TickSample* sample, bool overflow) {
   if (overflow) {
     msg.Append(",overflow");
   }
-  if (*(sample->stack)) {
-    for (size_t i = 0; sample->stack[i]; ++i) {
-      msg.Append(",0x%x", reinterpret_cast<unsigned int>(sample->stack[i]));
-    }
+  for (int i = 0; i < sample->frames_count; ++i) {
+    msg.Append(",%p", sample->stack[i]);
   }
   msg.Append('\n');
   msg.WriteToLogFile();
diff --git a/src/log.h b/src/log.h
index 4adec19..1066e34 100644
--- a/src/log.h
+++ b/src/log.h
@@ -277,8 +277,6 @@ class StackTracer BASE_EMBEDDED {
       : low_stack_bound_(low_stack_bound) { }
   void Trace(TickSample* sample);
  private:
-  // Maximum number of stack frames to capture
-  static const int kMaxStackFrames = 5;
 
   unsigned int low_stack_bound_;
 };
diff --git a/src/platform.h b/src/platform.h
index b58067f..3293708 100644
--- a/src/platform.h
+++ b/src/platform.h
@@ -466,26 +466,9 @@ class TickSample {
   unsigned int sp;  // Stack pointer.
   unsigned int fp;  // Frame pointer.
   StateTag state;   // The state of the VM.
-  SmartPointer<Address> stack;  // Call stack, null-terminated.
-
-  inline TickSample& operator=(const TickSample& rhs) {
-    if (this == &rhs) return *this;
-    pc = rhs.pc;
-    sp = rhs.sp;
-    fp = rhs.fp;
-    state = rhs.state;
-    DeleteArray(stack.Detach());
-    stack = rhs.stack;
-    return *this;
-  }
-
-  inline void InitStack(int depth) {
-    if (depth) {
-      stack = SmartPointer<Address>(NewArray<Address>(depth + 1));
-      // null-terminate
-      stack[depth] = 0;
-    }
-  }
+  static const int kMaxFramesCount = 5;
+  EmbeddedVector<Address, kMaxFramesCount> stack;  // Call stack.
+  int frames_count;  // Number of captured frames.
 };
 
 class Sampler {
diff --git a/test/cctest/test-log-ia32.cc b/test/cctest/test-log-ia32.cc
index 6d12715..588be71 100644
--- a/test/cctest/test-log-ia32.cc
+++ b/test/cctest/test-log-ia32.cc
@@ -94,9 +94,9 @@ TEST(PureCStackTrace) {
 #ifdef DEBUG
   // C stack trace works only in debug mode, in release mode EBP is
   // usually treated as a general-purpose register
+  CHECK_GT(sample.frames_count, 0);
   CheckRetAddrIsInCFunction(reinterpret_cast<unsigned int>(sample.stack[0]),
                             reinterpret_cast<unsigned int>(&CFunc));
-  CHECK_EQ(0, sample.stack[1]);
 #endif
 }
 
@@ -217,7 +217,7 @@ TEST(PureJSStackTrace) {
       "  JSFuncDoTrace();"
       "};\n"
       "JSTrace();");
-  CHECK_NE(0, *(sample.stack));
+  CHECK_GT(sample.frames_count, 1);
   CheckRetAddrIsInFunction(
       reinterpret_cast<unsigned int>(sample.stack[0]),
       reinterpret_cast<unsigned int>(call_trace_code->instruction_start()),
-- 
2.7.4