Remove code that analyzes tos values from tickprocessor

Assuming that the value on top of stack is return address for a frameless invocation is error-prone. Corresponding logic was removed from profile-generator.cc in r14205 (see https://code.google.com/p/v8/source/diff?spec=svn14205&r=14205&format=side&path=/branches/bleeding_edge/src/profile-generator.cc) and now it is time to remove it from the tick processor.

Since the tos is not used anymore by profiler it is also removed from TickSample.

BUG=None

Review URL: https://codereview.chromium.org/13873009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14252 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/log.cc b/src/log.cc
index 9a52d80..3e1ff1e 100644 (file)
--- a/src/log.cc
+++ b/src/log.cc
@@ -127,16 +127,7 @@ DISABLE_ASAN void StackTracer::Trace(Isolate* isolate, TickSample* sample) {
     return;
   }
 
-  const Address callback = isolate->external_callback();
-  if (callback != NULL) {
-    sample->external_callback = callback;
-    sample->has_external_callback = true;
-  } else {
-    // Sample potential return address value for frameless invocation of
-    // stubs (we'll figure out later, if this value makes sense).
-    sample->tos = Memory::Address_at(sample->sp);
-    sample->has_external_callback = false;
-  }
+  sample->external_callback = isolate->external_callback();
 
   SafeStackTraceFrameIterator it(isolate,
                                  sample->fp, sample->sp,
@@ -1492,13 +1483,7 @@ void Logger::TickEvent(TickSample* sample, bool overflow) {
   msg.Append(',');
   msg.AppendAddress(sample->sp);
   msg.Append(",%ld", static_cast<int>(OS::Ticks() - epoch_));
-  if (sample->has_external_callback) {
-    msg.Append(",1,");
-    msg.AppendAddress(sample->external_callback);
-  } else {
-    msg.Append(",0,");
-    msg.AppendAddress(sample->tos);
-  }
+  msg.AppendAddress(sample->external_callback);
   msg.Append(",%d", static_cast<int>(sample->state));
   if (overflow) {
     msg.Append(",overflow");

diff --git a/src/platform.h b/src/platform.h
index 1c655da..3aeb38e 100644 (file)
--- a/src/platform.h
+++ b/src/platform.h
@@ -742,21 +742,16 @@ class TickSample {
         pc(NULL),
         sp(NULL),
         fp(NULL),
-        tos(NULL),
-        frames_count(0),
-        has_external_callback(false) {}
+        external_callback(NULL),
+        frames_count(0) {}
   StateTag state;  // The state of the VM.
   Address pc;      // Instruction pointer.
   Address sp;      // Stack pointer.
   Address fp;      // Frame pointer.
-  union {
-    Address tos;   // Top stack value (*sp).
-    Address external_callback;
-  };
+  Address external_callback;
   static const int kMaxFramesCount = 64;
   Address stack[kMaxFramesCount];  // Call stack.
   int frames_count : 8;  // Number of captured frames.
-  bool has_external_callback : 1;
 };
 
 class Sampler {

diff --git a/src/profile-generator.cc b/src/profile-generator.cc
index e4c750c..eacabef 100644 (file)
--- a/src/profile-generator.cc
+++ b/src/profile-generator.cc
@@ -894,7 +894,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
   if (sample.pc != NULL) {
     *entry++ = code_map_.FindEntry(sample.pc);
 
-    if (sample.has_external_callback) {
+    if (sample.external_callback) {
       // Don't use PC when in external callback code, as it can point
       // inside callback's code, and we will erroneously report
       // that a callback calls itself.

diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc
index 6f895ae..ac5fe86 100644 (file)
--- a/test/cctest/test-cpu-profiler.cc
+++ b/test/cctest/test-cpu-profiler.cc
@@ -64,7 +64,6 @@ static void EnqueueTickSampleEvent(ProfilerEventsProcessor* proc,
                                    i::Address frame3 = NULL) {
   i::TickSample* sample = proc->TickSampleEvent();
   sample->pc = frame1;
-  sample->tos = frame1;
   sample->frames_count = 0;
   if (frame2 != NULL) {
     sample->stack[0] = frame2;
@@ -239,7 +238,6 @@ TEST(Issue1398) {
 
   i::TickSample* sample = processor.TickSampleEvent();
   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);

diff --git a/test/cctest/test-log-stack-tracer.cc b/test/cctest/test-log-stack-tracer.cc
index 402a05c..bd7c801 100644 (file)
--- a/test/cctest/test-log-stack-tracer.cc
+++ b/test/cctest/test-log-stack-tracer.cc
@@ -287,7 +287,7 @@ TEST(CFromJSStackTrace) {
   //         DoTrace(EBP) [native]
   //           StackTracer::Trace
 
-  CHECK(sample.has_external_callback);
+  CHECK(sample.external_callback);
   CHECK_EQ(FUNCTION_ADDR(TraceExtension::Trace), sample.external_callback);
 
   // Stack tracing will start from the first JS function, i.e. "JSFuncDoTrace"
@@ -337,7 +337,7 @@ TEST(PureJSStackTrace) {
   //             StackTracer::Trace
   //
 
-  CHECK(sample.has_external_callback);
+  CHECK(sample.external_callback);
   CHECK_EQ(FUNCTION_ADDR(TraceExtension::JSTrace), sample.external_callback);
 
   // Stack sampling will start from the caller of JSFuncDoTrace, i.e. "JSTrace"

diff --git a/test/cctest/test-profile-generator.cc b/test/cctest/test-profile-generator.cc
index 56b1788..0682fbc 100644 (file)
--- a/test/cctest/test-profile-generator.cc
+++ b/test/cctest/test-profile-generator.cc
@@ -624,13 +624,11 @@ TEST(RecordTickSample) {
   //      -> ccc -> aaa  - sample3
   TickSample sample1;
   sample1.pc = ToAddress(0x1600);
-  sample1.tos = ToAddress(0x1500);
   sample1.stack[0] = ToAddress(0x1510);
   sample1.frames_count = 1;
   generator.RecordTickSample(sample1);
   TickSample sample2;
   sample2.pc = ToAddress(0x1925);
-  sample2.tos = ToAddress(0x1900);
   sample2.stack[0] = ToAddress(0x1780);
   sample2.stack[1] = ToAddress(0x10000);  // non-existent.
   sample2.stack[2] = ToAddress(0x1620);
@@ -638,7 +636,6 @@ TEST(RecordTickSample) {
   generator.RecordTickSample(sample2);
   TickSample sample3;
   sample3.pc = ToAddress(0x1510);
-  sample3.tos = ToAddress(0x1500);
   sample3.stack[0] = ToAddress(0x1910);
   sample3.stack[1] = ToAddress(0x1610);
   sample3.frames_count = 2;

diff --git a/test/mjsunit/tools/tickprocessor-test-func-info.log b/test/mjsunit/tools/tickprocessor-test-func-info.log
index 5e64dc0..fccd87e 100644 (file)
--- a/test/mjsunit/tools/tickprocessor-test-func-info.log
+++ b/test/mjsunit/tools/tickprocessor-test-func-info.log
@@ -5,7 +5,7 @@ profiler,"begin",1
 code-creation,Stub,0,0x424260,348,"CompareStub_GE"
 code-creation,LazyCompile,0,0x2a8100,18535,"DrawQube 3d-cube.js:188",0xf43abcac,
 code-creation,LazyCompile,0,0x480100,3908,"DrawLine 3d-cube.js:17",0xf43abc50,
-tick,0x424284,0xbfffeea0,0,0,0x480600,0,0x2aaaa5
-tick,0x42429f,0xbfffed88,0,0,0x480600,0,0x2aacb4
-tick,0x48063d,0xbfffec7c,0,0,0x2d0f7c,0,0x2aaec6
+tick,0x424284,0xbfffeea0,0,0,0,0x480600,0x2aaaa5
+tick,0x42429f,0xbfffed88,0,0,0,0x480600,0x2aacb4
+tick,0x48063d,0xbfffec7c,0,0,0,0x2d0f7c,0x2aaec6
 profiler,"end"

diff --git a/test/mjsunit/tools/tickprocessor-test.log b/test/mjsunit/tools/tickprocessor-test.log
index 5ddad89..2733513 100644 (file)
--- a/test/mjsunit/tools/tickprocessor-test.log
+++ b/test/mjsunit/tools/tickprocessor-test.log
@@ -9,17 +9,17 @@ code-creation,LazyCompile,0,0xf541d120,145,"exp native math.js:41"
 function-creation,0xf441d280,0xf541d120
 code-creation,LoadIC,0,0xf541d280,117,"j"
 code-creation,LoadIC,0,0xf541d360,63,"i"
-tick,0x80f82d1,0xffdfe880,0,0,0,0,0xf541ce5c
-tick,0x80f89a1,0xffdfecf0,0,0,0,0,0xf541ce5c
-tick,0x8123b5c,0xffdff1a0,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0x8123b65,0xffdff1a0,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xf541d2be,0xffdff1e4,0,0,0,0
-tick,0xf541d320,0xffdff1dc,0,0,0,0
-tick,0xf541d384,0xffdff1d8,0,0,0,0
-tick,0xf7db94da,0xffdff0ec,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xf7db951c,0xffdff0f0,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xf7dbc508,0xffdff14c,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xf7dbff21,0xffdff198,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xf7edec90,0xffdff0ec,0,0,0,0,0xf541d1a1,0xf541ceea
-tick,0xffffe402,0xffdff488,0,0,0,0
+tick,0x80f82d1,0xffdfe880,0,0,0,0xf541ce5c
+tick,0x80f89a1,0xffdfecf0,0,0,0,0xf541ce5c
+tick,0x8123b5c,0xffdff1a0,0,0,0,0xf541d1a1,0xf541ceea
+tick,0x8123b65,0xffdff1a0,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xf541d2be,0xffdff1e4,0,0,0
+tick,0xf541d320,0xffdff1dc,0,0,0
+tick,0xf541d384,0xffdff1d8,0,0,0
+tick,0xf7db94da,0xffdff0ec,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xf7db951c,0xffdff0f0,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xf7dbc508,0xffdff14c,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xf7dbff21,0xffdff198,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xf7edec90,0xffdff0ec,0,0,0,0xf541d1a1,0xf541ceea
+tick,0xffffe402,0xffdff488,0,0,0
 profiler,"end"

diff --git a/tools/tickprocessor.js b/tools/tickprocessor.js
index c9ee101..0ffe734 100644 (file)
--- a/tools/tickprocessor.js
+++ b/tools/tickprocessor.js
@@ -170,7 +170,7 @@ function TickProcessor(
           processor: this.processSnapshotPosition },
       'tick': {
           parsers: [parseInt, parseInt, parseInt, parseInt,
-                    parseInt, parseInt, 'var-args'],
+                    parseInt, 'var-args'],
           processor: this.processTick },
       'heap-sample-begin': { parsers: [null, null, parseInt],
           processor: this.processHeapSampleBegin },
@@ -368,8 +368,7 @@ TickProcessor.prototype.includeTick = function(vmState) {
 TickProcessor.prototype.processTick = function(pc,
                                                sp,
                                                ns_since_start,
-                                               is_external_callback,
-                                               tos_or_external_callback,
+                                               external_callback,
                                                vmState,
                                                stack) {
   this.distortion += this.distortion_per_entry;
@@ -383,23 +382,15 @@ TickProcessor.prototype.processTick = function(pc,
     this.ticks_.excluded++;
     return;
   }
-  if (is_external_callback) {
+  if (external_callback) {
     // Don't use PC when in external callback code, as it can point
     // inside callback's code, and we will erroneously report
     // that a callback calls itself. Instead we use tos_or_external_callback,
     // as simply resetting PC will produce unaccounted ticks.
-    pc = tos_or_external_callback;
-    tos_or_external_callback = 0;
-  } else if (tos_or_external_callback) {
-    // Find out, if top of stack was pointing inside a JS function
-    // meaning that we have encountered a frameless invocation.
-    var funcEntry = this.profile_.findEntry(tos_or_external_callback);
-    if (!funcEntry || !funcEntry.isJSFunction || !funcEntry.isJSFunction()) {
-      tos_or_external_callback = 0;
-    }
-  }
+    pc = 0;
+ }
 
-  this.profile_.recordTick(this.processStack(pc, tos_or_external_callback, stack));
+  this.profile_.recordTick(this.processStack(pc, external_callback, stack));
 };