Speed up CreateRegExpMatchResult()

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sstangl, Unassigned)

Tracking

unspecified
mozilla24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 755132 [details] [diff] [review]
patch

This makes roughly the same change as in Bug 876976, which affected .match(), but this time it affects .exec() and therefore affects that jQuery benchmark I intended to speed up in the first place.
Attachment #755132 - Flags: review?(hv1989)
(Reporter)

Comment 1

5 years ago
Created attachment 755135 [details]
microbenchmark

Microbenchmark.

SM (unpatched): 630ms
SM (patched): 220ms
V8 (release): 210ms
That helps a bit.  On the testcase in bug 876846 it takes my times from about 1450ns/iteration to about 1300ns/iteration.

If I comment out these two calls, however:

    if (!SetPropertyHelper(cx, array, cx->names().index, index))
    if (!SetPropertyHelper(cx, array, cx->names().input, inputVal))

then I get something closer to 800ns/iteration.

Note that in the regexp in that testcase there aren't that many parens in the regexp...
Those should be using a define, not a set, btw, per spec... But using DefineNativeProperty doesn't seem to help too much in terms of performance.

A lot of the time here seems to be doing updateSlotsForSpan and whatnot...

Can we somehow preallocate this array with the right shape and slots?
(Reporter)

Comment 4

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #3)
> Can we somehow preallocate this array with the right shape and slots?

Yeah, I'll look into that next.
Attachment #755132 - Flags: review?(hv1989) → review+
Gah, that was the wrong one to backout. You can re-land. Sorry about that :(
https://hg.mozilla.org/mozilla-central/rev/782759aa8a03
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Sean, is there another bug tracking comment 4?
Flags: needinfo?(sstangl)
(Reporter)

Updated

5 years ago
Blocks: 879402
(Reporter)

Comment 11

5 years ago
Yes, filed Bug 879402.
Flags: needinfo?(sstangl)
You need to log in before you can comment on or make changes to this bug.