AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

ArrayConcat regression recover after r20312 (appeared on dromaeo benchmarks).

Browse Source 
BUG=chromium:358561
LOG=N
R=yangguo@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24269 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
ishell@chromium.org 2014-09-29 08:22:24 +00:00
parent 7e44408fc6
commit 40a9b4363e
3 changed files with 77 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/builtins.cc
View File

@ -987,12 +987,12 @@ BUILTIN(ArrayConcat) {
Handle<FixedArrayBase> storage(result_array->elements(), isolate); Handle<FixedArrayBase> storage(result_array->elements(), isolate);
ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind); ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind);
for (int i = 0; i < n_arguments; i++) { for (int i = 0; i < n_arguments; i++) {
// TODO(ishell): It is crucial to keep |array| as a raw pointer to avoid // It is crucial to keep |array| in a raw pointer form to avoid performance
// performance degradation. Revisit this later. // degradation.
JSArray* array = JSArray::cast(args[i]); JSArray* array = JSArray::cast(args[i]);
int len = Smi::cast(array->length())->value(); int len = Smi::cast(array->length())->value();
ElementsKind from_kind = array->GetElementsKind();
if (len > 0) { if (len > 0) {
ElementsKind from_kind = array->GetElementsKind();
accessor->CopyElements(array, 0, from_kind, storage, j, len); accessor->CopyElements(array, 0, from_kind, storage, j, len);
j += len; j += len;
} }

126
src/elements.cc
View File

@ -247,15 +247,18 @@ static void CopyDictionaryToObjectElements(
} }
static void CopyDoubleToObjectElements(Handle<FixedArrayBase> from_base, // NOTE: this method violates the handlified function signature convention:
// raw pointer parameters in the function that allocates.
// See ElementsAccessorBase::CopyElements() for details.
static void CopyDoubleToObjectElements(FixedArrayBase* from_base,
uint32_t from_start, uint32_t from_start,
Handle<FixedArrayBase> to_base, FixedArrayBase* to_base,
ElementsKind to_kind, ElementsKind to_kind, uint32_t to_start,
uint32_t to_start,
int raw_copy_size) { int raw_copy_size) {
DCHECK(IsFastSmiOrObjectElementsKind(to_kind)); DCHECK(IsFastSmiOrObjectElementsKind(to_kind));
int copy_size = raw_copy_size; int copy_size = raw_copy_size;
if (raw_copy_size < 0) { if (raw_copy_size < 0) {
DisallowHeapAllocation no_allocation;
DCHECK(raw_copy_size == ElementsAccessor::kCopyToEnd || DCHECK(raw_copy_size == ElementsAccessor::kCopyToEnd ||
raw_copy_size == ElementsAccessor::kCopyToEndAndInitializeToHole); raw_copy_size == ElementsAccessor::kCopyToEndAndInitializeToHole);
copy_size = Min(from_base->length() - from_start, copy_size = Min(from_base->length() - from_start,
@ -268,7 +271,7 @@ static void CopyDoubleToObjectElements(Handle<FixedArrayBase> from_base,
int length = to_base->length() - start; int length = to_base->length() - start;
if (length > 0) { if (length > 0) {
Heap* heap = from_base->GetHeap(); Heap* heap = from_base->GetHeap();
MemsetPointer(FixedArray::cast(*to_base)->data_start() + start, MemsetPointer(FixedArray::cast(to_base)->data_start() + start,
heap->the_hole_value(), length); heap->the_hole_value(), length);
} }
} }
@ -276,9 +279,12 @@ static void CopyDoubleToObjectElements(Handle<FixedArrayBase> from_base,
DCHECK((copy_size + static_cast<int>(to_start)) <= to_base->length() && DCHECK((copy_size + static_cast<int>(to_start)) <= to_base->length() &&
(copy_size + static_cast<int>(from_start)) <= from_base->length()); (copy_size + static_cast<int>(from_start)) <= from_base->length());
if (copy_size == 0) return; if (copy_size == 0) return;
// From here on, the code below could actually allocate. Therefore the raw
// values are wrapped into handles.
Isolate* isolate = from_base->GetIsolate(); Isolate* isolate = from_base->GetIsolate();
Handle<FixedDoubleArray> from = Handle<FixedDoubleArray>::cast(from_base); Handle<FixedDoubleArray> from(FixedDoubleArray::cast(from_base), isolate);
Handle<FixedArray> to = Handle<FixedArray>::cast(to_base); Handle<FixedArray> to(FixedArray::cast(to_base), isolate);
for (int i = 0; i < copy_size; ++i) { for (int i = 0; i < copy_size; ++i) {
HandleScope scope(isolate); HandleScope scope(isolate);
if (IsFastSmiElementsKind(to_kind)) { if (IsFastSmiElementsKind(to_kind)) {
@ -702,12 +708,9 @@ class ElementsAccessorBase : public ElementsAccessor {
uint32_t key, uint32_t key,
JSReceiver::DeleteMode mode) OVERRIDE = 0; JSReceiver::DeleteMode mode) OVERRIDE = 0;
static void CopyElementsImpl(Handle<FixedArrayBase> from, static void CopyElementsImpl(FixedArrayBase* from, uint32_t from_start,
uint32_t from_start, FixedArrayBase* to, ElementsKind from_kind,
Handle<FixedArrayBase> to, uint32_t to_start, int packed_size,
ElementsKind from_kind,
uint32_t to_start,
int packed_size,
int copy_size) { int copy_size) {
UNREACHABLE(); UNREACHABLE();
} }
@ -720,9 +723,15 @@ class ElementsAccessorBase : public ElementsAccessor {
uint32_t to_start, uint32_t to_start,
int copy_size) FINAL OVERRIDE { int copy_size) FINAL OVERRIDE {
DCHECK(!from.is_null()); DCHECK(!from.is_null());
ElementsAccessorSubclass::CopyElementsImpl( // NOTE: the ElementsAccessorSubclass::CopyElementsImpl() methods
from, from_start, to, from_kind, to_start, kPackedSizeNotKnown, // violate the handlified function signature convention:
copy_size); // raw pointer parameters in the function that allocates. This is done
// intentionally to avoid ArrayConcat() builtin performance degradation.
// See the comment in another ElementsAccessorBase::CopyElements() for
// details.
ElementsAccessorSubclass::CopyElementsImpl(*from, from_start, *to,
from_kind, to_start,
kPackedSizeNotKnown, copy_size);
} }
virtual void CopyElements( virtual void CopyElements(
@ -742,9 +751,18 @@ class ElementsAccessorBase : public ElementsAccessor {
packed_size = copy_size; packed_size = copy_size;
} }
} }
Handle<FixedArrayBase> from(from_holder->elements()); FixedArrayBase* from = from_holder->elements();
// NOTE: the ElementsAccessorSubclass::CopyElementsImpl() methods
// violate the handlified function signature convention:
// raw pointer parameters in the function that allocates. This is done
// intentionally to avoid ArrayConcat() builtin performance degradation.
//
// Details: The idea is that allocations actually happen only in case of
// copying from object with fast double elements to object with object
// elements. In all the other cases there are no allocations performed and
// handle creation causes noticeable performance degradation of the builtin.
ElementsAccessorSubclass::CopyElementsImpl( ElementsAccessorSubclass::CopyElementsImpl(
from, from_start, to, from_kind, to_start, packed_size, copy_size); from, from_start, *to, from_kind, to_start, packed_size, copy_size);
} }
virtual MaybeHandle<FixedArray> AddElementsToFixedArray( virtual MaybeHandle<FixedArray> AddElementsToFixedArray(
@ -1018,7 +1036,7 @@ class FastElementsAccessor
}; };
static inline ElementsKind ElementsKindForArray(Handle<FixedArrayBase> array) { static inline ElementsKind ElementsKindForArray(FixedArrayBase* array) {
switch (array->map()->instance_type()) { switch (array->map()->instance_type()) {
case FIXED_ARRAY_TYPE: case FIXED_ARRAY_TYPE:
if (array->IsDictionary()) { if (array->IsDictionary()) {
@ -1054,38 +1072,42 @@ class FastSmiOrObjectElementsAccessor
: FastElementsAccessor<FastElementsAccessorSubclass, : FastElementsAccessor<FastElementsAccessorSubclass,
KindTraits>(name) {} KindTraits>(name) {}
static void CopyElementsImpl(Handle<FixedArrayBase> from, // NOTE: this method violates the handlified function signature convention:
uint32_t from_start, // raw pointer parameters in the function that allocates.
Handle<FixedArrayBase> to, // See ElementsAccessor::CopyElements() for details.
ElementsKind from_kind, // This method could actually allocate if copying from double elements to
uint32_t to_start, // object elements.
int packed_size, static void CopyElementsImpl(FixedArrayBase* from, uint32_t from_start,
FixedArrayBase* to, ElementsKind from_kind,
uint32_t to_start, int packed_size,
int copy_size) { int copy_size) {
DisallowHeapAllocation no_gc;
ElementsKind to_kind = KindTraits::Kind; ElementsKind to_kind = KindTraits::Kind;
switch (from_kind) { switch (from_kind) {
case FAST_SMI_ELEMENTS: case FAST_SMI_ELEMENTS:
case FAST_HOLEY_SMI_ELEMENTS: case FAST_HOLEY_SMI_ELEMENTS:
case FAST_ELEMENTS: case FAST_ELEMENTS:
case FAST_HOLEY_ELEMENTS: case FAST_HOLEY_ELEMENTS:
CopyObjectToObjectElements(*from, from_kind, from_start, *to, to_kind, CopyObjectToObjectElements(from, from_kind, from_start, to, to_kind,
to_start, copy_size); to_start, copy_size);
break; break;
case FAST_DOUBLE_ELEMENTS: case FAST_DOUBLE_ELEMENTS:
case FAST_HOLEY_DOUBLE_ELEMENTS: case FAST_HOLEY_DOUBLE_ELEMENTS: {
AllowHeapAllocation allow_allocation;
CopyDoubleToObjectElements( CopyDoubleToObjectElements(
from, from_start, to, to_kind, to_start, copy_size); from, from_start, to, to_kind, to_start, copy_size);
break; break;
}
case DICTIONARY_ELEMENTS: case DICTIONARY_ELEMENTS:
CopyDictionaryToObjectElements(*from, from_start, *to, to_kind, CopyDictionaryToObjectElements(from, from_start, to, to_kind, to_start,
to_start, copy_size); copy_size);
break; break;
case SLOPPY_ARGUMENTS_ELEMENTS: { case SLOPPY_ARGUMENTS_ELEMENTS: {
// TODO(verwaest): This is a temporary hack to support extending // TODO(verwaest): This is a temporary hack to support extending
// SLOPPY_ARGUMENTS_ELEMENTS in SetFastElementsCapacityAndLength. // SLOPPY_ARGUMENTS_ELEMENTS in SetFastElementsCapacityAndLength.
// This case should be UNREACHABLE(). // This case should be UNREACHABLE().
Handle<FixedArray> parameter_map = Handle<FixedArray>::cast(from); FixedArray* parameter_map = FixedArray::cast(from);
Handle<FixedArrayBase> arguments( FixedArrayBase* arguments = FixedArrayBase::cast(parameter_map->get(1));
FixedArrayBase::cast(parameter_map->get(1)));
ElementsKind from_kind = ElementsKindForArray(arguments); ElementsKind from_kind = ElementsKindForArray(arguments);
CopyElementsImpl(arguments, from_start, to, from_kind, CopyElementsImpl(arguments, from_start, to, from_kind,
to_start, packed_size, copy_size); to_start, packed_size, copy_size);
@ -1179,31 +1201,29 @@ class FastDoubleElementsAccessor
} }
protected: protected:
static void CopyElementsImpl(Handle<FixedArrayBase> from, static void CopyElementsImpl(FixedArrayBase* from, uint32_t from_start,
uint32_t from_start, FixedArrayBase* to, ElementsKind from_kind,
Handle<FixedArrayBase> to, uint32_t to_start, int packed_size,
ElementsKind from_kind,
uint32_t to_start,
int packed_size,
int copy_size) { int copy_size) {
DisallowHeapAllocation no_allocation;
switch (from_kind) { switch (from_kind) {
case FAST_SMI_ELEMENTS: case FAST_SMI_ELEMENTS:
CopyPackedSmiToDoubleElements(*from, from_start, *to, to_start, CopyPackedSmiToDoubleElements(from, from_start, to, to_start,
packed_size, copy_size); packed_size, copy_size);
break; break;
case FAST_HOLEY_SMI_ELEMENTS: case FAST_HOLEY_SMI_ELEMENTS:
CopySmiToDoubleElements(*from, from_start, *to, to_start, copy_size); CopySmiToDoubleElements(from, from_start, to, to_start, copy_size);
break; break;
case FAST_DOUBLE_ELEMENTS: case FAST_DOUBLE_ELEMENTS:
case FAST_HOLEY_DOUBLE_ELEMENTS: case FAST_HOLEY_DOUBLE_ELEMENTS:
CopyDoubleToDoubleElements(*from, from_start, *to, to_start, copy_size); CopyDoubleToDoubleElements(from, from_start, to, to_start, copy_size);
break; break;
case FAST_ELEMENTS: case FAST_ELEMENTS:
case FAST_HOLEY_ELEMENTS: case FAST_HOLEY_ELEMENTS:
CopyObjectToDoubleElements(*from, from_start, *to, to_start, copy_size); CopyObjectToDoubleElements(from, from_start, to, to_start, copy_size);
break; break;
case DICTIONARY_ELEMENTS: case DICTIONARY_ELEMENTS:
CopyDictionaryToDoubleElements(*from, from_start, *to, to_start, CopyDictionaryToDoubleElements(from, from_start, to, to_start,
copy_size); copy_size);
break; break;
case SLOPPY_ARGUMENTS_ELEMENTS: case SLOPPY_ARGUMENTS_ELEMENTS:
@ -1439,12 +1459,9 @@ class DictionaryElementsAccessor
return isolate->factory()->true_value(); return isolate->factory()->true_value();
} }
static void CopyElementsImpl(Handle<FixedArrayBase> from, static void CopyElementsImpl(FixedArrayBase* from, uint32_t from_start,
uint32_t from_start, FixedArrayBase* to, ElementsKind from_kind,
Handle<FixedArrayBase> to, uint32_t to_start, int packed_size,
ElementsKind from_kind,
uint32_t to_start,
int packed_size,
int copy_size) { int copy_size) {
UNREACHABLE(); UNREACHABLE();
} }
@ -1654,12 +1671,9 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase<
return isolate->factory()->true_value(); return isolate->factory()->true_value();
} }
static void CopyElementsImpl(Handle<FixedArrayBase> from, static void CopyElementsImpl(FixedArrayBase* from, uint32_t from_start,
uint32_t from_start, FixedArrayBase* to, ElementsKind from_kind,
Handle<FixedArrayBase> to, uint32_t to_start, int packed_size,
ElementsKind from_kind,
uint32_t to_start,
int packed_size,
int copy_size) { int copy_size) {
UNREACHABLE(); UNREACHABLE();
} }
@ -1715,7 +1729,7 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase<
ElementsAccessor* ElementsAccessor::ForArray(Handle<FixedArrayBase> array) { ElementsAccessor* ElementsAccessor::ForArray(Handle<FixedArrayBase> array) {
return elements_accessors_[ElementsKindForArray(array)]; return elements_accessors_[ElementsKindForArray(*array)];
} }

7
src/elements.h
View File

@ -146,9 +146,10 @@ class ElementsAccessor {
uint32_t destination_start, uint32_t destination_start,
int copy_size) = 0; int copy_size) = 0;
// TODO(ishell): Keeping |source_holder| parameter in a non-handlified form // NOTE: this method violates the handlified function signature convention:
// helps avoiding ArrayConcat() builtin performance degradation. // raw pointer parameter |source_holder| in the function that allocates.
// Revisit this later. // This is done intentionally to avoid ArrayConcat() builtin performance
// degradation.
virtual void CopyElements( virtual void CopyElements(
JSObject* source_holder, JSObject* source_holder,
uint32_t source_start, uint32_t source_start,