Add fast paths for native RegExps in ES2015 subclass-aware code

src/js/regexp.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 (function(global, utils) {
 
 'use strict';
 
 %CheckIsBootstrapping();
 
@@ skipping 249 matching lines @@
 
   // Successful match.
   if (updateLastIndex) {
     this.lastIndex = RegExpLastMatchInfo[CAPTURE1];
   }
   RETURN_NEW_RESULT_FROM_MATCH_INFO(matchIndices, string);
 }
 
 
 // ES#sec-regexpexec Runtime Semantics: RegExpExec ( R, S )
-function RegExpSubclassExec(regexp, string) {
-  var exec = regexp.exec;
+// Also takes an optional exec method in case our caller
+// has already fetched exec.
+function RegExpSubclassExec(regexp, string, exec) {
+  if (IS_UNDEFINED(exec)) {
+    exec = regexp.exec;
+  }
   if (IS_CALLABLE(exec)) {
     var result = %_Call(exec, regexp, string);
     if (!IS_RECEIVER(result) && !IS_NULL(result)) {
       throw MakeTypeError(kInvalidRegExpExecResult);
     }
     return result;
   }
   return %_Call(RegExpExecJS, regexp, string);
 }
+%SetForceInlineFlag(RegExpSubclassExec);
 
 
 // One-element cache for the simplified test regexp.
 var regexp_key;
 var regexp_val;
 
 // Legacy implementation of RegExp.prototype.test
 // Section 15.10.6.3 doesn't actually make sense, but the intention seems to be
 // that test is defined in terms of String.prototype.exec. However, it probably
 // means the original value of String.prototype.exec, which is what everybody
@@ skipping 185 matching lines @@
 // ES#sec-regexp.prototype-@@split
 // RegExp.prototype [ @@split ] ( string, limit )
 function RegExpSubclassSplit(string, limit) {
   if (!IS_RECEIVER(this)) {
     throw MakeTypeError(kIncompatibleMethodReceiver,
                         "RegExp.prototype.@@split", this);
   }
   string = TO_STRING(string);
   var constructor = SpeciesConstructor(this, GlobalRegExp);
   var flags = TO_STRING(this.flags);
+
+  // TODO(adamk): this fast path is wrong with respect to this.global
+  // and this.sticky, but hopefully the spec will remove those gets
+  // and thus make the assumption of 'exec' having no side-effects
+  // more correct. Also, we doesn't ensure that 'exec' is actually
+  // a data property on RegExp.prototype.
+  var exec;
+  if (IS_REGEXP(this) && constructor === GlobalRegExp) {
+    exec = this.exec;
+    if (exec === RegExpSubclassExecJS) {
+      return %_Call(RegExpSplit, this, string, limit);
+    }
+  }
+
   var unicode = %StringIndexOf(flags, 'u', 0) >= 0;
   var sticky = %StringIndexOf(flags, 'y', 0) >= 0;
   var newFlags = sticky ? flags : flags + "y";
   var splitter = new constructor(this, newFlags);
   var array = new GlobalArray();
   var arrayIndex = 0;
   var lim = (IS_UNDEFINED(limit)) ? kMaxUint32 : TO_UINT32(limit);
   var size = string.length;
   var prevStringIndex = 0;
   if (lim === 0) return array;
   var result;
   if (size === 0) {
     result = RegExpSubclassExec(splitter, string);
     if (IS_NULL(result)) AddIndexedProperty(array, 0, string);
     return array;
   }
   var stringIndex = prevStringIndex;
   while (stringIndex < size) {
     splitter.lastIndex = stringIndex;
-    result = RegExpSubclassExec(splitter, string);
+    result = RegExpSubclassExec(splitter, string, exec);
+    // Ensure exec will be read again on the next loop through.
+    exec = UNDEFINED;
     if (IS_NULL(result)) {
       stringIndex += AdvanceStringIndex(string, stringIndex, unicode);
     } else {
       var end = MinSimple(TO_LENGTH(splitter.lastIndex), size);
       if (end === stringIndex) {
         stringIndex += AdvanceStringIndex(string, stringIndex, unicode);
       } else {
         AddIndexedProperty(
             array, arrayIndex,
             %_SubString(string, prevStringIndex, stringIndex));
@@ skipping 38 matching lines @@
 function RegExpSubclassMatch(string) {
   if (!IS_RECEIVER(this)) {
     throw MakeTypeError(kIncompatibleMethodReceiver,
                         "RegExp.prototype.@@match", this);
   }
   string = TO_STRING(string);
   var global = this.global;
   if (!global) return RegExpSubclassExec(this, string);
   var unicode = this.unicode;
   this.lastIndex = 0;
-  var array = [];
+  var array = new InternalArray();
   var n = 0;
   var result;
   while (true) {
     result = RegExpSubclassExec(this, string);
     if (IS_NULL(result)) {
       if (n === 0) return null;
-      return array;
+      break;
     }
     var matchStr = TO_STRING(result[0]);
-    %AddElement(array, n, matchStr);
+    array[n] = matchStr;
     if (matchStr === "") SetAdvancedStringIndex(this, string, unicode);
     n++;
   }
+  var resultArray = [];
+  %MoveArrayContents(array, resultArray);
+  return resultArray;
 }
 %FunctionRemovePrototype(RegExpSubclassMatch);
 
 
 // Legacy implementation of RegExp.prototype[Symbol.replace] which
 // doesn't properly call the underlying exec method.
 
 // TODO(lrn): This array will survive indefinitely if replace is never
 // called again. However, it will be empty, since the contents are cleared
 // in the finally block.
@@ skipping 256 matching lines @@
     var first = %_StringCharCodeAt(string, index);
     if (first >= 0xD800 && first <= 0xDBFF && string.length > index + 1) {
       var second = %_StringCharCodeAt(string, index + 1);
       if (second >= 0xDC00 && second <= 0xDFFF) {
         increment = 2;
       }
     }
   }
   return increment;
 }
+%SetForceInlineFlag(AdvanceStringIndex);
 
 
 function SetAdvancedStringIndex(regexp, string, unicode) {
   var lastIndex = regexp.lastIndex;
   regexp.lastIndex = lastIndex +
                      AdvanceStringIndex(string, lastIndex, unicode);
 }
+%SetForceInlineFlag(SetAdvancedStringIndex);

[Yang, 2016/03/29 12:19:04]

I see this is only used as a helper. Can we write this as macro instead? That
way we don't have to allocate a function object and have it inlined in
non-optimized code as well.

Same goes for AdvanceStringIndex above.

[adamk, 2016/03/29 20:19:20]

For this patch, it turns out we don't need this stuff anyway (since the fast
path never calls these functions). Macro-ifying them is going to be somewhat
ugly, but that can be handled later if need be.

 
 
 // ES#sec-regexp.prototype-@@replace
 // RegExp.prototype [ @@replace ] ( string, replaceValue )
 function RegExpSubclassReplace(string, replace) {
   if (!IS_RECEIVER(this)) {
     throw MakeTypeError(kIncompatibleMethodReceiver,
                         "RegExp.prototype.@@replace", this);
   }
   string = TO_STRING(string);
   var length = string.length;
   var functionalReplace = IS_CALLABLE(replace);
   if (!functionalReplace) replace = TO_STRING(replace);
-  var global = this.global;
+  var global = TO_BOOLEAN(this.global);
   if (global) {
-    var unicode = this.unicode;
+    var unicode = TO_BOOLEAN(this.unicode);
     this.lastIndex = 0;
   }
+
+  // TODO(adamk): this fast path is wrong with respect to this.global
+  // and this.sticky, but hopefully the spec will remove those gets
+  // and thus make the assumption of 'exec' having no side-effects
+  // more correct. Also, we doesn't ensure that 'exec' is actually
+  // a data property on RegExp.prototype, nor does the fast path
+  // correctly handle lastIndex setting.
+  var exec;
+  if (IS_REGEXP(this)) {
+    exec = this.exec;
+    if (exec === RegExpSubclassExecJS) {
+      return %_Call(RegExpReplace, this, string, replace);
+    }
+  }
+
   var results = new InternalArray();
   var result, replacement;
   while (true) {
-    result = RegExpSubclassExec(this, string);
+    result = RegExpSubclassExec(this, string, exec);
+    // Ensure exec will be read again on the next loop through.
+    exec = UNDEFINED;
     if (IS_NULL(result)) {
       break;
     } else {
       results.push(result);
       if (!global) break;
       var matchStr = TO_STRING(result[0]);
       if (matchStr === "") SetAdvancedStringIndex(this, string, unicode);
     }
   }
   var accumulatedResult = "";
@@ skipping 134 matching lines @@
         kRegExpNonObject, "RegExp.prototype.flags", TO_STRING(this));
   }
   var result = '';
   if (this.global) result += 'g';
   if (this.ignoreCase) result += 'i';
   if (this.multiline) result += 'm';
   if (this.unicode) result += 'u';
   if (this.sticky) result += 'y';
   return result;
 }
+%SetForceInlineFlag(RegExpGetFlags);

[Yang, 2016/03/29 12:19:04]

I can see the need for individual flag getters below, if they are used in other
regexp functions. But do we need it for .flags() as well?

[adamk, 2016/03/29 20:19:20]

No, flags barely shows up on the profile, these got added for symmetry. Happy to
remove them. I agree that I got a little bit over-zealous with the
%SetForceInlineFlag...

 
 
 // ES6 21.2.5.4.
 function RegExpGetGlobal() {
   if (!IS_REGEXP(this)) {
     // TODO(littledan): Remove this RegExp compat workaround
     if (this === GlobalRegExpPrototype) {
       %IncrementUseCounter(kRegExpPrototypeOldFlagGetter);
       return UNDEFINED;
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.global");
   }
-  return !!REGEXP_GLOBAL(this);
+  return TO_BOOLEAN(REGEXP_GLOBAL(this));
 }
+%SetForceInlineFlag(RegExpGetGlobal);
 
 
 // ES6 21.2.5.5.
 function RegExpGetIgnoreCase() {
   if (!IS_REGEXP(this)) {
     // TODO(littledan): Remove this RegExp compat workaround
     if (this === GlobalRegExpPrototype) {
       %IncrementUseCounter(kRegExpPrototypeOldFlagGetter);
       return UNDEFINED;
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.ignoreCase");
   }
-  return !!REGEXP_IGNORE_CASE(this);
+  return TO_BOOLEAN(REGEXP_IGNORE_CASE(this));
 }
+%SetForceInlineFlag(RegExpGetIgnoreCase);
 
 
 // ES6 21.2.5.7.
 function RegExpGetMultiline() {
   if (!IS_REGEXP(this)) {
     // TODO(littledan): Remove this RegExp compat workaround
     if (this === GlobalRegExpPrototype) {
       %IncrementUseCounter(kRegExpPrototypeOldFlagGetter);
       return UNDEFINED;
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.multiline");
   }
-  return !!REGEXP_MULTILINE(this);
+  return TO_BOOLEAN(REGEXP_MULTILINE(this));
 }
+%SetForceInlineFlag(RegExpGetMultiline);

[Yang, 2016/03/29 12:19:04]

Is this only for symmetry? I don't think any of the subclass functions use this.

[adamk, 2016/03/29 20:19:20]

Removed, as noted. The only getters that still have them are "global", "sticky",
and "unicode".

 
 
 // ES6 21.2.5.10.
 function RegExpGetSource() {
   if (!IS_REGEXP(this)) {
     // TODO(littledan): Remove this RegExp compat workaround
     if (this === GlobalRegExpPrototype) {
       %IncrementUseCounter(kRegExpPrototypeSourceGetter);
       return UNDEFINED;
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.source");
   }
   return REGEXP_SOURCE(this);
 }
 
 
 // ES6 21.2.5.12.
 function RegExpGetSticky() {
   if (!IS_REGEXP(this)) {
     // Compat fix: RegExp.prototype.sticky == undefined; UseCounter tracks it
     // TODO(littledan): Remove this workaround or standardize it
     if (this === GlobalRegExpPrototype) {
       %IncrementUseCounter(kRegExpPrototypeStickyGetter);
       return UNDEFINED;
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.sticky");
   }
-  return !!REGEXP_STICKY(this);
+  return TO_BOOLEAN(REGEXP_STICKY(this));
 }
+%SetForceInlineFlag(RegExpGetSticky);
 
 // -------------------------------------------------------------------
 
 %FunctionSetInstanceClassName(GlobalRegExp, 'RegExp');
 GlobalRegExpPrototype = new GlobalObject();
 %FunctionSetPrototype(GlobalRegExp, GlobalRegExpPrototype);
 %AddNamedProperty(
     GlobalRegExp.prototype, 'constructor', GlobalRegExp, DONT_ENUM);
 %SetCode(GlobalRegExp, RegExpConstructor);
 
@@ skipping 70 matching lines @@
   to.RegExpSubclassMatch = RegExpSubclassMatch;
   to.RegExpSubclassReplace = RegExpSubclassReplace;
   to.RegExpSubclassSearch = RegExpSubclassSearch;
   to.RegExpSubclassSplit = RegExpSubclassSplit;
   to.RegExpSubclassTest = RegExpSubclassTest;
   to.RegExpTest = RegExpTest;
   to.IsRegExp = IsRegExp;
 });
 
 })