Closed Bug 836087 Opened 9 years ago Closed 9 years ago

Avoid double lastIndex in ExecuteRegExp().

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: sstangl, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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+
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: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.