Fix a bug in the regexp caching. Also add a few more places to

cache.  We now cache most of the places where Opera cache and
one or two where they do not cache for some reason.  Since
these optimizations aren't necessarily useful on real code we
may remove them if and when the Dromaeo website makes the
benchmarks harder to game.
Review URL: http://codereview.chromium.org/995005

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

src/regexp-delay.js

@@ -95,16 +95,7 @@ function DoConstructRegExp(object, pattern, flags, isConstructorCall) {
     %IgnoreAttributesAndSetProperty(object, 'ignoreCase', ignoreCase);
     %IgnoreAttributesAndSetProperty(object, 'multiline', multiline);
     %IgnoreAttributesAndSetProperty(object, 'lastIndex', 0);
-    // Clear the regexp result cache.
-    cachedRegexp = 0;
-    cachedSubject = 0;
-    cachedLastIndex = 0;
-    cachedAnswer = 0;
-    // These are from string.js.
-    cachedReplaceSubject = 0;
-    cachedReplaceRegexp = 0;
-    cachedReplaceReplacement = 0;
-    cachedReplaceAnswer = 0;
+    regExpCache.type = 'none';
   }
 
   // Call internal function to compile the pattern.
@@ -150,10 +141,17 @@ function DoRegExpExec(regexp, string, index) {
 }
 
 
-var cachedRegexp;
-var cachedSubject;
-var cachedLastIndex;
-var cachedAnswer;
+function RegExpCache() {
+  this.type = 'none';
+  this.regExp = 0;
+  this.subject = 0;
+  this.replaceString = 0;
+  this.lastIndex = 0;
+  this.answer = 0;
+}
+
+
+var regExpCache = new RegExpCache();
 
 
 function CloneRegexpAnswer(array) {
@@ -169,10 +167,18 @@ function CloneRegexpAnswer(array) {
 
 
 function RegExpExec(string) {
-  if (%_ObjectEquals(cachedLastIndex, this.lastIndex) &&
-      %_ObjectEquals(cachedRegexp, this) &&
-      %_ObjectEquals(cachedSubject, string)) {
-    var last = cachedAnswer;
+  if (!IS_REGEXP(this)) {
+    throw MakeTypeError('incompatible_method_receiver',
+                        ['RegExp.prototype.exec', this]);
+  }
+
+  var cache = regExpCache;
+
+  if (%_ObjectEquals(cache.type, 'exec') &&
+      %_ObjectEquals(cache.lastIndex, this.lastIndex) &&
+      %_ObjectEquals(cache.regExp, this) &&
+      %_ObjectEquals(cache.subject, string)) {
+    var last = cache.answer;
     if (last == null) {
       return last;
     } else {
@@ -180,10 +186,6 @@ function RegExpExec(string) {
     }
   }
 
-  if (!IS_REGEXP(this)) {
-    throw MakeTypeError('incompatible_method_receiver',
-                        ['RegExp.prototype.exec', this]);
-  }
   if (%_ArgumentsLength() == 0) {
     var regExpInput = LAST_INPUT(lastMatchInfo);
     if (IS_UNDEFINED(regExpInput)) {
@@ -212,10 +214,11 @@ function RegExpExec(string) {
 
   if (matchIndices == null) {
     if (this.global) this.lastIndex = 0;
-    cachedLastIndex = lastIndex;
-    cachedRegexp = this;
-    cachedSubject = s;
-    cachedAnswer = matchIndices;  // Null.
+    cache.lastIndex = lastIndex;
+    cache.regExp = this;
+    cache.subject = s;
+    cache.answer = matchIndices;  // Null.
+    cache.type = 'exec';
     return matchIndices;        // No match.
   }
 
@@ -246,10 +249,11 @@ function RegExpExec(string) {
     this.lastIndex = lastMatchInfo[CAPTURE1];
     return result;
   } else {
-    cachedRegexp = this;
-    cachedSubject = s;
-    cachedLastIndex = lastIndex;
-    cachedAnswer = result;
+    cache.regExp = this;
+    cache.subject = s;
+    cache.lastIndex = lastIndex;
+    cache.answer = result;
+    cache.type = 'exec';
     return CloneRegexpAnswer(result);
   }
 }
@@ -271,13 +275,35 @@ function RegExpTest(string) {
     }
     string = regExpInput;
   }
-  var s = ToString(string);
-  var length = s.length;
+  var s;
+  if (IS_STRING(string)) {
+    s = string;
+  } else {
+    s = ToString(string);
+  }
+
   var lastIndex = this.lastIndex;
+
+  var cache = regExpCache;
+
+  if (%_ObjectEquals(cache.type, 'test') &&
+      %_ObjectEquals(cache.regExp, this) &&
+      %_ObjectEquals(cache.subject, string) &&
+      %_ObjectEquals(cache.lastIndex, lastIndex)) {
+    return cache.answer;
+  }
+
+  var length = s.length;
   var i = this.global ? TO_INTEGER(lastIndex) : 0;
 
+  cache.type = 'test';
+  cache.regExp = this;
+  cache.subject = s;
+  cache.lastIndex = i;
+
   if (i < 0 || i > s.length) {
     this.lastIndex = 0;
+    cache.answer = false;
     return false;
   }
 
@@ -287,10 +313,12 @@ function RegExpTest(string) {
 
   if (matchIndices == null) {
     if (this.global) this.lastIndex = 0;
+    cache.answer = false;
     return false;
   }
 
   if (this.global) this.lastIndex = lastMatchInfo[CAPTURE1];
+  cache.answer = true;
   return true;
 }
 
@@ -409,6 +437,7 @@ function SetupRegExp() {
     return IS_UNDEFINED(regExpInput) ? "" : regExpInput;
   }
   function RegExpSetInput(string) {
+    regExpCache.type = 'none';
     LAST_INPUT(lastMatchInfo) = ToString(string);
   };
 

src/string.js

@@ -168,9 +168,32 @@ function StringMatch(regexp) {
   var subject = TO_STRING_INLINE(this);
 
   if (!regexp.global) return regexp.exec(subject);
+
+  var cache = regExpCache;
+
+  if (%_ObjectEquals(cache.type, 'match') &&
+      %_ObjectEquals(cache.regExp, regexp) &&
+      %_ObjectEquals(cache.subject, subject)) {
+    var last = cache.answer;
+    if (last == null) {
+      return last;
+    } else {
+      return CloneRegexpAnswer(last);
+    }
+  }
+
   %_Log('regexp', 'regexp-match,%0S,%1r', [subject, regexp]);
   // lastMatchInfo is defined in regexp-delay.js.
-  return %StringMatch(subject, regexp, lastMatchInfo);
+  var result = %StringMatch(subject, regexp, lastMatchInfo);
+  cache.type = 'match';
+  cache.regExp = regexp;
+  cache.subject = subject;
+  cache.answer = result;
+  if (result == null) {
+    return result;
+  } else {
+    return CloneRegexpAnswer(result);
+  }
 }
 
 
@@ -206,6 +229,7 @@ function StringReplace(search, replace) {
   if (IS_REGEXP(search)) {
     %_Log('regexp', 'regexp-replace,%0r,%1S', [search, subject]);
     if (IS_FUNCTION(replace)) {
+      regExpCache.type = 'none';
       return StringReplaceRegExpWithFunction(subject, search, replace);
     } else {
       return StringReplaceRegExp(subject, search, replace);
@@ -239,27 +263,25 @@ function StringReplace(search, replace) {
 }
 
 
-var cachedReplaceSubject;
-var cachedReplaceRegexp;
-var cachedReplaceReplacement;
-var cachedReplaceAnswer;
-
 // Helper function for regular expressions in String.prototype.replace.
 function StringReplaceRegExp(subject, regexp, replace) {
-  if (%_ObjectEquals(replace, cachedReplaceReplacement) &&
-      %_ObjectEquals(subject, cachedReplaceSubject) &&
-      %_ObjectEquals(regexp, cachedReplaceRegexp)) {
-    return cachedReplaceAnswer;
+  var cache = regExpCache;
+  if (%_ObjectEquals(cache.regExp, regexp) &&
+      %_ObjectEquals(cache.type, 'replace') &&
+      %_ObjectEquals(cache.replaceString, replace) &&
+      %_ObjectEquals(cache.subject, subject)) {
+    return cache.answer;
   }
   replace = TO_STRING_INLINE(replace);
   var answer = %StringReplaceRegExpWithString(subject,
                                               regexp,
                                               replace,
                                               lastMatchInfo);
-  cachedReplaceSubject = subject;
-  cachedReplaceRegexp = regexp;
-  cachedReplaceReplacement = replace;
-  cachedReplaceAnswer = answer;
+  cache.subject = subject;
+  cache.regExp = regexp;
+  cache.replaceString = replace;
+  cache.answer = answer;
+  cache.type = 'replace';
   return answer;
 }
 
@@ -577,10 +599,26 @@ function StringSplit(separator, limit) {
     return result;
   }
 
+  var cache = regExpCache;
+
+  if (%_ObjectEquals(cache.type, 'split') &&
+      %_ObjectEquals(cache.regExp, separator) &&
+      %_ObjectEquals(cache.subject, subject)) {
+    return CloneRegexpAnswer(cache.answer);
+  }
+
+  cache.type = 'split';
+  cache.regExp = separator;
+  cache.subject = subject;
+
   %_Log('regexp', 'regexp-split,%0S,%1r', [subject, separator]);
 
   if (length === 0) {
-    if (splitMatch(separator, subject, 0, 0) != null) return [];
+    if (splitMatch(separator, subject, 0, 0) != null) {
+      cache.answer = [];
+      return [];
+    }
+    cache.answer = [subject];
     return [subject];
   }
 
@@ -592,14 +630,16 @@ function StringSplit(separator, limit) {
 
     if (startIndex === length) {
       result[result.length] = subject.slice(currentIndex, length);
-      return result;
+      cache.answer = result;
+      return CloneRegexpAnswer(result);
     }
 
     var matchInfo = splitMatch(separator, subject, currentIndex, startIndex);
 
     if (IS_NULL(matchInfo)) {
       result[result.length] = subject.slice(currentIndex, length);
-      return result;
+      cache.answer = result;
+      return CloneRegexpAnswer(result);
     }
 
     var endIndex = matchInfo[CAPTURE1];
@@ -611,7 +651,10 @@ function StringSplit(separator, limit) {
     }
 
     result[result.length] = SubString(subject, currentIndex, matchInfo[CAPTURE0]);
-    if (result.length === limit) return result;
+    if (result.length === limit) {
+      cache.answer = result;
+      return CloneRegexpAnswer(result);
+    }
 
     var num_captures = NUMBER_OF_CAPTURES(matchInfo);
     for (var i = 2; i < num_captures; i += 2) {
@@ -622,7 +665,10 @@ function StringSplit(separator, limit) {
       } else {
         result[result.length] = void 0;
       }
-      if (result.length === limit) return result;
+      if (result.length === limit) {
+        cache.answer = result;
+        return CloneRegexpAnswer(result);
+      }
     }
 
     startIndex = currentIndex = endIndex;

test/mjsunit/regexp-cache-replace.js

+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Tests that regexp caching isn't messing things up.
+
+var re1 = /(o)/g;
+assertEquals("FxxBar", "FooBar".replace(re1, "x"));
+assertEquals("o", RegExp.$1);
+assertTrue(/(x)/.test("abcxdef"));
+assertEquals("x", RegExp.$1);
+assertEquals("FxxBar", "FooBar".replace(re1, "x"));
+assertEquals("o", RegExp.$1);