From 186d832c79ae67d81985125605fb5a491f71bf6d Mon Sep 17 00:00:00 2001 From: "antonm@chromium.org" Date: Mon, 14 Feb 2011 17:25:12 +0000 Subject: [PATCH] Introduce new runtime function to make join with lower memory usage. Do not use generic StringBuilderConcat which requires array passed to keep both elements and separator (which roughly double size of the array). That should be faster as well. BUG=crbug.com/54580 Review URL: http://codereview.chromium.org/6520004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6777 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/array.js | 10 +---- src/runtime.cc | 78 ++++++++++++++++++++++++++++++++++++ src/runtime.h | 1 + test/mjsunit/fuzz-natives.js | 3 +- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/array.js b/src/array.js index 1298434d59..ef82674d78 100644 --- a/src/array.js +++ b/src/array.js @@ -161,15 +161,7 @@ function Join(array, length, separator, convert) { var result = %_FastAsciiArrayJoin(elements, separator); if (!IS_UNDEFINED(result)) return result; - var length2 = (length << 1) - 1; - var j = length2; - var i = length; - elements[--j] = elements[--i]; - while (i > 0) { - elements[--j] = separator; - elements[--j] = elements[--i]; - } - return %StringBuilderConcat(elements, length2, ''); + return %StringBuilderJoin(elements, length, separator); } finally { // Make sure to remove the last element of the visited array no // matter what happens. diff --git a/src/runtime.cc b/src/runtime.cc index 599c8eded8..60e5e383ca 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -5803,6 +5803,84 @@ static MaybeObject* Runtime_StringBuilderConcat(Arguments args) { } +static MaybeObject* Runtime_StringBuilderJoin(Arguments args) { + NoHandleAllocation ha; + ASSERT(args.length() == 3); + CONVERT_CHECKED(JSArray, array, args[0]); + if (!args[1]->IsSmi()) { + Top::context()->mark_out_of_memory(); + return Failure::OutOfMemoryException(); + } + int array_length = Smi::cast(args[1])->value(); + CONVERT_CHECKED(String, separator, args[2]); + + if (!array->HasFastElements()) { + return Top::Throw(Heap::illegal_argument_symbol()); + } + FixedArray* fixed_array = FixedArray::cast(array->elements()); + if (fixed_array->length() < array_length) { + array_length = fixed_array->length(); + } + + if (array_length == 0) { + return Heap::empty_string(); + } else if (array_length == 1) { + Object* first = fixed_array->get(0); + if (first->IsString()) return first; + } + + int separator_length = separator->length(); + int max_nof_separators = + (String::kMaxLength + separator_length - 1) / separator_length; + if (max_nof_separators < (array_length - 1)) { + Top::context()->mark_out_of_memory(); + return Failure::OutOfMemoryException(); + } + int length = (array_length - 1) * separator_length; + for (int i = 0; i < array_length; i++) { + String* element = String::cast(fixed_array->get(i)); + int increment = element->length(); + if (increment > String::kMaxLength - length) { + Top::context()->mark_out_of_memory(); + return Failure::OutOfMemoryException(); + } + length += increment; + } + + Object* object; + { MaybeObject* maybe_object = Heap::AllocateRawTwoByteString(length); + if (!maybe_object->ToObject(&object)) return maybe_object; + } + SeqTwoByteString* answer = SeqTwoByteString::cast(object); + + uc16* sink = answer->GetChars(); +#ifdef DEBUG + uc16* end = sink + length; +#endif + + String* first = String::cast(fixed_array->get(0)); + int first_length = first->length(); + String::WriteToFlat(first, sink, 0, first_length); + sink += first_length; + + for (int i = 1; i < array_length; i++) { + ASSERT(sink + separator_length <= end); + String::WriteToFlat(separator, sink, 0, separator_length); + sink += separator_length; + + String* element = String::cast(fixed_array->get(i)); + int element_length = element->length(); + ASSERT(sink + element_length <= end); + String::WriteToFlat(element, sink, 0, element_length); + sink += element_length; + } + ASSERT(sink == end); + + ASSERT(!answer->HasOnlyAsciiChars()); // Use %_FastAsciiArrayJoin instead. + return answer; +} + + static MaybeObject* Runtime_NumberOr(Arguments args) { NoHandleAllocation ha; ASSERT(args.length() == 2); diff --git a/src/runtime.h b/src/runtime.h index fb2ff93c6f..8551196a7c 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -128,6 +128,7 @@ namespace internal { \ F(StringAdd, 2, 1) \ F(StringBuilderConcat, 3, 1) \ + F(StringBuilderJoin, 3, 1) \ \ /* Bit operations */ \ F(NumberOr, 2, 1) \ diff --git a/test/mjsunit/fuzz-natives.js b/test/mjsunit/fuzz-natives.js index 020e3c0c85..cefef0a4b3 100644 --- a/test/mjsunit/fuzz-natives.js +++ b/test/mjsunit/fuzz-natives.js @@ -118,8 +118,9 @@ var knownProblems = { "Abort": true, // Avoid calling the concat operation, because weird lengths - // may lead to out-of-memory. + // may lead to out-of-memory. Ditto for StringBuilderJoin. "StringBuilderConcat": true, + "StringBuilderJoin": true, // These functions use pseudo-stack-pointers and are not robust // to unexpected integer values.