From: yurys@chromium.org Date: Fri, 12 Apr 2013 11:20:22 +0000 (+0000) Subject: Remove code that analyzes tos values from tickprocessor X-Git-Tag: upstream/4.7.83~14595 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5206b0845131a03ab21c391906aebedcec08884e;p=platform%2Fupstream%2Fv8.git 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 --- 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(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(sample->state)); if (overflow) { msg.Append(",overflow"); diff --git a/src/platform.h b/src/platform.h index 1c655da..3aeb38e 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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)); };