AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert of [profiler] Fix attribution for the top-most interpreted frame. (patchset #3 id:40001 of https://codereview.chromium.org/2667253004/ )

Browse Source 
Reason for revert:
Flaky crashes on mac asan:
https://build.chromium.org/p/client.v8/builders/V8%20Mac64%20ASAN/builds/10739

Original issue's description:
> [profiler] Fix attribution for the top-most interpreted frame.
>
> Before this change, we attributed samples for the top-most interpreter frame to the second-topmost frame if we were in a bytecode handler with elided frame. With this change we try to detect that we are in a handler without a frame. If we are, we do not drop the topmost frame.
>
> For example, consider the program
>
> function inner() {
>   var s = 0;
>   for (var i = 0; i < 100000; i++) {
>     s += i * i;
>   }
>   return s;
> }
>
> function trivial() {
>   return inner();
> }
>
> for (var i = 0; i < 2000; i++) {
>   trivial();
> }
>
>
> Before this change, d8 --prof --ignition --nocrankshaft and linux-tick-processor would produce:
>
>   [JavaScript]:
>    ticks  total  nonlib   name
>    4885   83.4%   83.5%  Function: ~trivial a.js:15:17
>     759   13.0%   13.0%  Function: ~inner a.js:7:15
>
> After this change, we get
>
>  [JavaScript]:
>    ticks  total  nonlib   name
>    5486   95.9%   96.2%  Function: ~inner a.js:7:15
>       4    0.1%    0.1%  Function: ~trivial a.js:15:17
>
> Review-Url: https://codereview.chromium.org/2667253004
> Cr-Commit-Position: refs/heads/master@{#42894}
> Committed: d07f6540c1

TBR=bmeurer@chromium.org,jarin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2670843005
Cr-Commit-Position: refs/heads/master@{#42912}
This commit is contained in:
machenbach 2017-02-02 23:37:32 -08:00 committed by Commit bot
parent dac1327596
commit 63dea876da
1 changed files with 17 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
src/frames.cc
View File

@ -180,22 +180,6 @@ void StackTraceFrameIterator::AdvanceToArgumentsFrame() {
DCHECK(iterator_.frame()->is_arguments_adaptor());
}
static bool IsInterpreterFramePc(Isolate* isolate, Address pc) {
Code* interpreter_entry_trampoline =
isolate->builtins()->builtin(Builtins::kInterpreterEntryTrampoline);
Code* interpreter_bytecode_advance =
isolate->builtins()->builtin(Builtins::kInterpreterEnterBytecodeAdvance);
Code* interpreter_bytecode_dispatch =
isolate->builtins()->builtin(Builtins::kInterpreterEnterBytecodeDispatch);
return (pc >= interpreter_entry_trampoline->instruction_start() &&
pc < interpreter_entry_trampoline->instruction_end()) ||
(pc >= interpreter_bytecode_advance->instruction_start() &&
pc < interpreter_bytecode_advance->instruction_end()) ||
(pc >= interpreter_bytecode_dispatch->instruction_start() &&
pc < interpreter_bytecode_dispatch->instruction_end());
}
// -------------------------------------------------------------------------
@ -210,7 +194,6 @@ SafeStackFrameIterator::SafeStackFrameIterator(
StackFrame::State state;
StackFrame::Type type;
ThreadLocalTop* top = isolate->thread_local_top();
bool advance_frame = true;
if (IsValidTop(top)) {
type = ExitFrame::GetStateForFramePointer(Isolate::c_entry_fp(top), &state);
top_frame_type_ = type;
@ -220,19 +203,6 @@ SafeStackFrameIterator::SafeStackFrameIterator(
state.sp = sp;
state.pc_address = StackFrame::ResolveReturnAddressLocation(
reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp)));
// If the top of stack is a return address to the interpreter trampoline,
// then we are likely in a bytecode handler with elided frame. In that
// case, set the PC properly and make sure we do not drop the frame.
if (IsValidStackAddress(sp)) {
MSAN_MEMORY_IS_INITIALIZED(sp, kPointerSize);
Address tos = Memory::Address_at(reinterpret_cast<Address>(sp));
if (IsInterpreterFramePc(isolate, tos)) {
state.pc_address = reinterpret_cast<Address*>(sp);
advance_frame = false;
}
}
// StackFrame::ComputeType will read both kContextOffset and kMarkerOffset,
// we check only that kMarkerOffset is within the stack bounds and do
// compile time check that kContextOffset slot is pushed on the stack before
@ -243,10 +213,6 @@ SafeStackFrameIterator::SafeStackFrameIterator(
if (IsValidStackAddress(frame_marker)) {
type = StackFrame::ComputeType(this, &state);
top_frame_type_ = type;
// We only keep the top frame if we believe it to be interpreted frame.
if (type != StackFrame::INTERPRETED) {
advance_frame = true;
}
} else {
// Mark the frame as JAVA_SCRIPT if we cannot determine its type.
// The frame anyways will be skipped.
@ -258,7 +224,7 @@ SafeStackFrameIterator::SafeStackFrameIterator(
return;
}
frame_ = SingletonFor(type, &state);
if (advance_frame && frame_) Advance();
if (frame_) Advance();
}
@ -423,6 +389,22 @@ void StackFrame::SetReturnAddressLocationResolver(
return_address_location_resolver_ = resolver;
}
static bool IsInterpreterFramePc(Isolate* isolate, Address pc) {
Code* interpreter_entry_trampoline =
isolate->builtins()->builtin(Builtins::kInterpreterEntryTrampoline);
Code* interpreter_bytecode_advance =
isolate->builtins()->builtin(Builtins::kInterpreterEnterBytecodeAdvance);
Code* interpreter_bytecode_dispatch =
isolate->builtins()->builtin(Builtins::kInterpreterEnterBytecodeDispatch);
return (pc >= interpreter_entry_trampoline->instruction_start() &&
pc < interpreter_entry_trampoline->instruction_end()) ||
(pc >= interpreter_bytecode_advance->instruction_start() &&
pc < interpreter_bytecode_advance->instruction_end()) ||
(pc >= interpreter_bytecode_dispatch->instruction_start() &&
pc < interpreter_bytecode_dispatch->instruction_end());
}
StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
State* state) {
DCHECK(state->fp != NULL);