Allocation Info Tracking, continued.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index a65a3174fb5a4d2f6607a54a5a0e15bcf092daa3..009cd5453d4825ba1a0cc422b1a073fdb1cbfa42 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1,4 +1,4 @@
-// Copyright 2012 the V8 project authors. All rights reserved.
+// Copyright 2013 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:
@@ -7512,6 +7512,21 @@ AllocationSiteInfo* AllocationSiteInfo::FindForJSObject(JSObject* object) {
 }
 
 
+bool AllocationSiteInfo::GetElementsKindPayload(ElementsKind* kind) {
+  ASSERT(kind != NULL);
+  if (payload()->IsJSGlobalPropertyCell()) {
+    JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(payload());
+    Object* cell_contents = cell->value();
+    if (cell_contents->IsSmi()) {
+      *kind = static_cast<ElementsKind>(
+          Smi::cast(cell_contents)->value());
+      return true;
+    }
+  }
+  return false;
+}
+
+
 // Heuristic: We only need to create allocation site info if the boilerplate
 // elements kind is the initial elements kind.
 AllocationSiteMode AllocationSiteInfo::GetMode(
@@ -9336,11 +9351,17 @@ MaybeObject* JSArray::Initialize(int capacity) {
   Heap* heap = GetHeap();
   ASSERT(capacity >= 0);
   set_length(Smi::FromInt(0));
-  FixedArray* new_elements;
+  FixedArrayBase* new_elements;
   if (capacity == 0) {
     new_elements = heap->empty_fixed_array();
   } else {
-    MaybeObject* maybe_obj = heap->AllocateFixedArrayWithHoles(capacity);
+    ElementsKind elements_kind = GetElementsKind();
+    MaybeObject* maybe_obj;
+    if (IsFastDoubleElementsKind(elements_kind)) {
+      maybe_obj = heap->AllocateFixedDoubleArrayWithHoles(capacity);
+    } else {
+      maybe_obj = heap->AllocateFixedArrayWithHoles(capacity);
+    }

[Toon Verwaest, 2013/02/13 15:14:51]

Seems like this is overlapping with AllocateJSArrayAndStorage.

[mvstanton, 2013/02/19 11:04:08]

Done.

     if (!maybe_obj->To(&new_elements)) return maybe_obj;
   }
   set_elements(new_elements);
@@ -10530,6 +10551,12 @@ Handle<Object> JSObject::TransitionElementsKind(Handle<JSObject> object,
 }
 
 
+// TODO(mvstanton): rename this method to reflect what it actually does.
+// If a boilerplate object is discovered, then it will transition it.
+// If instead there is a elements kind, then update it as long as the
+// to_kind variable is more general than what we find, but don't
+// ever take the double->fastobject transition (that represents poisoning),
+// just ignore that case.
 MaybeObject* JSObject::PossiblyTransitionArrayBoilerplate(
     ElementsKind to_kind) {
   MaybeObject* ret = NULL;

[Toon Verwaest, 2013/02/13 15:14:51]

Seems like you don't need this variable. Below you can just return NULL; or
return payload->TransitionElementsKind(to_kind).

[mvstanton, 2013/02/19 11:04:08]

Done. And I addressed my TODO above, renaming the method.

@@ -10542,22 +10569,43 @@ MaybeObject* JSObject::PossiblyTransitionArrayBoilerplate(
     return ret;
   }
 
-  ASSERT(info->payload()->IsJSArray());
-  JSArray* payload = JSArray::cast(info->payload());
-  ElementsKind kind = payload->GetElementsKind();
-  if (IsMoreGeneralElementsKindTransition(kind, to_kind)) {
-    // If the array is huge, it's not likely to be defined in a local
-    // function, so we shouldn't make new instances of it very often.
-    uint32_t length = 0;
-    CHECK(payload->length()->ToArrayIndex(&length));
-    if (length <= 8 * 1024) {
-      ret = payload->TransitionElementsKind(to_kind);
-      if (FLAG_trace_track_allocation_sites) {
-        PrintF(
-            "AllocationSiteInfo: JSArray %p boilerplate updated %s->%s\n",
-            reinterpret_cast<void*>(this),
-            ElementsKindToString(kind),
-            ElementsKindToString(to_kind));
+  if (info->payload()->IsJSArray()) {
+    JSArray* payload = JSArray::cast(info->payload());
+    ElementsKind kind = payload->GetElementsKind();
+    if (IsMoreGeneralElementsKindTransition(kind, to_kind)) {
+      // If the array is huge, it's not likely to be defined in a local
+      // function, so we shouldn't make new instances of it very often.
+      uint32_t length = 0;
+      CHECK(payload->length()->ToArrayIndex(&length));
+      if (length <= 8*1024) {

[Toon Verwaest, 2013/02/13 15:14:51]

Define a constant.

[mvstanton, 2013/02/19 11:04:08]

Done.

+        ret = payload->TransitionElementsKind(to_kind);
+        if (FLAG_trace_track_allocation_sites) {
+          PrintF(
+              "AllocationSiteInfo: JSArray %p boilerplate updated %s->%s\n",
+              reinterpret_cast<void*>(this),
+              ElementsKindToString(kind),
+              ElementsKindToString(to_kind));
+        }
+      }
+    }
+  } else if (info->payload()->IsJSGlobalPropertyCell()) {
+    JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(info->payload());
+    Object* cell_contents = cell->value();
+    if (cell_contents->IsSmi()) {
+      ElementsKind kind = static_cast<ElementsKind>(
+          Smi::cast(cell_contents)->value());
+      // Specifically exclude DOUBLE(HOLEY) -> FAST(HOLEY)
+      bool double_to_fast = IsFastDoubleElementsKind(kind) &&
+          IsFastObjectElementsKind(to_kind);

[Toon Verwaest, 2013/02/13 15:14:51]

This heuristic seems very buried away. Can we make it more top-level using some
function? Don't you already have that somewhere?

[mvstanton, 2013/02/19 11:04:08]

Yep, I just didn't revisit this code in a while, thx!

[mvstanton, 2013/02/19 11:04:08]

Done.

+      if (IsMoreGeneralElementsKindTransition(kind, to_kind) &&
+          !double_to_fast) {
+        if (FLAG_trace_track_allocation_sites) {
+          PrintF("AllocationSiteInfo: JSArray %p info updated %s->%s\n",
+                 reinterpret_cast<void*>(this),
+                 ElementsKindToString(kind),
+                 ElementsKindToString(to_kind));
+        }
+        cell->set_value(Smi::FromInt(to_kind));
       }
     }
   }