Closed Bug 1415161 Opened 2 years ago Closed 2 years ago

JSContext::findVersion slows down CheckPatternSyntax

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
New candidate for silliest perf bug of the year. CheckPatternSyntax initializes a CompileOptions but doesn't pass a JSVersion, so the CompileOptions constructor ends up calling JSContext::findVersion which can be really slow because it walks the stack.

Hopefully we can remove JSVersion soon, but for now here's a patch to use JSVERSION_DEFAULT in CheckPatternSyntax.

This improves the micro-benchmark below from 223 ms to 200 ms.

function f() {
    var t = new Date;
    for (let i = 0; i < 1000000; i++) {
        re = new RegExp("foo");
    }
    print(new Date - t);
}
f();
Attachment #8925915 - Flags: review?(andrebargull)
(I think there's more we can optimize here btw; looking into that still.)
Comment on attachment 8925915 [details] [diff] [review]
Patch

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

Oh wow, I'm kind of speechless right now! :-)
Attachment #8925915 - Flags: review?(andrebargull) → review+
Oh I should fix the Maybe<CompileOptions> in RegExpObject::create too I guess.
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d82266d06a5b
Get rid of slow JSContext::findVersion calls when parsing regular expressions. r=anba
https://hg.mozilla.org/mozilla-central/rev/d82266d06a5b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.