Improve performance of Array.prototype.join and String.prototype.substring

by tweaking the JavaScript implementation of these functions.
Review URL: http://codereview.chromium.org/519061

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3545 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/array.js

@@ -70,19 +70,22 @@ function GetSortedArrayKeys(array, intervals) {
 // Optimized for sparse arrays if separator is ''.
 function SparseJoin(array, len, convert) {
   var keys = GetSortedArrayKeys(array, %GetArrayKeys(array, len));
-  var builder = new StringBuilder();
   var last_key = -1;
   var keys_length = keys.length;
+
+  var elements = new $Array(keys_length);
+  var elements_length = 0;
+
   for (var i = 0; i < keys_length; i++) {
     var key = keys[i];
     if (key != last_key) {
       var e = array[key];
-      if (typeof(e) !== 'string') e = convert(e);
-      builder.add(e);
+      if (!IS_STRING(e)) e = convert(e);
+      elements[elements_length++] = e;
       last_key = key;
     }
   }
-  return builder.generate();
+  return %StringBuilderConcat(elements, elements_length, '');
 }
 
 
@@ -107,7 +110,7 @@ function Join(array, length, separator, convert) {
 
   // Attempt to convert the elements.
   try {
-    if (UseSparseVariant(array, length, is_array) && separator === '') {
+    if (UseSparseVariant(array, length, is_array) && (separator.length == 0)) {
       return SparseJoin(array, length, convert);
     }
 
@@ -115,39 +118,37 @@ function Join(array, length, separator, convert) {
     if (length == 1) {
       var e = array[0];
       if (!IS_UNDEFINED(e) || (0 in array)) {
-        if (typeof(e) === 'string') return e;
+        if (IS_STRING(e)) return e;
         return convert(e);
       }
     }
 
-    var builder = new StringBuilder();
+    // Construct an array for the elements.
+    var elements;
+    var elements_length = 0;
 
     // We pull the empty separator check outside the loop for speed!
     if (separator.length == 0) {
+      elements = new $Array(length);
       for (var i = 0; i < length; i++) {
         var e = array[i];
         if (!IS_UNDEFINED(e) || (i in array)) {
-          if (typeof(e) !== 'string') e = convert(e);
-          if (e.length > 0) {
-            var elements = builder.elements;
-            elements[elements.length] = e;
-          }
+          if (!IS_STRING(e)) e = convert(e);
+          elements[elements_length++] = e;
         }
       }
     } else {
+      elements = new $Array(length << 1);
       for (var i = 0; i < length; i++) {
         var e = array[i];
-        if (i != 0) builder.add(separator);
+        if (i != 0) elements[elements_length++] = separator;
         if (!IS_UNDEFINED(e) || (i in array)) {
-          if (typeof(e) !== 'string') e = convert(e);
-          if (e.length > 0) {
-            var elements = builder.elements;
-            elements[elements.length] = e;
-          }
+          if (!IS_STRING(e)) e = convert(e);
+          elements[elements_length++] = e;
         }
       }
     }
-    return builder.generate();
+    return %StringBuilderConcat(elements, elements_length, '');
   } finally {
     // Make sure to pop the visited array no matter what happens.
     if (is_array) visited_arrays.pop();
@@ -156,16 +157,15 @@ function Join(array, length, separator, convert) {
 
 
 function ConvertToString(e) {
-  if (typeof(e) === 'string') return e;
   if (e == null) return '';
   else return ToString(e);
 }
 
 
 function ConvertToLocaleString(e) {
-  if (typeof(e) === 'string') return e;
-  if (e == null) return '';
-  else {
+  if (e == null) {
+    return '';
+  } else {
     // e_obj's toLocaleString might be overwritten, check if it is a function.
     // Call ToString if toLocaleString is not a function.
     // See issue 877615.
@@ -359,16 +359,20 @@ function ArrayToLocaleString() {
 
 
 function ArrayJoin(separator) {
-  if (IS_UNDEFINED(separator)) separator = ',';
-  else separator = ToString(separator);
-  return Join(this, ToUint32(this.length), separator, ConvertToString);
+  if (IS_UNDEFINED(separator)) {
+    separator = ',';
+  } else if (!IS_STRING(separator)) {
+    separator = ToString(separator);
+  }
+  var length = TO_UINT32(this.length);
+  return Join(this, length, separator, ConvertToString);
 }
 
 
 // Removes the last element from the array and returns it. See
 // ECMA-262, section 15.4.4.6.
 function ArrayPop() {
-  var n = ToUint32(this.length);
+  var n = TO_UINT32(this.length);
   if (n == 0) {
     this.length = n;
     return;
@@ -384,7 +388,7 @@ function ArrayPop() {
 // Appends the arguments to the end of the array and returns the new
 // length of the array. See ECMA-262, section 15.4.4.7.
 function ArrayPush() {
-  var n = ToUint32(this.length);
+  var n = TO_UINT32(this.length);
   var m = %_ArgumentsLength();
   for (var i = 0; i < m; i++) {
     this[i+n] = %_Arguments(i);
@@ -452,7 +456,7 @@ function SparseReverse(array, len) {
 
 
 function ArrayReverse() {
-  var j = ToUint32(this.length) - 1;
+  var j = TO_UINT32(this.length) - 1;
 
   if (UseSparseVariant(this, j, IS_ARRAY(this))) {
     SparseReverse(this, j+1);
@@ -483,7 +487,7 @@ function ArrayReverse() {
 
 
 function ArrayShift() {
-  var len = ToUint32(this.length);
+  var len = TO_UINT32(this.length);
 
   if (len === 0) {
     this.length = 0;
@@ -504,7 +508,7 @@ function ArrayShift() {
 
 
 function ArrayUnshift(arg1) {  // length == 1
-  var len = ToUint32(this.length);
+  var len = TO_UINT32(this.length);
   var num_arguments = %_ArgumentsLength();
 
   if (IS_ARRAY(this))
@@ -523,7 +527,7 @@ function ArrayUnshift(arg1) {  // length == 1
 
 
 function ArraySlice(start, end) {
-  var len = ToUint32(this.length);
+  var len = TO_UINT32(this.length);
   var start_i = TO_INTEGER(start);
   var end_i = len;
 
@@ -568,7 +572,7 @@ function ArraySplice(start, delete_count) {
   // compatibility.
   if (num_arguments == 0) return;
 
-  var len = ToUint32(this.length);
+  var len = TO_UINT32(this.length);
   var start_i = TO_INTEGER(start);
 
   if (start_i < 0) {
@@ -850,7 +854,7 @@ function ArraySort(comparefn) {
     return first_undefined;
   }
 
-  length = ToUint32(this.length);
+  length = TO_UINT32(this.length);
   if (length < 2) return this;
 
   var is_array = IS_ARRAY(this);

src/macros.py

@@ -97,7 +97,8 @@ macro FLOOR(arg)                = $floor(arg);
 # Inline macros. Use %IS_VAR to make sure arg is evaluated only once.
 macro NUMBER_IS_NAN(arg) = (!%_IsSmi(%IS_VAR(arg)) && !(arg == arg));
 macro TO_INTEGER(arg)    = (%_IsSmi(%IS_VAR(arg)) ? arg : ToInteger(arg));
-macro TO_INT32(arg)      = (%_IsSmi(%IS_VAR(arg)) ? arg : ToInt32(arg));
+macro TO_INT32(arg)      = (%_IsSmi(%IS_VAR(arg)) ? arg : (arg >> 0));
+macro TO_UINT32(arg)     = (arg >>> 0);
 
 # Macros implemented in Python.
 python macro CHAR_CODE(str) = ord(str[1]);

src/math.js

@@ -103,7 +103,7 @@ function MathFloor(x) {
     // them to an unsigned 32-bit value using the shift operator.
     // We avoid doing so for -0, because the result of Math.floor(-0)
     // has to be -0, which wouldn't be the case with the shift.
-    return x << 0;
+    return TO_UINT32(x);
   } else {
     return %Math_floor(x);
   }

src/runtime.cc

@@ -3914,20 +3914,19 @@ static inline void StringBuilderConcatHelper(String* special,
 
 static Object* Runtime_StringBuilderConcat(Arguments args) {
   NoHandleAllocation ha;
-  ASSERT(args.length() == 2);
+  ASSERT(args.length() == 3);
   CONVERT_CHECKED(JSArray, array, args[0]);
-  CONVERT_CHECKED(String, special, args[1]);
+  if (!args[1]->IsSmi()) {
+    Top::context()->mark_out_of_memory();
+    return Failure::OutOfMemoryException();
+  }
+  int array_length = Smi::cast(args[1])->value();
+  CONVERT_CHECKED(String, special, args[2]);
 
   // This assumption is used by the slice encoding in one or two smis.
   ASSERT(Smi::kMaxValue >= String::kMaxLength);
 
   int special_length = special->length();
-  Object* smi_array_length = array->length();
-  if (!smi_array_length->IsSmi()) {
-    Top::context()->mark_out_of_memory();
-    return Failure::OutOfMemoryException();
-  }
-  int array_length = Smi::cast(smi_array_length)->value();
   if (!array->HasFastElements()) {
     return Top::Throw(Heap::illegal_argument_symbol());
   }

src/runtime.h

@@ -103,7 +103,7 @@ namespace internal {
   F(NumberUnaryMinus, 1, 1) \
   \
   F(StringAdd, 2, 1) \
-  F(StringBuilderConcat, 2, 1) \
+  F(StringBuilderConcat, 3, 1) \
   \
   /* Bit operations */ \
   F(NumberOr, 2, 1) \

src/runtime.js

@@ -427,7 +427,7 @@ function APPLY_PREPARE(args) {
     }
   }
 
-  length = (args == null) ? 0 : %ToUint32(args.length);
+  length = (args == null) ? 0 : TO_UINT32(args.length);
 
   // We can handle any number of apply arguments if the stack is
   // big enough, but sanity check the value to avoid overflow when

src/string.js

@@ -505,7 +505,7 @@ function StringSlice(start, end) {
 // ECMA-262 section 15.5.4.14
 function StringSplit(separator, limit) {
   var subject = ToString(this);
-  limit = (limit === void 0) ? 0xffffffff : ToUint32(limit);
+  limit = (IS_UNDEFINED(limit)) ? 0xffffffff : TO_UINT32(limit);
   if (limit === 0) return [];
 
   // ECMA-262 says that if separator is undefined, the result should
@@ -604,22 +604,30 @@ function splitMatch(separator, subject, current_index, start_index) {
 
 // ECMA-262 section 15.5.4.15
 function StringSubstring(start, end) {
-  var s = ToString(this);
+  var s = this;
+  if (!IS_STRING(s)) s = ToString(s);
   var s_len = s.length;
+
   var start_i = TO_INTEGER(start);
+  if (start_i < 0) {
+    start_i = 0;
+  } else if (start_i > s_len) {
+    start_i = s_len;
+  }
+
   var end_i = s_len;
-  if (!IS_UNDEFINED(end))
+  if (!IS_UNDEFINED(end)) {
     end_i = TO_INTEGER(end);
-
-  if (start_i < 0) start_i = 0;
-  if (start_i > s_len) start_i = s_len;
-  if (end_i < 0) end_i = 0;
-  if (end_i > s_len) end_i = s_len;
-
-  if (start_i > end_i) {
-    var tmp = end_i;
-    end_i = start_i;
-    start_i = tmp;
+    if (end_i > s_len) {
+      end_i = s_len;
+    } else {
+      if (end_i < 0) end_i = 0;
+      if (start_i > end_i) {
+        var tmp = end_i;
+        end_i = start_i;
+        start_i = tmp;
+      }
+    }
   }
 
   return SubString(s, start_i, end_i);
@@ -790,21 +798,14 @@ function StringSup() {
 }
 
 
-// StringBuilder support.
-
-function StringBuilder() {
-  this.elements = new $Array();
-}
-
-
+// ReplaceResultBuilder support.
 function ReplaceResultBuilder(str) {
   this.elements = new $Array();
   this.special_string = str;
 }
 
 
-ReplaceResultBuilder.prototype.add =
-StringBuilder.prototype.add = function(str) {
+ReplaceResultBuilder.prototype.add = function(str) {
   if (!IS_STRING(str)) str = ToString(str);
   if (str.length > 0) {
     var elements = this.elements;
@@ -828,13 +829,9 @@ ReplaceResultBuilder.prototype.addSpecialSlice = function(start, end) {
 }
 
 
-StringBuilder.prototype.generate = function() {
-  return %StringBuilderConcat(this.elements, "");
-}
-
-
 ReplaceResultBuilder.prototype.generate = function() {
-  return %StringBuilderConcat(this.elements, this.special_string);
+  var elements = this.elements;
+  return %StringBuilderConcat(elements, elements.length, this.special_string);
 }
 
 

test/mjsunit/fuzz-natives.js

@@ -95,7 +95,11 @@ function testArgumentTypes(name, argc) {
 
 var knownProblems = {
   "Abort": true,
-  
+
+  // Avoid calling the concat operation, because weird lengths
+  // may lead to out-of-memory.
+  "StringBuilderConcat": true,
+
   // These functions use pseudo-stack-pointers and are not robust
   // to unexpected integer values.
   "DebugEvaluate": true,
@@ -114,7 +118,7 @@ var knownProblems = {
   // the rest of the tests.
   "DisableAccessChecks": true,
   "EnableAccessChecks": true,
-  
+
   // These functions should not be callable as runtime functions.
   "NewContext": true,
   "NewArgumentsFast": true,