[cpu-profiler] use new source position information for deoptimization in cpu profiler

src/profiler/profiler-listener.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/profiler/profiler-listener.h"
 
 #include "src/deoptimizer.h"
 #include "src/profiler/cpu-profiler.h"
 #include "src/profiler/profile-generator-inl.h"
 #include "src/source-position-table.h"
@@ skipping 76 matching lines @@
   Script* script = Script::cast(shared->script());
   JITLineInfoTable* line_table = NULL;
   if (script) {
     line_table = new JITLineInfoTable();
     int offset = abstract_code->IsCode() ? Code::kHeaderSize
                                          : BytecodeArray::kHeaderSize;
     int start_position = shared->start_position();
     int end_position = shared->end_position();
     for (SourcePositionTableIterator it(abstract_code->source_position_table());
          !it.done(); it.Advance()) {
+      if (it.source_position().InliningId() != SourcePosition::kNotInlined)

[alph, 2016/11/17 00:09:44]

Why do you skip inlined positions?
If it's not ready for this patch could you please keep the TODO.

[Tobias Tebbi, 2016/11/17 11:41:34]

Sorry, yes, the TODO should stay. I skip inlined positions to maintain the old
behavior. Is it safe to add inlined positions here as long as they are in the
same script? We can easily get all needed information here, including the
inlining stack and script ids. I just don't understand the cpu profiler well
enough to know what to do here.

+        continue;
       int position = it.source_position().ScriptOffset();
-      // TODO(alph): in case of inlining the position may correspond to an
-      // inlined function source code. Do not collect positions that fall
-      // beyond the function source code. There's however a chance the
-      // inlined function has similar positions but in another script. So
-      // the proper fix is to store script_id in some form along with the
-      // inlined function positions.
-      if (position < start_position || position >= end_position) continue;
       int line_number = script->GetLineNumber(position) + 1;
       int pc_offset = it.code_offset() + offset;
       line_table->SetPosition(pc_offset, line_number);
     }
   }
   rec->entry = NewCodeEntry(
       tag, GetFunctionName(shared->DebugName()), CodeEntry::kEmptyNamePrefix,
       GetName(InferScriptName(script_name, shared)), line, column, line_table,
       abstract_code->instruction_start());
   RecordInliningInfo(rec->entry, abstract_code);
@@ skipping 34 matching lines @@
   DispatchCodeEvent(evt_rec);
 }
 
 void ProfilerListener::CodeDeoptEvent(Code* code, Address pc,
                                       int fp_to_sp_delta) {
   CodeEventsContainer evt_rec(CodeEventRecord::CODE_DEOPT);
   CodeDeoptEventRecord* rec = &evt_rec.CodeDeoptEventRecord_;
   Deoptimizer::DeoptInfo info = Deoptimizer::GetDeoptInfo(code, pc);
   rec->start = code->address();
   rec->deopt_reason = DeoptimizeReasonToString(info.deopt_reason);
-  rec->position = info.position;
   rec->deopt_id = info.deopt_id;
   rec->pc = reinterpret_cast<void*>(pc);
   rec->fp_to_sp_delta = fp_to_sp_delta;
   DispatchCodeEvent(evt_rec);
 }
 
 void ProfilerListener::GetterCallbackEvent(Name* name, Address entry_point) {
   CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
   CodeCreateEventRecord* rec = &evt_rec.CodeCreateEventRecord_;
   rec->start = entry_point;
@@ skipping 77 matching lines @@
     if (!inline_stack.empty()) {
       entry->AddInlineStack(pc_offset, inline_stack);
       DCHECK(inline_stack.empty());
     }
   }
 }
 
 void ProfilerListener::RecordDeoptInlinedFrames(CodeEntry* entry,
                                                 AbstractCode* abstract_code) {
   if (abstract_code->kind() != AbstractCode::OPTIMIZED_FUNCTION) return;
-  Code* code = abstract_code->GetCode();
-  DeoptimizationInputData* deopt_input_data =
-      DeoptimizationInputData::cast(code->deoptimization_data());
-  int const mask = RelocInfo::ModeMask(RelocInfo::DEOPT_ID);
-  for (RelocIterator rit(code, mask); !rit.done(); rit.next()) {
-    RelocInfo* reloc_info = rit.rinfo();
-    DCHECK(RelocInfo::IsDeoptId(reloc_info->rmode()));
-    int deopt_id = static_cast<int>(reloc_info->data());
-    int translation_index =
-        deopt_input_data->TranslationIndex(deopt_id)->value();
-    TranslationIterator it(deopt_input_data->TranslationByteArray(),
-                           translation_index);
-    Translation::Opcode opcode = static_cast<Translation::Opcode>(it.Next());
-    DCHECK_EQ(Translation::BEGIN, opcode);
-    it.Skip(Translation::NumberOfOperandsFor(opcode));
-    std::vector<CodeEntry::DeoptInlinedFrame> inlined_frames;
-    while (it.HasNext() &&
-           Translation::BEGIN !=
-               (opcode = static_cast<Translation::Opcode>(it.Next()))) {
-      if (opcode != Translation::JS_FRAME &&
-          opcode != Translation::INTERPRETED_FRAME) {
-        it.Skip(Translation::NumberOfOperandsFor(opcode));
-        continue;
+  Handle<Code> code(abstract_code->GetCode());

[alph, 2016/11/17 00:09:44]

Why it needs a handle?

[Tobias Tebbi, 2016/11/17 11:41:34]

It needs a handle because last_position.InliningStack(code) is handlified (the
reason is that it might initialize line ends, which allocates). Is there a
reason why it should stay unhandlified? Is it safe to allocate from the profile
listener?

[alph, 2016/11/18 22:51:46]

Makes sense. Thanks. Handles are slower than raw access, so I'd avoid them
whenever possible.

+
+  SourcePosition last_position = SourcePosition::Unknown();
+  int mask = RelocInfo::ModeMask(RelocInfo::DEOPT_ID) |
+             RelocInfo::ModeMask(RelocInfo::DEOPT_SCRIPT_OFFSET) |
+             RelocInfo::ModeMask(RelocInfo::DEOPT_INLINING_ID);
+  for (RelocIterator it(*code, mask); !it.done(); it.next()) {
+    RelocInfo* info = it.rinfo();
+    if (info->rmode() == RelocInfo::DEOPT_SCRIPT_OFFSET) {
+      int script_offset = static_cast<int>(info->data());
+      it.next();
+      DCHECK(it.rinfo()->rmode() == RelocInfo::DEOPT_INLINING_ID);
+      int inlining_id = static_cast<int>(it.rinfo()->data());
+      last_position = SourcePosition(script_offset, inlining_id);

[alph, 2016/11/17 00:09:44]

nit: use continue

+    } else if (info->rmode() == RelocInfo::DEOPT_ID) {
+      int deopt_id = static_cast<int>(info->data());
+      if (!last_position.IsKnown()) continue;
+      std::vector<CpuProfileDeoptFrame> inlined_frames;
+      for (SourcePositionInfo& pos_info : last_position.InliningStack(code)) {
+        DCHECK(pos_info.position.ScriptOffset() != kNoSourcePosition);
+        size_t offset = static_cast<size_t>(pos_info.position.ScriptOffset());
+        int script_id = Script::cast(pos_info.function->script())->id();
+        inlined_frames.push_back(CpuProfileDeoptFrame({script_id, offset}));
       }
-      BailoutId ast_id = BailoutId(it.Next());
-      int shared_info_id = it.Next();
-      it.Next();  // Skip height
-      SharedFunctionInfo* shared = SharedFunctionInfo::cast(
-          deopt_input_data->LiteralArray()->get(shared_info_id));
-      int source_position;
-      if (opcode == Translation::INTERPRETED_FRAME) {
-        source_position =
-            Deoptimizer::ComputeSourcePositionFromBytecodeArray(shared, ast_id);
-      } else {
-        DCHECK(opcode == Translation::JS_FRAME);
-        source_position =
-            Deoptimizer::ComputeSourcePositionFromBaselineCode(shared, ast_id);
+      if (!inlined_frames.empty() &&
+          !entry->HasDeoptInlinedFramesFor(deopt_id)) {
+        entry->AddDeoptInlinedFrames(deopt_id, inlined_frames);
+        DCHECK(inlined_frames.empty());
       }
-      int script_id = v8::UnboundScript::kNoScriptId;
-      if (shared->script()->IsScript()) {
-        Script* script = Script::cast(shared->script());
-        script_id = script->id();
-      }
-      CodeEntry::DeoptInlinedFrame frame = {source_position, script_id};
-      inlined_frames.push_back(frame);
-    }
-    if (!inlined_frames.empty() && !entry->HasDeoptInlinedFramesFor(deopt_id)) {
-      entry->AddDeoptInlinedFrames(deopt_id, inlined_frames);
-      DCHECK(inlined_frames.empty());
     }
   }
 }
 
 CodeEntry* ProfilerListener::NewCodeEntry(
     CodeEventListener::LogEventsAndTags tag, const char* name,
     const char* name_prefix, const char* resource_name, int line_number,
     int column_number, JITLineInfoTable* line_info, Address instruction_start) {
   CodeEntry* code_entry =
       new CodeEntry(tag, name, name_prefix, resource_name, line_number,
@@ skipping 12 matching lines @@
 
 void ProfilerListener::RemoveObserver(CodeEventObserver* observer) {
   base::LockGuard<base::Mutex> guard(&mutex_);
   auto it = std::find(observers_.begin(), observers_.end(), observer);
   if (it == observers_.end()) return;
   observers_.erase(it);
 }
 
 }  // namespace internal
 }  // namespace v8