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

src/js/regexp.js

Index: src/js/regexp.js
diff --git a/src/js/regexp.js b/src/js/regexp.js
index 611f780612d35a6628f3f9524d852c37a2238294..e3c6c25f62f7d6a8e83284ce719103edce3881a4 100644
--- a/src/js/regexp.js
+++ b/src/js/regexp.js
@@ -267,8 +267,12 @@ function RegExpExecJS(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)) {
@@ -278,6 +282,7 @@ function RegExpSubclassExec(regexp, string) {
   }
   return %_Call(RegExpExecJS, regexp, string);
 }
+%SetForceInlineFlag(RegExpSubclassExec);
 
 
 // One-element cache for the simplified test regexp.
@@ -483,6 +488,20 @@ function RegExpSubclassSplit(string, limit) {
   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";
@@ -502,7 +521,9 @@ function RegExpSubclassSplit(string, limit) {
   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 {
@@ -561,20 +582,23 @@ function RegExpSubclassMatch(string) {
   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);
 
@@ -851,6 +875,7 @@ function AdvanceStringIndex(string, index, unicode) {
   }
   return increment;
 }
+%SetForceInlineFlag(AdvanceStringIndex);
 
 
 function SetAdvancedStringIndex(regexp, string, unicode) {
@@ -858,6 +883,7 @@ function SetAdvancedStringIndex(regexp, string, unicode) {
   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
@@ -871,15 +897,32 @@ function RegExpSubclassReplace(string, replace) {
   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 {
@@ -1034,6 +1077,7 @@ function RegExpGetFlags() {
   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.
@@ -1046,8 +1090,9 @@ function RegExpGetGlobal() {
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.global");
   }
-  return !!REGEXP_GLOBAL(this);
+  return TO_BOOLEAN(REGEXP_GLOBAL(this));
 }
+%SetForceInlineFlag(RegExpGetGlobal);
 
 
 // ES6 21.2.5.5.
@@ -1060,8 +1105,9 @@ function RegExpGetIgnoreCase() {
     }
     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.
@@ -1074,8 +1120,9 @@ function RegExpGetMultiline() {
     }
     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.
@@ -1103,8 +1150,9 @@ function RegExpGetSticky() {
     }
     throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.sticky");
   }
-  return !!REGEXP_STICKY(this);
+  return TO_BOOLEAN(REGEXP_STICKY(this));
 }
+%SetForceInlineFlag(RegExpGetSticky);
 
 // -------------------------------------------------------------------