Optimizing BuildResultFromMatchInfo, StringReplace and StringSplit.

src/string.js

Index: src/string.js
===================================================================
--- src/string.js	(revision 5973)
+++ src/string.js	(working copy)
@@ -208,12 +208,15 @@
                                                         replace);
       }
     } else {
-      return StringReplaceRegExp(subject, search, replace);
+      return %StringReplaceRegExpWithString(subject,
+                                            search,
+                                            TO_STRING_INLINE(replace),
+                                            lastMatchInfo);
     }
   }
 
   // Convert the search argument to a string and search for it.
-  search = TO_STRING_INLINE(search);
+  if (!IS_STRING(search)) search = NonStringToString(search);
   var start = %StringIndexOf(subject, search, 0);
   if (start < 0) return subject;
   var end = start + search.length;
@@ -224,11 +227,15 @@
 
   // Compute the string to replace with.
   if (IS_FUNCTION(replace)) {
-    builder.add(replace.call(null, search, start, subject));
+    builder.add(%_CallFunction(%GetGlobalReceiver(),
+                               search,
+                               start,
+                               subject,
+                               replace));
   } else {
     reusableMatchInfo[CAPTURE0] = start;
     reusableMatchInfo[CAPTURE1] = end;
-    replace = TO_STRING_INLINE(replace);
+    if (!IS_STRING(replace)) replace = NonStringToString(replace);

[Lasse Reichstein, 2010/12/13 09:29:13]

Is this really faster than the macro:

macro TO_STRING_INLINE(arg) = (IS_STRING(%IS_VAR(arg)) ? arg :
NonStringToString(arg));

If so, our compiler should maybe detect assignments from a variable to itself.

[sandholm, 2010/12/13 12:03:35]

No, not really. I reverted this change.

     ExpandReplacement(replace, subject, reusableMatchInfo, builder);
   }
 
@@ -239,15 +246,6 @@
 }
 
 
-// Helper function for regular expressions in String.prototype.replace.
-function StringReplaceRegExp(subject, regexp, replace) {
-  return %StringReplaceRegExpWithString(subject,
-                                        regexp,
-                                        TO_STRING_INLINE(replace),
-                                        lastMatchInfo);
-}
-
-
 // Expand the $-expressions in the string and return a new string with
 // the result.
 function ExpandReplacement(string, subject, matchInfo, builder) {
@@ -576,14 +574,14 @@
   while (true) {
 
     if (startIndex === length) {
-      result[result.length] = subject.slice(currentIndex, length);
+      result.push(subject.slice(currentIndex, length));
       break;
     }
 
     var matchInfo = splitMatch(separator, subject, currentIndex, startIndex);
 
     if (IS_NULL(matchInfo)) {
-      result[result.length] = subject.slice(currentIndex, length);
+      result.push(subject.slice(currentIndex, length));
       break;
     }
 
@@ -595,17 +593,21 @@
       continue;
     }
 
-    result[result.length] = SubString(subject, currentIndex, matchInfo[CAPTURE0]);
+    result.push(SubString(subject, currentIndex, matchInfo[CAPTURE0]));

[Lasse Reichstein, 2010/12/13 09:29:13]

No special casing of this substring?

[sandholm, 2010/12/13 12:03:35]

I have only inlined SubString calls inside loops.

     if (result.length === limit) break;
 
-    var num_captures = NUMBER_OF_CAPTURES(matchInfo);
-    for (var i = 2; i < num_captures; i += 2) {
-      var start = matchInfo[CAPTURE(i)];
-      var end = matchInfo[CAPTURE(i + 1)];
-      if (start != -1 && end != -1) {
-        result[result.length] = SubString(subject, start, end);
+    var num_captures_plus3 = NUMBER_OF_CAPTURES(matchInfo) + 3;

[Lasse Reichstein, 2010/12/13 09:29:13]

Use REGEXP_FIRST_CAPTURE instead of 3.

[sandholm, 2010/12/13 12:03:35]

Done.

+    for (var i = 5; i < num_captures_plus3; ) {

[Lasse Reichstein, 2010/12/13 09:29:13]

And REGEXP_FIRST_CAPTURE + 2 instead of 5.

[sandholm, 2010/12/13 12:03:35]

Done.

+      var start = matchInfo[i++];
+      var end = matchInfo[i++];
+      if (end != -1) {
+        if (start + 1 == end) { 
+          result.push(%_StringCharAt(subject, start));
+        } else {
+          result.push(%_SubString(subject, start, end));
+        }
       } else {
-        result[result.length] = void 0;
+        result.push(void 0);
       }
       if (result.length === limit) break outer_loop;
     }