Inline array constructor.

src/runtime.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 14646 matching lines @@
   return isolate->heap()->ToBoolean(access_allowed);
 }
 
 
 static MaybeObject* ArrayConstructorCommon(Isolate* isolate,
                                            Handle<JSFunction> constructor,
                                            Handle<Object> type_info,
                                            Arguments* caller_args) {
   bool holey = false;
   bool can_use_type_feedback = true;
+  Handle<AllocationSite> site;

[Toon Verwaest, 2013/11/06 16:42:44]

Can we just declare it below, right above the if where it's initialized?

[mvstanton, 2013/11/07 16:34:05]

Done.

   if (caller_args->length() == 1) {
     Object* argument_one = (*caller_args)[0];
     if (argument_one->IsSmi()) {
       int value = Smi::cast(argument_one)->value();
       if (value < 0 || value >= JSObject::kInitialMaxFastElementArray) {
         // the array is a dictionary in this case.
         can_use_type_feedback = false;
       } else if (value != 0) {
         holey = true;
       }
     } else {
       // Non-smi length argument produces a dictionary
       can_use_type_feedback = false;
     }
   }
 
+  if (!type_info.is_null() &&
+      *type_info != isolate->heap()->undefined_value() &&
+      Cell::cast(*type_info)->value()->IsAllocationSite()) {
+    site = Handle<AllocationSite>(
+        AllocationSite::cast(Cell::cast(*type_info)->value()), isolate);
+  }
+
+  if (!site.is_null() && !can_use_type_feedback) {

[Toon Verwaest, 2013/11/06 16:42:44]

I find all these !site.is_null() a bit hard to read. Can we reorder the branches
and/or extract code into separate functions?

[mvstanton, 2013/11/07 16:34:05]

I pulled out turning the type_info into an allocationsite to the caller, and
consolidated the two points in the function where I set
site->SetDoNotInlineCall().

I think it looks better even though !site.is_null() still exists.

+    // Mark the allocation site as un-inlinable.
+    site->SetDoNotInlineCall();
+  }
+
   JSArray* array;
   MaybeObject* maybe_array;
-  if (!type_info.is_null() &&
-      *type_info != isolate->heap()->undefined_value() &&
-      Cell::cast(*type_info)->value()->IsAllocationSite() &&
-      can_use_type_feedback) {
-    Handle<Cell> cell = Handle<Cell>::cast(type_info);
-    Handle<AllocationSite> site = Handle<AllocationSite>(
-        AllocationSite::cast(cell->value()), isolate);
+  if (!site.is_null() && can_use_type_feedback) {
     ASSERT(!site->SitePointsToLiteral());
     ElementsKind to_kind = site->GetElementsKind();
     if (holey && !IsFastHoleyElementsKind(to_kind)) {
       to_kind = GetHoleyElementsKind(to_kind);
       // Update the allocation site info to reflect the advice alteration.
       site->SetElementsKind(to_kind);
     }
 
     maybe_array = isolate->heap()->AllocateJSObjectWithAllocationSite(
         *constructor, site);
     if (!maybe_array->To(&array)) return maybe_array;
   } else {
     maybe_array = isolate->heap()->AllocateJSObject(*constructor);
     if (!maybe_array->To(&array)) return maybe_array;
     // We might need to transition to holey
     ElementsKind kind = constructor->initial_map()->elements_kind();
     if (holey && !IsFastHoleyElementsKind(kind)) {
       kind = GetHoleyElementsKind(kind);
       maybe_array = array->TransitionElementsKind(kind);
       if (maybe_array->IsFailure()) return maybe_array;
     }
   }
 
   maybe_array = isolate->heap()->AllocateJSArrayStorage(array, 0, 0,
       DONT_INITIALIZE_ARRAY_ELEMENTS);
   if (maybe_array->IsFailure()) return maybe_array;
+  ElementsKind old_kind = array->GetElementsKind();
   maybe_array = ArrayConstructInitializeElements(array, caller_args);
   if (maybe_array->IsFailure()) return maybe_array;
+  if (old_kind != array->GetElementsKind() && !site.is_null()) {
+    // The arguments passed in caused a transition. This kind of complexity
+    // can't be dealt with in the inlined hydrogen array constructor case.
+    // We must mark the allocationsite as un-inlinable.
+    site->SetDoNotInlineCall();
+  }
   return array;
 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_ArrayConstructor) {
   HandleScope scope(isolate);
   // If we get 2 arguments then they are the stub parameters (constructor, type
   // info).  If we get 3, then the first one is a pointer to the arguments
   // passed by the caller.
   Arguments empty_args(0, NULL);
@@ skipping 108 matching lines @@
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
                                        "Runtime::PerformGC");
   }
 }
 
 
 } }  // namespace v8::internal