Add new FrameArray type

src/isolate.cc

Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index 624f2a8d93be453c855f272ef982d08cb69d7cf8..17d5746b74ae2e82f0d678fc705632aea545285a 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -315,21 +315,7 @@ void Isolate::PushStackTraceAndDie(unsigned int magic, void* ptr1, void* ptr2,
   base::OS::Abort();
 }
 
-static Handle<FixedArray> MaybeGrow(Isolate* isolate,
-                                    Handle<FixedArray> elements,
-                                    int cur_position, int new_size) {
-  if (new_size > elements->length()) {
-    int new_capacity = JSObject::NewElementsCapacity(elements->length());
-    Handle<FixedArray> new_elements =
-        isolate->factory()->NewFixedArrayWithHoles(new_capacity);
-    for (int i = 0; i < cur_position; i++) {
-      new_elements->set(i, elements->get(i));
-    }
-    elements = new_elements;
-  }
-  DCHECK(new_size <= elements->length());
-  return elements;
-}
+namespace {
 
 class StackTraceHelper {
  public:
@@ -435,8 +421,6 @@ class StackTraceHelper {
   bool encountered_strict_function_;
 };
 
-namespace {
-
 // TODO(jgruber): Fix all cases in which frames give us a hole value (e.g. the
 // receiver in RegExp constructor frames.
 Handle<Object> TheHoleToUndefined(Isolate* isolate, Handle<Object> in) {
@@ -444,7 +428,8 @@ Handle<Object> TheHoleToUndefined(Isolate* isolate, Handle<Object> in) {
              ? Handle<Object>::cast(isolate->factory()->undefined_value())
              : in;
 }
-}
+
+}  // namespace
 
 Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
                                                 FrameSkipMode mode,
@@ -463,13 +448,10 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
   limit = Max(limit, 0);  // Ensure that limit is not negative.
 
   int initial_size = Min(limit, 10);
-  Handle<FixedArray> elements =
-      factory()->NewFixedArrayWithHoles(initial_size * 4 + 1);
+  Handle<FrameArray> elements = FrameArray::Allocate(this, initial_size);
 
   StackTraceHelper helper(this, mode, caller);
 
-  // First element is reserved to store the number of sloppy frames.
-  int cursor = 1;
   int frames_seen = 0;

[Toon Verwaest, 2016/08/23 13:14:08]

Perhaps we should just encapsulate frames_seen as an additional first integer in
the fixed array, basically use fixed-array-length as "capacity" and have an
additional "length" field?

[jgruber, 2016/08/23 15:32:13]

Done.

   for (StackFrameIterator iter(this); !iter.done() && frames_seen < limit;
        iter.Advance()) {
@@ -502,13 +484,11 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
               recv = handle(heap()->call_site_constructor_symbol(), this);
             }
           }
-          Handle<Smi> offset(Smi::FromInt(frames[i].code_offset()), this);
+          const int offset = frames[i].code_offset();
 
-          elements = MaybeGrow(this, elements, cursor, cursor + 4);
-          elements->set(cursor++, *TheHoleToUndefined(this, recv));
-          elements->set(cursor++, *fun);
-          elements->set(cursor++, *abstract_code);
-          elements->set(cursor++, *offset);
+          elements = FrameArray::AppendJSFrame(elements, frames_seen,
+                                               TheHoleToUndefined(this, recv),
+                                               fun, abstract_code, offset);
           frames_seen++;
         }
       } break;
@@ -534,26 +514,30 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
           recv = handle(exit_frame->receiver(), this);
         }
 
-        elements = MaybeGrow(this, elements, cursor, cursor + 4);
-        elements->set(cursor++, *recv);
-        elements->set(cursor++, *fun);
-        elements->set(cursor++, *code);
-        elements->set(cursor++, Smi::FromInt(offset));
+        elements =
+            FrameArray::AppendJSFrame(elements, frames_seen, recv, fun,
+                                      Handle<AbstractCode>::cast(code), offset);
         frames_seen++;
       } break;
 
       case StackFrame::WASM: {
         WasmFrame* wasm_frame = WasmFrame::cast(frame);
+        Handle<Object> wasm_object = handle(wasm_frame->wasm_obj(), this);
+        const int wasm_function_index = wasm_frame->function_index();
         Code* code = wasm_frame->unchecked_code();
         Handle<AbstractCode> abstract_code =
             Handle<AbstractCode>(AbstractCode::cast(code), this);
-        int offset =
+        const int offset =
             static_cast<int>(wasm_frame->pc() - code->instruction_start());
-        elements = MaybeGrow(this, elements, cursor, cursor + 4);
-        elements->set(cursor++, wasm_frame->wasm_obj());
-        elements->set(cursor++, Smi::FromInt(wasm_frame->function_index()));
-        elements->set(cursor++, *abstract_code);
-        elements->set(cursor++, Smi::FromInt(offset));
+
+        // TODO(wasm): The wasm object returned by the WasmFrame should always
+        //             be a wasm object.
+        DCHECK(wasm::IsWasmObject(*wasm_object) ||
+               wasm_object->IsUndefined(this));
+
+        elements = FrameArray::AppendWasmFrame(elements, frames_seen,
+                                               wasm_object, wasm_function_index,
+                                               abstract_code, offset);
         frames_seen++;
       } break;
 
@@ -562,9 +546,13 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
     }
   }
   elements->set(0, Smi::FromInt(helper.sloppy_frames()));
-  elements->Shrink(cursor);
+
+  const int elements_length = FrameArray::LengthFor(frames_seen);

[Toon Verwaest, 2016/08/23 13:14:08]

What about elements->Shrink(frames_seen) to avoid leaking here that it's a fixed
array? We don't need to pass in frames-seen even if we store it in the fixed
array. Slightly less efficient but it probably doesn't matter.

[jgruber, 2016/08/23 15:32:13]

Done.

+  elements->Shrink(elements_length);
+
   Handle<JSArray> result = factory()->NewJSArrayWithElements(elements);
-  result->set_length(Smi::FromInt(cursor));
+  result->set_length(Smi::FromInt(elements_length));
+
   // TODO(yangguo): Queue this structured stack trace for preprocessing on GC.
   return result;
 }
@@ -773,19 +761,6 @@ class CaptureStackTraceHelper {
   Handle<String> constructor_key_;
 };
 
-
-int PositionFromStackTrace(Handle<FixedArray> elements, int index) {
-  DisallowHeapAllocation no_gc;
-  Object* maybe_code = elements->get(index + 2);
-  if (maybe_code->IsSmi()) {
-    return Smi::cast(maybe_code)->value();
-  } else {
-    AbstractCode* abstract_code = AbstractCode::cast(maybe_code);
-    int code_offset = Smi::cast(elements->get(index + 3))->value();
-    return abstract_code->SourcePosition(code_offset);
-  }
-}
-
 Handle<JSArray> Isolate::CaptureCurrentStackTrace(
     int frame_limit, StackTrace::StackTraceOptions options) {
   DisallowJavascriptExecution no_js(this);
@@ -1531,22 +1506,25 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
   if (!property->IsJSArray()) return false;
   Handle<JSArray> simple_stack_trace = Handle<JSArray>::cast(property);
 
-  Handle<FixedArray> elements(FixedArray::cast(simple_stack_trace->elements()));
-  int elements_limit = Smi::cast(simple_stack_trace->length())->value();
+  Handle<FrameArray> elements(FrameArray::cast(simple_stack_trace->elements()));
 
-  for (int i = 1; i < elements_limit; i += 4) {
-    Handle<Object> fun_obj = handle(elements->get(i + 1), this);
-    if (fun_obj->IsSmi()) {
+  const int frame_count = elements->FrameCount();
+  for (int i = 0; i < frame_count; i++) {
+    if (elements->IsWasmFrame(i)) {
       // TODO(clemensh): handle wasm frames
       return false;
     }
-    Handle<JSFunction> fun = Handle<JSFunction>::cast(fun_obj);
+
+    Handle<JSFunction> fun = handle(elements->Function(i), this);
     if (!fun->shared()->IsSubjectToDebugging()) continue;
 
     Object* script = fun->shared()->script();
     if (script->IsScript() &&
         !(Script::cast(script)->source()->IsUndefined(this))) {
-      int pos = PositionFromStackTrace(elements, i);
+      AbstractCode* abstract_code = elements->Code(i);
+      const int code_offset = elements->Offset(i)->value();
+      const int pos = abstract_code->SourcePosition(code_offset);
+
       Handle<Script> casted_script(Script::cast(script));
       *target = MessageLocation(casted_script, pos, pos + 1);
       return true;