836087 - Avoid double lastIndex in ExecuteRegExp().

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sean Stangl [:sstangl]]

Attachment: Avoid using doubles for lastIndex in ExecuteRegExp().

Moves v8-regexp 3650 -> 3730. x87 floating-point operations in ExecuteRegExp() showed up extremely heavily in profiles due to unnecessary double conversion, when lastIndex is always an integer unless the user manually sets it to something stupid. This patch puts all the slow double code into a path that is only taken in the user-set case.

Comment 1

[Nicolas B. Pierron [:nbp]]

Comment on attachment 707871 [details] [diff] [review]
Avoid using doubles for lastIndex in ExecuteRegExp().

Review of attachment 707871 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine for me, just one simple question, can the length be INT_MAX?

FYI, Bug 647385:

diff --git a/js/src/jsregexp.cpp b/js/src/jsregexp.cpp
index 40ca19a..0df8ae5 100644
--- a/js/src/jsregexp.cpp
+++ b/js/src/jsregexp.cpp
@@ -678,15 +678,8 @@ ExecuteRegExp(JSContext *cx, ExecType execType, uintN argc, Value *vp)
 
     /* Step 5. */
     jsdouble i;
-    if (lastIndex.isInt32()) {
-        i = lastIndex.toInt32();
-    } else {
-        if (lastIndex.isDouble())
-            i = lastIndex.toDouble();
-        else if (!ValueToNumber(cx, lastIndex, &i))
-            return false;
-        i = js_DoubleToInteger(i);
-    }
+    if (!ToInteger(cx, lastIndex, &i))
+        return false;
 
     /* Steps 6-7 (with sticky extension). */
     if (!re->global() && !re->sticky())

Comment 2

[Sean Stangl [:sstangl]]

Thanks for the review!

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #1)
> Comment on attachment 707871 [details] [diff] [review]
> Avoid using doubles for lastIndex in ExecuteRegExp().
> 
> Review of attachment 707871 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is fine for me, just one simple question, can the length be INT_MAX?

Strings are limited to a length of 2^28.
 
> FYI, Bug 647385:

Inlining steps 6, 7, and 9a avoids this problem. :)

Comment 3

[Sean Stangl [:sstangl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/be266bba7f88

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/be266bba7f88