Supporting AllocationSiteInfo for Nested arrays

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index f808d1060cf4a4b7c00b5a6b47685742bcd76e6b..c630bd59aafa12efc57dac241d9d1a2a1e38b5af 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -140,14 +140,24 @@ namespace internal {
       static_cast<LanguageMode>(args.smi_at(index));
 
 
-MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
-                                                   JSObject* boilerplate) {
+
+MUST_USE_RESULT MaybeObject* Runtime::DeepCopyBoilerplate(
+    Isolate* isolate,
+    JSObject* boilerplate,
+    AllocationSiteMode allocation_site_mode) {
   StackLimitCheck check(isolate);
   if (check.HasOverflowed()) return isolate->StackOverflow();
 
   Heap* heap = isolate->heap();
   Object* result;
-  { MaybeObject* maybe_result = heap->CopyJSObject(boilerplate);
+  AllocationSiteMode current_object_mode = DONT_TRACK_ALLOCATION_SITE;
+  if (allocation_site_mode == TRACK_ALLOCATION_SITE &&
+      boilerplate->ShouldTrackAllocationInfo()) {
+    current_object_mode = TRACK_ALLOCATION_SITE;
+  }
+
+  { MaybeObject* maybe_result = heap->CopyJSObject(boilerplate,
+                                                   current_object_mode);
     if (!maybe_result->ToObject(&result)) return maybe_result;
   }
   JSObject* copy = JSObject::cast(result);
@@ -159,7 +169,8 @@ MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
       Object* value = properties->get(i);
       if (value->IsJSObject()) {
         JSObject* js_object = JSObject::cast(value);
-        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object);
+        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object,
+                                                          allocation_site_mode);
           if (!maybe_result->ToObject(&result)) return maybe_result;
         }
         properties->set(i, result);
@@ -170,7 +181,8 @@ MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
       Object* value = copy->InObjectPropertyAt(i);
       if (value->IsJSObject()) {
         JSObject* js_object = JSObject::cast(value);
-        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object);
+        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object,
+                                                          allocation_site_mode);
           if (!maybe_result->ToObject(&result)) return maybe_result;
         }
         copy->InObjectPropertyAtPut(i, result);
@@ -196,7 +208,8 @@ MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
           copy->GetProperty(key_string, &attributes)->ToObjectUnchecked();
       if (value->IsJSObject()) {
         JSObject* js_object = JSObject::cast(value);
-        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object);
+        { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate, js_object,
+                                                          allocation_site_mode);
           if (!maybe_result->ToObject(&result)) return maybe_result;
         }
         { MaybeObject* maybe_result =
@@ -233,7 +246,8 @@ MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
           if (value->IsJSObject()) {
             JSObject* js_object = JSObject::cast(value);
             { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate,
-                                                              js_object);
+                  js_object,
+                  allocation_site_mode);
               if (!maybe_result->ToObject(&result)) return maybe_result;
             }
             elements->set(i, result);
@@ -252,7 +266,8 @@ MUST_USE_RESULT static MaybeObject* DeepCopyBoilerplate(Isolate* isolate,
           if (value->IsJSObject()) {
             JSObject* js_object = JSObject::cast(value);
             { MaybeObject* maybe_result = DeepCopyBoilerplate(isolate,
-                                                              js_object);
+                  js_object,
+                  allocation_site_mode);
               if (!maybe_result->ToObject(&result)) return maybe_result;
             }
             element_dictionary->ValueAtPut(i, result);
@@ -586,6 +601,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateObjectLiteral) {
   CONVERT_SMI_ARG_CHECKED(flags, 3);
   bool should_have_fast_elements = (flags & ObjectLiteral::kFastElements) != 0;
   bool has_function_literal = (flags & ObjectLiteral::kHasFunction) != 0;
+  bool allocation_sites_allowed =
+      (flags & ObjectLiteral::kAllocationSiteInfoAllowed) != 0;

[danno, 2013/02/11 15:05:21]

The flag tells object literal instantiation to create allocation sites wherever
possible, so does it make sense to rename this flag to
"kCreateAllocationSiteInfos", since they are not allowed, they are desired, and
requested to be created if the boilerplate object supports them. If you agree,
rename the boolean variable appropriately.

[mvstanton, 2013/02/15 07:36:43]

"they are not allowed, they are desired." Beautiful :).

+

[danno, 2013/02/11 15:05:21]

Also, perhaps you should fashion the calculation of this boolean value to
include "&& FLAG_track_allocation_sites" as you do later in this file for
consistency. Even better, to reduce the number of sites that need to check
"FLAG_track_allocation_sites", just make sure never to set the
"kAllocationSiteInfoAllowed" flag if FLAG_track_allocation_sites is false, then
you don't have to check the flag at the the sites in this file.

[mvstanton, 2013/02/15 07:36:43]

right on, done.

+  AllocationSiteMode allocation_site_mode = DONT_TRACK_ALLOCATION_SITE;
+  if (FLAG_track_allocation_sites && allocation_sites_allowed) {
+    allocation_site_mode = TRACK_ALLOCATION_SITE;
+  }
 
   // Check if boilerplate exists. If not, create it first.
   Handle<Object> boilerplate(literals->get(literals_index), isolate);
@@ -599,7 +621,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateObjectLiteral) {
     // Update the functions literal and return the boilerplate.
     literals->set(literals_index, *boilerplate);
   }
-  return DeepCopyBoilerplate(isolate, JSObject::cast(*boilerplate));
+
+  return Runtime::DeepCopyBoilerplate(isolate,
+                                      JSObject::cast(*boilerplate),
+                                      allocation_site_mode);
 }
 
 
@@ -612,6 +637,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateObjectLiteralShallow) {
   CONVERT_SMI_ARG_CHECKED(flags, 3);
   bool should_have_fast_elements = (flags & ObjectLiteral::kFastElements) != 0;
   bool has_function_literal = (flags & ObjectLiteral::kHasFunction) != 0;
+  bool allocation_site_info_allowed =
+      (flags & ObjectLiteral::kAllocationSiteInfoAllowed) != 0
+      && FLAG_track_allocation_sites;
 
   // Check if boilerplate exists. If not, create it first.
   Handle<Object> boilerplate(literals->get(literals_index), isolate);
@@ -625,16 +653,29 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateObjectLiteralShallow) {
     // Update the functions literal and return the boilerplate.
     literals->set(literals_index, *boilerplate);
   }
-  return isolate->heap()->CopyJSObject(JSObject::cast(*boilerplate));
+
+  AllocationSiteMode mode = allocation_site_info_allowed
+      ? TRACK_ALLOCATION_SITE
+      : DONT_TRACK_ALLOCATION_SITE;
+  return isolate->heap()->CopyJSObject(JSObject::cast(*boilerplate),
+                                       mode);

[danno, 2013/02/11 15:05:21]

nit: mode fits on previous line.

[mvstanton, 2013/02/15 07:36:43]

Changes based on other recommendations affected this line differently.

 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteral) {
   HandleScope scope(isolate);
-  ASSERT(args.length() == 3);
+  ASSERT(args.length() == 4);
   CONVERT_ARG_HANDLE_CHECKED(FixedArray, literals, 0);
   CONVERT_SMI_ARG_CHECKED(literals_index, 1);
   CONVERT_ARG_HANDLE_CHECKED(FixedArray, elements, 2);
+  CONVERT_SMI_ARG_CHECKED(flags, 3);
+  bool allocation_sites_allowed =
+      (flags & ArrayLiteral::kAllocationSiteInfoAllowed) != 0;
+
+  AllocationSiteMode allocation_site_mode = DONT_TRACK_ALLOCATION_SITE;
+  if (FLAG_track_allocation_sites && allocation_sites_allowed) {
+    allocation_site_mode = TRACK_ALLOCATION_SITE;
+  }
 
   // Check if boilerplate exists. If not, create it first.
   Handle<Object> boilerplate(literals->get(literals_index), isolate);
@@ -646,16 +687,22 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteral) {
     // Update the functions literal and return the boilerplate.
     literals->set(literals_index, *boilerplate);
   }
-  return DeepCopyBoilerplate(isolate, JSObject::cast(*boilerplate));
+
+  return Runtime::DeepCopyBoilerplate(isolate,
+                                      JSObject::cast(*boilerplate),
+                                      allocation_site_mode);
 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteralShallow) {
   HandleScope scope(isolate);
-  ASSERT(args.length() == 3);
+  ASSERT(args.length() == 4);
   CONVERT_ARG_HANDLE_CHECKED(FixedArray, literals, 0);
   CONVERT_SMI_ARG_CHECKED(literals_index, 1);
   CONVERT_ARG_HANDLE_CHECKED(FixedArray, elements, 2);
+  CONVERT_SMI_ARG_CHECKED(flags, 3);
+  bool allocation_site_info_allowed = FLAG_track_allocation_sites &&
+      ((flags & ArrayLiteral::kAllocationSiteInfoAllowed) != 0);
 
   // Check if boilerplate exists. If not, create it first.
   Handle<Object> boilerplate(literals->get(literals_index), isolate);
@@ -673,8 +720,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateArrayLiteralShallow) {
   }
 
   JSObject* boilerplate_object = JSObject::cast(*boilerplate);
-  AllocationSiteMode mode = AllocationSiteInfo::GetMode(
-      boilerplate_object->GetElementsKind());
+  AllocationSiteMode mode = allocation_site_info_allowed &&
+      boilerplate_object->ShouldTrackAllocationInfo()

[danno, 2013/02/11 15:05:21]

nite: Just for clarity and easy, separate the boolean calculation into a
temporary variable then used in the ternary operator.

[mvstanton, 2013/02/15 07:36:43]

Done.

+      ? TRACK_ALLOCATION_SITE
+      : DONT_TRACK_ALLOCATION_SITE;
   return isolate->heap()->CopyJSObject(boilerplate_object, mode);
 }