Transitions from DOUBLE to FAST were not checking for allocation site info.

This creates a confusing result. It's better to let allocation sites
transition to their end state than artificially stop tracking at the
double/fast boundary.

BUG=
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/22868004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16820 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.cc b/src/objects.cc
index f8897b0..b91134b 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -10905,6 +10905,10 @@ MaybeObject* JSObject::SetFastElementsCapacityAndLength(
     }
     ValidateElements();
     set_map_and_elements(new_map, new_elements);
+
+    // Transition through the allocation site as well if present.
+    maybe_obj = UpdateAllocationSite(new_elements_kind);
+    if (maybe_obj->IsFailure()) return maybe_obj;
   } else {
     FixedArray* parameter_map = FixedArray::cast(old_elements);
     parameter_map->set(1, new_elements);
@@ -12442,7 +12446,7 @@ MaybeObject* JSObject::UpdateAllocationSite(ElementsKind to_kind) {
     if (IsHoleyElementsKind(kind)) {
       to_kind = GetHoleyElementsKind(to_kind);
     }
-    if (AllocationSite::GetMode(kind, to_kind) == TRACK_ALLOCATION_SITE) {
+    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;
@@ -12464,7 +12468,7 @@ MaybeObject* JSObject::UpdateAllocationSite(ElementsKind to_kind) {
     if (IsHoleyElementsKind(kind)) {
       to_kind = GetHoleyElementsKind(to_kind);
     }
-    if (AllocationSite::GetMode(kind, to_kind) == TRACK_ALLOCATION_SITE) {
+    if (IsMoreGeneralElementsKindTransition(kind, to_kind)) {
       if (FLAG_trace_track_allocation_sites) {
         PrintF("AllocationSite: JSArray %p site updated %s->%s\n",
                reinterpret_cast<void*>(this),

diff --git a/test/mjsunit/allocation-site-info.js b/test/mjsunit/allocation-site-info.js
index 5f6817b..0f5870d 100644 (file)
--- a/test/mjsunit/allocation-site-info.js
+++ b/test/mjsunit/allocation-site-info.js
@@ -150,17 +150,14 @@ if (support_smi_only_arrays) {
   // Verify that we will not pretransition the double->fast path.
   obj = fastliteralcase(get_standard_literal(), "elliot");
   assertKind(elements_kind.fast, obj);
-  // This fails until we turn off optimistic transitions to the
-  // most general elements kind seen on keyed stores. It's a goal
-  // to turn it off, but for now we need it.
-  // obj = fastliteralcase(3);
-  // assertKind(elements_kind.fast_double, obj);
+  obj = fastliteralcase(get_standard_literal(), 3);
+  assertKind(elements_kind.fast, obj);
 
   // Make sure this works in crankshafted code too.
   %OptimizeFunctionOnNextCall(get_standard_literal);
   get_standard_literal();
   obj = get_standard_literal();
-  assertKind(elements_kind.fast_double, obj);
+  assertKind(elements_kind.fast, obj);
 
   function fastliteralcase_smifast(value) {
     var literal = [1, 2, 3, 4];
@@ -231,16 +228,14 @@ if (support_smi_only_arrays) {
   obj = newarraycase_length_smidouble(2);
   assertKind(elements_kind.fast_double, obj);
 
-  // Try to continue the transition to fast object, but
-  // we will not pretransition from double->fast, because
-  // it may hurt performance ("poisoning").
+  // Try to continue the transition to fast object. This won't work for
+  // constructed arrays because constructor dispatch is done on the
+  // elements kind, and a DOUBLE array constructor won't create an allocation
+  // memento.
   obj = newarraycase_length_smidouble("coates");
   assertKind(elements_kind.fast, obj);
-  obj = newarraycase_length_smidouble(2.5);
-  // However, because of optimistic transitions, we will
-  // transition to the most general kind of elements kind found,
-  // therefore I can't count on this assert yet.
-  // assertKind(elements_kind.fast_double, obj);
+  obj = newarraycase_length_smidouble(2);
+  assertKind(elements_kind.fast_double, obj);
 
   function newarraycase_length_smiobj(value) {
     var a = new Array(3);

diff --git a/test/mjsunit/array-literal-feedback.js b/test/mjsunit/array-literal-feedback.js
index 3378394..d2245c6 100644 (file)
--- a/test/mjsunit/array-literal-feedback.js
+++ b/test/mjsunit/array-literal-feedback.js
@@ -44,6 +44,42 @@ if (support_smi_only_arrays) {
   print("Tests do NOT include smi-only arrays.");
 }
 
+var elements_kind = {
+  fast_smi_only            :  'fast smi only elements',
+  fast                     :  'fast elements',
+  fast_double              :  'fast double elements',
+  dictionary               :  'dictionary elements',
+  external_byte            :  'external byte elements',
+  external_unsigned_byte   :  'external unsigned byte elements',
+  external_short           :  'external short elements',
+  external_unsigned_short  :  'external unsigned short elements',
+  external_int             :  'external int elements',
+  external_unsigned_int    :  'external unsigned int elements',
+  external_float           :  'external float elements',
+  external_double          :  'external double elements',
+  external_pixel           :  'external pixel elements'
+}
+
+function getKind(obj) {
+  if (%HasFastSmiElements(obj)) return elements_kind.fast_smi_only;
+  if (%HasFastObjectElements(obj)) return elements_kind.fast;
+  if (%HasFastDoubleElements(obj)) return elements_kind.fast_double;
+  if (%HasDictionaryElements(obj)) return elements_kind.dictionary;
+}
+
+function isHoley(obj) {
+  if (%HasFastHoleyElements(obj)) return true;
+  return false;
+}
+
+function assertKind(expected, obj, name_opt) {
+  if (!support_smi_only_arrays &&
+      expected == elements_kind.fast_smi_only) {
+    expected = elements_kind.fast;
+  }
+  assertEquals(expected, getKind(obj), name_opt);
+}
+
 if (support_smi_only_arrays) {
 
   function get_literal(x) {
@@ -72,4 +108,19 @@ if (support_smi_only_arrays) {
   b = get_literal(3);
   assertTrue(%HasFastDoubleElements(b));
   assertOptimized(get_literal);
+
+
+  // Test: make sure allocation site information is updated through a
+  // transition from SMI->DOUBLE->FAST
+  (function() {
+    function bar(a, b, c) {
+      return [a, b, c];
+    }
+
+    a = bar(1, 2, 3);
+    a[0] = 3.5;
+    a[1] = 'hi';
+    b = bar(1, 2, 3);
+    assertKind(elements_kind.fast, b);
+  })();
 }