From 59c32b64817064ba9e3aa048a2c7bf5c9c291a27 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Thu, 19 Sep 2013 09:48:50 +0000 Subject: [PATCH] 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 --- src/objects.cc | 8 ++++-- test/mjsunit/allocation-site-info.js | 23 ++++++--------- test/mjsunit/array-literal-feedback.js | 51 ++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index f8897b0..b91134b 100644 --- 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(this), diff --git a/test/mjsunit/allocation-site-info.js b/test/mjsunit/allocation-site-info.js index 5f6817b..0f5870d 100644 --- 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 --- 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); + })(); } -- 2.7.4