Make the frame inspector use TranslatedState rather than the full deoptimizer.

src/deoptimizer.cc

Index: src/deoptimizer.cc
diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc
index ba77292279f2d7a0f5da22625d33ce543d8bc12d..65b7b1c0f257e6244197b02968d39dc4ec8baee5 100644
--- a/src/deoptimizer.cc
+++ b/src/deoptimizer.cc
@@ -134,70 +134,26 @@ DeoptimizedFrameInfo* Deoptimizer::DebuggerInspectableFrame(
     Isolate* isolate) {
   CHECK(frame->is_optimized());
 
-  // Get the function and code from the frame.
-  JSFunction* function = frame->function();
-  Code* code = frame->LookupCode();
-
-  // Locate the deoptimization point in the code. As we are at a call the
-  // return address must be at a place in the code with deoptimization support.
-  SafepointEntry safepoint_entry = code->GetSafepointEntry(frame->pc());
-  int deoptimization_index = safepoint_entry.deoptimization_index();
-  CHECK_NE(deoptimization_index, Safepoint::kNoDeoptimizationIndex);
-
-  // Always use the actual stack slots when calculating the fp to sp
-  // delta adding two for the function and context.
-  unsigned stack_slots = code->stack_slots();
-  unsigned arguments_stack_height =
-      Deoptimizer::ComputeOutgoingArgumentSize(code, deoptimization_index);
-  unsigned fp_to_sp_delta = (stack_slots * kPointerSize) +
-                            StandardFrameConstants::kFixedFrameSizeFromFp +
-                            arguments_stack_height;
-
-  Deoptimizer* deoptimizer = new Deoptimizer(isolate,
-                                             function,
-                                             Deoptimizer::DEBUGGER,
-                                             deoptimization_index,
-                                             frame->pc(),
-                                             fp_to_sp_delta,
-                                             code);
-  Address tos = frame->fp() - fp_to_sp_delta;
-  deoptimizer->FillInputFrame(tos, frame);
-
-  // Calculate the output frames.
-  Deoptimizer::ComputeOutputFrames(deoptimizer);
-
-  // Create the GC safe output frame information and register it for GC
-  // handling.
-  CHECK_LT(jsframe_index, deoptimizer->jsframe_count());
-
-  // Convert JS frame index into frame index.
-  int frame_index = deoptimizer->ConvertJSFrameIndexToFrameIndex(jsframe_index);
-
-  bool has_arguments_adaptor =
-      frame_index > 0 &&
-      deoptimizer->output_[frame_index - 1]->GetFrameType() ==
-      StackFrame::ARGUMENTS_ADAPTOR;
-
-  int construct_offset = has_arguments_adaptor ? 2 : 1;
-  bool has_construct_stub =
-      frame_index >= construct_offset &&
-      deoptimizer->output_[frame_index - construct_offset]->GetFrameType() ==
-      StackFrame::CONSTRUCT;
-
-  DeoptimizedFrameInfo* info = new DeoptimizedFrameInfo(deoptimizer,
-                                                        frame_index,
-                                                        has_arguments_adaptor,
-                                                        has_construct_stub);
-
-  // Done with the GC-unsafe frame descriptions. This re-enables allocation.
-  deoptimizer->DeleteFrameDescriptions();
-
-  // Allocate a heap number for the doubles belonging to this frame.
-  deoptimizer->MaterializeHeapNumbersForDebuggerInspectableFrame(
-      frame_index, info->parameters_count(), info->expression_count(), info);
+  TranslatedState translated_values(frame);
+  translated_values.Prepare(false, frame->fp());
+
+  TranslatedState::iterator frame_it = translated_values.end();
+  int counter = jsframe_index;
+  for (auto it = translated_values.begin(); it != translated_values.end();
+       it++) {
+    if (it->kind() == TranslatedFrame::kFunction ||
+        it->kind() == TranslatedFrame::kInterpretedFunction) {
+      if (counter == 0) {
+        frame_it = it;
+        break;
+      }
+      counter--;
+    }
+  }
+  CHECK(frame_it != translated_values.end());
 
-  // Finished using the deoptimizer instance.
-  delete deoptimizer;
+  DeoptimizedFrameInfo* info =
+      new DeoptimizedFrameInfo(&translated_values, frame_it, isolate);
 
   return info;
 }
@@ -563,7 +519,11 @@ Deoptimizer::Deoptimizer(Isolate* isolate, JSFunction* function,
     PROFILE(isolate_, CodeDeoptEvent(compiled_code_, from_, fp_to_sp_delta_));
   }
   unsigned size = ComputeInputFrameSize();
-  input_ = new(size) FrameDescription(size, function);
+  int parameter_count =
+      function == nullptr
+          ? 0
+          : (function->shared()->internal_formal_parameter_count() + 1);
+  input_ = new (size) FrameDescription(size, parameter_count);
   input_->SetFrameType(frame_type);
 }
 
@@ -899,8 +859,9 @@ void Deoptimizer::DoComputeJSFrame(int frame_index, bool goto_catch_handler) {
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
-  FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, function);
+  int parameter_count = shared->internal_formal_parameter_count() + 1;
+  FrameDescription* output_frame = new (output_frame_size)
+      FrameDescription(output_frame_size, parameter_count);
   output_frame->SetFrameType(StackFrame::JAVA_SCRIPT);
 
   CHECK(frame_index >= 0 && frame_index < output_count_);
@@ -931,7 +892,6 @@ void Deoptimizer::DoComputeJSFrame(int frame_index, bool goto_catch_handler) {
   output_frame->SetTop(top_address);
 
   // Compute the incoming parameter translation.
-  int parameter_count = shared->internal_formal_parameter_count() + 1;
   unsigned output_offset = output_frame_size;
   unsigned input_offset = input_frame_size;
   for (int i = 0; i < parameter_count; ++i) {
@@ -1147,8 +1107,9 @@ void Deoptimizer::DoComputeInterpretedFrame(int frame_index,
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
-  FrameDescription* output_frame =
-      new (output_frame_size) FrameDescription(output_frame_size, function);
+  int parameter_count = shared->internal_formal_parameter_count() + 1;
+  FrameDescription* output_frame = new (output_frame_size)
+      FrameDescription(output_frame_size, parameter_count);
   output_frame->SetFrameType(StackFrame::INTERPRETED);
 
   bool is_bottommost = (0 == frame_index);
@@ -1175,7 +1136,6 @@ void Deoptimizer::DoComputeInterpretedFrame(int frame_index,
   output_frame->SetTop(top_address);
 
   // Compute the incoming parameter translation.
-  int parameter_count = shared->internal_formal_parameter_count() + 1;
   unsigned output_offset = output_frame_size;
   unsigned input_offset = input_frame_size;
   for (int i = 0; i < parameter_count; ++i) {
@@ -1383,8 +1343,9 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(int frame_index) {
   unsigned output_frame_size = height_in_bytes + fixed_frame_size;
 
   // Allocate and store the output frame description.
-  FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, function);
+  int parameter_count = height;
+  FrameDescription* output_frame = new (output_frame_size)
+      FrameDescription(output_frame_size, parameter_count);
   output_frame->SetFrameType(StackFrame::ARGUMENTS_ADAPTOR);
 
   // Arguments adaptor can not be topmost or bottommost.
@@ -1399,7 +1360,6 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(int frame_index) {
   output_frame->SetTop(top_address);
 
   // Compute the incoming parameter translation.
-  int parameter_count = height;
   unsigned output_offset = output_frame_size;
   for (int i = 0; i < parameter_count; ++i) {
     output_offset -= kPointerSize;
@@ -1479,7 +1439,7 @@ void Deoptimizer::DoComputeConstructStubFrame(int frame_index) {
   Code* construct_stub = builtins->builtin(Builtins::kJSConstructStubGeneric);
   unsigned height = translated_frame->height();
   unsigned height_in_bytes = height * kPointerSize;
-  JSFunction* function = JSFunction::cast(value_iterator->GetRawValue());
+  // Skip function.
   value_iterator++;
   input_index++;
   if (trace_scope_ != NULL) {
@@ -1492,7 +1452,7 @@ void Deoptimizer::DoComputeConstructStubFrame(int frame_index) {
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, function);
+      new (output_frame_size) FrameDescription(output_frame_size);
   output_frame->SetFrameType(StackFrame::CONSTRUCT);
 
   // Construct stub can not be topmost or bottommost.
@@ -1605,7 +1565,7 @@ void Deoptimizer::DoComputeAccessorStubFrame(int frame_index,
   TranslatedFrame::iterator value_iterator = translated_frame->begin();
   int input_index = 0;
 
-  JSFunction* accessor = JSFunction::cast(value_iterator->GetRawValue());
+  // Skip accessor.
   value_iterator++;
   input_index++;
   // The receiver (and the implicit return value, if any) are expected in
@@ -1632,7 +1592,7 @@ void Deoptimizer::DoComputeAccessorStubFrame(int frame_index,
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, accessor);
+      new (output_frame_size) FrameDescription(output_frame_size);
   output_frame->SetFrameType(StackFrame::INTERNAL);
 
   // A frame for an accessor stub can not be the topmost or bottommost one.
@@ -1791,7 +1751,7 @@ void Deoptimizer::DoComputeCompiledStubFrame(int frame_index) {
 
   // The stub failure trampoline is a single frame.
   FrameDescription* output_frame =
-      new(output_frame_size) FrameDescription(output_frame_size, NULL);
+      new (output_frame_size) FrameDescription(output_frame_size);
   output_frame->SetFrameType(StackFrame::STUB_FAILURE_TRAMPOLINE);
   CHECK_EQ(frame_index, 0);
   output_[frame_index] = output_frame;
@@ -1992,55 +1952,6 @@ void Deoptimizer::MaterializeHeapObjects(JavaScriptFrameIterator* it) {
 }
 
 
-void Deoptimizer::MaterializeHeapNumbersForDebuggerInspectableFrame(
-    int frame_index, int parameter_count, int expression_count,
-    DeoptimizedFrameInfo* info) {
-  CHECK_EQ(DEBUGGER, bailout_type_);
-
-  translated_state_.Prepare(false, nullptr);
-
-  TranslatedFrame* frame = &(translated_state_.frames()[frame_index]);
-  CHECK(frame->kind() == TranslatedFrame::kFunction);
-  int frame_arg_count = frame->shared_info()->internal_formal_parameter_count();
-
-  // The height is #expressions + 1 for context.
-  CHECK_EQ(expression_count + 1, frame->height());
-  TranslatedFrame* argument_frame = frame;
-  if (frame_index > 0) {
-    TranslatedFrame* previous_frame =
-        &(translated_state_.frames()[frame_index - 1]);
-    if (previous_frame->kind() == TranslatedFrame::kArgumentsAdaptor) {
-      argument_frame = previous_frame;
-      CHECK_EQ(parameter_count, argument_frame->height() - 1);
-    } else {
-      CHECK_EQ(frame_arg_count, parameter_count);
-    }
-  } else {
-    CHECK_EQ(frame_arg_count, parameter_count);
-  }
-
-  TranslatedFrame::iterator arg_iter = argument_frame->begin();
-  arg_iter++;  // Skip the function.
-  arg_iter++;  // Skip the receiver.
-  for (int i = 0; i < parameter_count; i++, arg_iter++) {
-    if (!arg_iter->IsMaterializedObject()) {
-      info->SetParameter(i, arg_iter->GetValue());
-    }
-  }
-
-  TranslatedFrame::iterator iter = frame->begin();
-  // Skip the function, receiver, context and arguments.
-  for (int i = 0; i < frame_arg_count + 3; i++, iter++) {
-  }
-
-  for (int i = 0; i < expression_count; i++, iter++) {
-    if (!iter->IsMaterializedObject()) {
-      info->SetExpression(i, iter->GetValue());
-    }
-  }
-}
-
-
 void Deoptimizer::WriteTranslatedValueToOutput(
     TranslatedFrame::iterator* iterator, int* input_index, int frame_index,
     unsigned output_offset, const char* debug_hint_string,
@@ -2193,11 +2104,9 @@ void Deoptimizer::EnsureCodeForDeoptimizationEntry(Isolate* isolate,
   data->deopt_entry_code_entries_[type] = entry_count;
 }
 
-
-FrameDescription::FrameDescription(uint32_t frame_size,
-                                   JSFunction* function)
+FrameDescription::FrameDescription(uint32_t frame_size, int parameter_count)
     : frame_size_(frame_size),
-      function_(function),
+      parameter_count_(parameter_count),
       top_(kZapUint32),
       pc_(kZapUint32),
       fp_(kZapUint32),
@@ -2221,10 +2130,10 @@ FrameDescription::FrameDescription(uint32_t frame_size,
 int FrameDescription::ComputeFixedSize() {
   if (type_ == StackFrame::INTERPRETED) {
     return InterpreterFrameConstants::kFixedFrameSize +
-           (ComputeParametersCount() + 1) * kPointerSize;
+           parameter_count() * kPointerSize;
   } else {
     return StandardFrameConstants::kFixedFrameSize +
-           (ComputeParametersCount() + 1) * kPointerSize;
+           parameter_count() * kPointerSize;
   }
 }
 
@@ -2237,54 +2146,13 @@ unsigned FrameDescription::GetOffsetFromSlotIndex(int slot_index) {
     return base - ((slot_index + 1) * kPointerSize);
   } else {
     // Incoming parameter.
-    int arg_size = (ComputeParametersCount() + 1) * kPointerSize;
+    int arg_size = parameter_count() * kPointerSize;
     unsigned base = GetFrameSize() - arg_size;
     return base - ((slot_index + 1) * kPointerSize);
   }
 }
 
 
-int FrameDescription::ComputeParametersCount() {
-  switch (type_) {
-    case StackFrame::JAVA_SCRIPT:
-      return function_->shared()->internal_formal_parameter_count();
-    case StackFrame::ARGUMENTS_ADAPTOR: {
-      // Last slot contains number of incomming arguments as a smi.
-      // Can't use GetExpression(0) because it would cause infinite recursion.
-      return reinterpret_cast<Smi*>(*GetFrameSlotPointer(0))->value();
-    }
-    case StackFrame::STUB:
-      return -1;  // Minus receiver.
-    default:
-      FATAL("Unexpected stack frame type");
-      return 0;
-  }
-}
-
-
-Object* FrameDescription::GetParameter(int index) {
-  CHECK_GE(index, 0);
-  CHECK_LT(index, ComputeParametersCount());
-  // The slot indexes for incoming arguments are negative.
-  unsigned offset = GetOffsetFromSlotIndex(index - ComputeParametersCount());
-  return reinterpret_cast<Object*>(*GetFrameSlotPointer(offset));
-}
-
-
-unsigned FrameDescription::GetExpressionCount() {
-  CHECK_EQ(StackFrame::JAVA_SCRIPT, type_);
-  unsigned size = GetFrameSize() - ComputeFixedSize();
-  return size / kPointerSize;
-}
-
-
-Object* FrameDescription::GetExpression(int index) {
-  DCHECK_EQ(StackFrame::JAVA_SCRIPT, type_);
-  unsigned offset = GetOffsetFromSlotIndex(index);
-  return reinterpret_cast<Object*>(*GetFrameSlotPointer(offset));
-}
-
-
 void TranslationBuffer::Add(int32_t value, Zone* zone) {
   // This wouldn't handle kMinInt correctly if it ever encountered it.
   DCHECK(value != kMinInt);
@@ -2606,50 +2474,110 @@ Handle<FixedArray> MaterializedObjectStore::EnsureStackEntries(int length) {
   return new_array;
 }
 
+namespace {
 
-DeoptimizedFrameInfo::DeoptimizedFrameInfo(Deoptimizer* deoptimizer,
-                                           int frame_index,
-                                           bool has_arguments_adaptor,
-                                           bool has_construct_stub) {
-  FrameDescription* output_frame = deoptimizer->output_[frame_index];
-  function_ =
-      Handle<JSFunction>(output_frame->GetFunction(), deoptimizer->isolate());
-  context_ =
-      Handle<Object>(reinterpret_cast<Object*>(output_frame->GetContext()),
-                     deoptimizer->isolate());
-  has_construct_stub_ = has_construct_stub;
-  expression_stack_.resize(
-      static_cast<size_t>(output_frame->GetExpressionCount()));
-  // Get the source position using the unoptimized code.
-  Address pc = reinterpret_cast<Address>(output_frame->GetPc());
-  Code* code = Code::cast(deoptimizer->isolate()->FindCodeObject(pc));
-  int offset = static_cast<int>(pc - code->instruction_start());
-  source_position_ = code->SourcePosition(offset);
-
-  for (int i = 0; i < static_cast<int>(expression_count()); i++) {
-    Object* value = output_frame->GetExpression(i);
-    // Replace materialization markers with the undefined value.
-    if (value == deoptimizer->isolate()->heap()->arguments_marker()) {
-      value = deoptimizer->isolate()->heap()->undefined_value();
+Handle<Object> GetValueForDebugger(TranslatedFrame::iterator it,
+                                   Isolate* isolate) {
+  if (it->GetRawValue() == isolate->heap()->arguments_marker()) {
+    if (!it->IsMaterializableByDebugger()) {
+      return isolate->factory()->undefined_value();
     }
-    SetExpression(i, Handle<Object>(value, deoptimizer->isolate()));
   }
+  return it->GetValue();
+}
 
-  if (has_arguments_adaptor) {
-    output_frame = deoptimizer->output_[frame_index - 1];
-    CHECK_EQ(output_frame->GetFrameType(), StackFrame::ARGUMENTS_ADAPTOR);
+int ComputeSourcePosition(Handle<SharedFunctionInfo> shared,
+                          BailoutId node_id) {
+  if (shared->HasBytecodeArray()) {
+    BytecodeArray* bytecodes = shared->bytecode_array();
+    return bytecodes->SourcePosition(node_id.ToInt());
+  } else {
+    Code* non_optimized_code = shared->code();
+    FixedArray* raw_data = non_optimized_code->deoptimization_data();
+    DeoptimizationOutputData* data = DeoptimizationOutputData::cast(raw_data);
+    unsigned pc_and_state = Deoptimizer::GetOutputInfo(data, node_id, *shared);
+    unsigned pc_offset = FullCodeGenerator::PcField::decode(pc_and_state);
+    return non_optimized_code->SourcePosition(pc_offset);
   }
+}
 
-  parameters_.resize(
-      static_cast<size_t>(output_frame->ComputeParametersCount()));
-  for (int i = 0; i < output_frame->ComputeParametersCount(); i++) {
-    Object* value = output_frame->GetParameter(i);
-    // Replace materialization markers with the undefined value.
-    if (value == deoptimizer->isolate()->heap()->arguments_marker()) {
-      value = deoptimizer->isolate()->heap()->undefined_value();
-    }
-    SetParameter(i, Handle<Object>(value, deoptimizer->isolate()));
+}  // namespace
+
+DeoptimizedFrameInfo::DeoptimizedFrameInfo(TranslatedState* state,
+                                           TranslatedState::iterator frame_it,
+                                           Isolate* isolate) {
+  // If the previous frame is an adaptor frame, we will take the parameters
+  // from there.
+  TranslatedState::iterator parameter_frame = frame_it;
+  if (parameter_frame != state->begin()) {
+    parameter_frame--;
+  }
+  int parameter_count;
+  if (parameter_frame->kind() == TranslatedFrame::kArgumentsAdaptor) {
+    parameter_count = parameter_frame->height() - 1;  // Ignore the receiver.
+  } else {
+    parameter_frame = frame_it;
+    parameter_count =
+        frame_it->shared_info()->internal_formal_parameter_count();
   }
+  TranslatedFrame::iterator parameter_it = parameter_frame->begin();
+  parameter_it++;  // Skip the function.
+  parameter_it++;  // Skip the receiver.
+
+  // Figure out whether there is a construct stub frame on top of
+  // the parameter frame.
+  has_construct_stub_ =
+      parameter_frame != state->begin() &&
+      (parameter_frame - 1)->kind() == TranslatedFrame::kConstructStub;
+
+  source_position_ =
+      ComputeSourcePosition(frame_it->shared_info(), frame_it->node_id());
+
+  TranslatedFrame::iterator value_it = frame_it->begin();
+  // Get the function. Note that this might materialize the function.
+  // In case the debugger mutates this value, we should deoptimize
+  // the function and remember the value in the materialized value store.
+  function_ = Handle<JSFunction>::cast(value_it->GetValue());
+
+  parameters_.resize(static_cast<size_t>(parameter_count));
+  DCHECK(parameter_count == parameters_count());

[Benedikt Meurer, 2016/02/15 05:48:26]

Nit: Remove this useless DCHECK.

[Jarin, 2016/02/15 06:53:54]

Done.

+  for (int i = 0; i < parameter_count; i++) {
+    Handle<Object> parameter = GetValueForDebugger(parameter_it, isolate);
+    SetParameter(i, parameter);
+    parameter_it++;
+  }
+
+  // Skip the function, the receiver and the arguments.
+  int skip_count =
+      frame_it->shared_info()->internal_formal_parameter_count() + 2;
+  TranslatedFrame::iterator stack_it = frame_it->begin();
+  for (int i = 0; i < skip_count; i++) {
+    stack_it++;
+  }
+
+  // Get the context.
+  context_ = GetValueForDebugger(stack_it, isolate);
+  stack_it++;
+
+  // Get the expression stack.
+  int stack_height = frame_it->height();
+  if (frame_it->kind() == TranslatedFrame::kFunction) {
+    // For full-code frames, we should not count the context.
+    // TODO(jarin): Clean up the indexing in translated frames.
+    stack_height--;
+  }
+  expression_stack_.resize(static_cast<size_t>(stack_height));
+  for (int i = 0; i < stack_height; i++) {
+    Handle<Object> expression = GetValueForDebugger(stack_it, isolate);
+    SetExpression(i, expression);
+    stack_it++;
+  }
+
+  // For interpreter frame, skip the accumulator.
+  if (parameter_frame->kind() == TranslatedFrame::kInterpretedFunction) {
+    stack_it++;
+  }
+  DCHECK(stack_it == frame_it->end());

[Benedikt Meurer, 2016/02/15 05:48:26]

Nit: Please use a CHECK here.

[Jarin, 2016/02/15 06:53:54]

Done.

 }
 
 
@@ -2929,6 +2857,10 @@ bool TranslatedValue::IsMaterializedObject() const {
   }
 }
 
+bool TranslatedValue::IsMaterializableByDebugger() const {
+  // At the moment, we only allow materialization of doubles.
+  return (kind() == kDouble);
+}
 
 int TranslatedValue::GetChildrenCount() const {
   if (kind() == kCapturedObject || kind() == kArgumentsObject) {