Avoid double lastIndex in ExecuteRegExp().

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: sstangl, Unassigned)

Tracking

(Blocks 1 bug)

unspecified
mozilla21
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

7 years ago
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.
Attachment #707871 - Flags: review?(nicolas.b.pierron)
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())
Attachment #707871 - Flags: review?(nicolas.b.pierron) → review+
Reporter

Comment 2

7 years ago
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. :)
https://hg.mozilla.org/mozilla-central/rev/be266bba7f88
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.