Reviewers: mvstanton,
Message:
PTAL
Description:
- Initialize the result array with holes if we concat a double array into an
object array, since it may cause a marking step while boxing a double.
- Ensure we go holey if we are concatting any holey array.
Please review this at https://chromiumcodereview.appspot.com/11413142/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/builtins.cc
M src/factory.cc
M src/heap.cc
M src/objects.cc
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index
79a4b5c229da1e10612ad65c354b2bcf0d2b1ccd..526c6e467088c6e5dfdd70729ddd6543b2e5e9a4
100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -268,7 +268,7 @@ static MaybeObject* ArrayCodeGenericCommon(Arguments*
args,
maybe_elms = heap->AllocateFixedArrayWithHoles(number_of_elements);
}
FixedArrayBase* elms;
- if (!maybe_elms->To<FixedArrayBase>(&elms)) return maybe_elms;
+ if (!maybe_elms->To(&elms)) return maybe_elms;
// Fill in the content
switch (array->GetElementsKind()) {
@@ -394,7 +394,7 @@ static FixedArrayBase* LeftTrimFixedArray(Heap* heap,
const int len = elms->length();
- if (to_trim > FixedArrayBase::kHeaderSize / entry_size &&
+ if (to_trim * entry_size > FixedArrayBase::kHeaderSize &&
elms->IsFixedArray() &&
!heap->new_space()->Contains(elms)) {
// If we are doing a big trim in old space then we zap the space that
was
@@ -577,7 +577,7 @@ BUILTIN(ArrayPush) {
if (len > 0) {
ElementsAccessor* accessor = array->GetElementsAccessor();
MaybeObject* maybe_failure =
- accessor->CopyElements(array, 0, new_elms, kind, 0, len,
elms_obj);
+ accessor->CopyElements(NULL, 0, new_elms, kind, 0, len,
elms_obj);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
}
@@ -626,7 +626,7 @@ BUILTIN(ArrayPush) {
if (len > 0) {
ElementsAccessor* accessor = array->GetElementsAccessor();
MaybeObject* maybe_failure =
- accessor->CopyElements(array, 0, new_elms, kind, 0, len,
elms_obj);
+ accessor->CopyElements(NULL, 0, new_elms, kind, 0, len,
elms_obj);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
}
@@ -791,7 +791,7 @@ BUILTIN(ArrayUnshift) {
ElementsKind kind = array->GetElementsKind();
ElementsAccessor* accessor = array->GetElementsAccessor();
MaybeObject* maybe_failure =
- accessor->CopyElements(array, 0, new_elms, kind, to_add, len,
elms);
+ accessor->CopyElements(NULL, 0, new_elms, kind, to_add, len,
elms);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
}
@@ -936,12 +936,13 @@ BUILTIN(ArraySlice) {
result_len,
result_len);
+ AssertNoAllocation no_gc;
if (result_len == 0) return maybe_array;
if (!maybe_array->To(&result_array)) return maybe_array;
ElementsAccessor* accessor = object->GetElementsAccessor();
MaybeObject* maybe_failure =
- accessor->CopyElements(object, k, result_array->elements(),
+ accessor->CopyElements(NULL, k, result_array->elements(),
kind, 0, result_len, elms);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
@@ -1040,15 +1041,19 @@ BUILTIN(ArraySplice) {
actual_delete_count);
if (!maybe_array->To(&result_array)) return maybe_array;
- if (actual_delete_count > 0) {
- ElementsAccessor* accessor = array->GetElementsAccessor();
- MaybeObject* maybe_failure =
- accessor->CopyElements(array, actual_start,
result_array->elements(),
- elements_kind, 0, actual_delete_count,
elms_obj);
- // Cannot fail since the origin and target array are of the same
elements
- // kind.
- ASSERT(!maybe_failure->IsFailure());
- USE(maybe_failure);
+ {
+ AssertNoAllocation no_gc;
+
+ if (actual_delete_count > 0) {
+ ElementsAccessor* accessor = array->GetElementsAccessor();
+ MaybeObject* maybe_failure =
+ accessor->CopyElements(NULL, actual_start,
result_array->elements(),
+ elements_kind, 0, actual_delete_count,
elms_obj);
+ // Cannot fail since the origin and target array are of the same
elements
+ // kind.
+ ASSERT(!maybe_failure->IsFailure());
+ USE(maybe_failure);
+ }
}
bool elms_changed = false;
@@ -1103,19 +1108,21 @@ BUILTIN(ArraySplice) {
MaybeObject* maybe_obj =
heap->AllocateUninitializedFixedArray(capacity);
if (!maybe_obj->To(&new_elms)) return maybe_obj;
+ AssertNoAllocation no_gc;
+
ElementsKind kind = array->GetElementsKind();
ElementsAccessor* accessor = array->GetElementsAccessor();
if (actual_start > 0) {
// Copy the part before actual_start as is.
MaybeObject* maybe_failure = accessor->CopyElements(
- array, 0, new_elms, kind, 0, actual_start, elms);
+ NULL, 0, new_elms, kind, 0, actual_start, elms);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
}
const int to_copy = len - actual_delete_count - actual_start;
if (to_copy > 0) {
MaybeObject* maybe_failure = accessor->CopyElements(
- array, actual_start + actual_delete_count, new_elms, kind,
+ NULL, actual_start + actual_delete_count, new_elms, kind,
actual_start + item_count, to_copy, elms);
ASSERT(!maybe_failure->IsFailure());
USE(maybe_failure);
@@ -1177,6 +1184,8 @@ BUILTIN(ArrayConcat) {
int n_arguments = args.length();
int result_len = 0;
ElementsKind elements_kind = GetInitialFastElementsKind();
+ bool has_double = false;
+ bool is_holey = false;
for (int i = 0; i < n_arguments; i++) {
Object* arg = args[i];
if (!arg->IsJSArray() ||
@@ -1198,18 +1207,28 @@ BUILTIN(ArrayConcat) {
}
ElementsKind arg_kind = JSArray::cast(arg)->map()->elements_kind();
+ has_double = has_double || IsFastDoubleElementsKind(arg_kind);
+ is_holey = is_holey || IsFastHoleyElementsKind(arg_kind);
if (IsMoreGeneralElementsKindTransition(elements_kind, arg_kind)) {
- elements_kind = IsFastHoleyElementsKind(elements_kind)
- ? GetHoleyElementsKind(arg_kind) : arg_kind;
+ elements_kind = arg_kind;
}
}
+ if (is_holey) elements_kind = GetHoleyElementsKind(elements_kind);
+
+ // If a double array is concatted into a fast elements array, the fast
+ // elements array needs to be initialized to contain proper holes, since
+ // boxing doubles may cause incremental marking.
+ ArrayStorageAllocationMode mode =
+ has_double && IsFastObjectElementsKind(elements_kind)
+ ? INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE :
DONT_INITIALIZE_ARRAY_ELEMENTS;
JSArray* result_array;
// Allocate result.
MaybeObject* maybe_array =
heap->AllocateJSArrayAndStorage(elements_kind,
result_len,
- result_len);
+ result_len,
+ mode);
if (!maybe_array->To(&result_array)) return maybe_array;
if (result_len == 0) return result_array;
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
5e38d06a8fda45816b5cd2733cafc6787fedf77f..556f2b01b793bc11c45a1c0a1f094e57b02ff835
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -950,6 +950,9 @@ Handle<JSObject>
Factory::NewJSObjectFromMap(Handle<Map> map) {
Handle<JSArray> Factory::NewJSArray(int capacity,
ElementsKind elements_kind,
PretenureFlag pretenure) {
+ if (capacity != 0) {
+ elements_kind = GetHoleyElementsKind(elements_kind);
+ }
CALL_HEAP_FUNCTION(isolate(),
isolate()->heap()->AllocateJSArrayAndStorage(
elements_kind,
Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index
976d65e959fdc35e50a8efe8964ff9e00a1388c4..d31016c0dc02fbadb0452127f8263fe18db74eb5
100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -4232,9 +4232,6 @@ MaybeObject* Heap::AllocateJSArrayAndStorage(
ArrayStorageAllocationMode mode,
PretenureFlag pretenure) {
ASSERT(capacity >= length);
- if (length != 0 && mode == INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE) {
- elements_kind = GetHoleyElementsKind(elements_kind);
- }
MaybeObject* maybe_array = AllocateJSArray(elements_kind, pretenure);
JSArray* array;
if (!maybe_array->To(&array)) return maybe_array;
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
05c00824009233bb90642c648d78d262f6a73e17..78560df6310daf2f6084417616c580cfde7eb2c4
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -10489,6 +10489,8 @@ MaybeObject*
JSObject::TransitionElementsKind(ElementsKind to_kind) {
to_kind = GetHoleyElementsKind(to_kind);
}
+ if (from_kind == to_kind) return this;
+
Isolate* isolate = GetIsolate();
if (elements() == isolate->heap()->empty_fixed_array() ||
(IsFastSmiOrObjectElementsKind(from_kind) &&
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
