Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(174)

Unified Diff: src/elements.cc

Issue 1827573003: Reland of [elements] Minor hardening and cleanup of concat (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/elements.cc
diff --git a/src/elements.cc b/src/elements.cc
index 1cc5c8ba299c4a8a350388d5f437e5cd967ed239..7c2d3ef1dc9cb34e0a627938c9502b318ba86bef 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -2854,17 +2854,18 @@
Handle<JSArray> ElementsAccessor::Concat(Isolate* isolate, Arguments* args,
uint32_t concat_size) {
- int result_len = 0;
- ElementsKind elements_kind = GetInitialFastElementsKind();
- bool has_double = false;
+ uint32_t result_len = 0;
+ bool has_raw_doubles = false;
+ ElementsKind result_elements_kind = GetInitialFastElementsKind();
{
DisallowHeapAllocation no_gc;
+ bool is_holey = false;
// Iterate through all the arguments performing checks
// and calculating total length.
- bool is_holey = false;
for (uint32_t i = 0; i < concat_size; i++) {
- Object* arg = (*args)[i];
- int len = Smi::cast(JSArray::cast(arg)->length())->value();
+ JSArray* array = JSArray::cast((*args)[i]);
+ uint32_t len = 0;
+ array->length()->ToArrayLength(&len);
// We shouldn't overflow when adding another len.
const int kHalfOfMaxInt = 1 << (kBitsPerInt - 2);
@@ -2874,42 +2875,45 @@
DCHECK(0 <= result_len);
DCHECK(result_len <= FixedDoubleArray::kMaxLength);
- ElementsKind arg_kind = JSArray::cast(arg)->map()->elements_kind();
- has_double = has_double || IsFastDoubleElementsKind(arg_kind);
+ ElementsKind arg_kind = array->GetElementsKind();
+ has_raw_doubles = has_raw_doubles || IsFastDoubleElementsKind(arg_kind);
is_holey = is_holey || IsFastHoleyElementsKind(arg_kind);
- elements_kind = GetMoreGeneralElementsKind(elements_kind, arg_kind);
+ result_elements_kind =
+ GetMoreGeneralElementsKind(result_elements_kind, arg_kind);
}
if (is_holey) {
- elements_kind = GetHoleyElementsKind(elements_kind);
+ result_elements_kind = GetHoleyElementsKind(result_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;
+ bool requires_double_boxing =
+ has_raw_doubles && !IsFastDoubleElementsKind(result_elements_kind);
+ ArrayStorageAllocationMode mode = requires_double_boxing
+ ? INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE
+ : DONT_INITIALIZE_ARRAY_ELEMENTS;
Handle<JSArray> result_array = isolate->factory()->NewJSArray(
- elements_kind, result_len, result_len, mode);
+ result_elements_kind, result_len, result_len, mode);
if (result_len == 0) return result_array;
- int j = 0;
+
+ uint32_t insertion_index = 0;
Handle<FixedArrayBase> storage(result_array->elements(), isolate);
- ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind);
+ ElementsAccessor* accessor = ElementsAccessor::ForKind(result_elements_kind);
for (uint32_t i = 0; i < concat_size; i++) {
// It is crucial to keep |array| in a raw pointer form to avoid
// performance degradation.
JSArray* array = JSArray::cast((*args)[i]);
- int len = Smi::cast(array->length())->value();
- if (len > 0) {
- ElementsKind from_kind = array->GetElementsKind();
- accessor->CopyElements(array, 0, from_kind, storage, j, len);
- j += len;
- }
- }
-
- DCHECK(j == result_len);
+ uint32_t len = 0;
+ array->length()->ToArrayLength(&len);
+ if (len == 0) continue;
+ ElementsKind from_kind = array->GetElementsKind();
+ accessor->CopyElements(array, 0, from_kind, storage, insertion_index, len);
+ insertion_index += len;
+ }
+
+ DCHECK_EQ(insertion_index, result_len);
return result_array;
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698