Closed Bug 1184945 Opened 5 years ago Closed 4 years ago

Increase the size of MatchResultTemplateObject

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: h4writer, Assigned: h4writer)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In octane-regexp we often miss our fastpath to go from ion jit -> irregexp jit because we need more space in the MatchResultTemplateObject. As a result we take the slow path and do "ion jit" -> c++ -> "irregexp jit".

We allow up to 6 submatches in the fast case. In the benchmark we see cases up to 11 submatches. For these we take a slow path.

I doubled the size of the TemplateObject to be a (8 to 16), which allows 14 submatches.

This gives me an improvement from 2950 to 3450 on regexp.
Blocks: 806646
Attached patch PatchSplinter Review
Assignee: nobody → hv1989
Attachment #8635272 - Flags: review?(bhackett1024)
Comment on attachment 8635272 [details] [diff] [review]
Patch

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

Nice!  How often are we still taking a slow path because of an undersized template object?  It would be nifty to be able to size the result object's array based on the number of matches needed, for better memory utilization and robustness.

::: js/src/jit/CodeGenerator.cpp
@@ +1008,5 @@
>      callVM(CloneRegExpObjectInfo, lir);
>  }
>  
>  // The maximum number of pairs we can handle when executing RegExps inline.
> +static const size_t RegExpMaxPairCount = 14;

Preexisting, but it would be nice if this constant was in a header file that could be referenced by createMatchResultTemplateObject when sizing the template array.
Attachment #8635272 - Flags: review?(bhackett1024) → review+
(In reply to Brian Hackett (:bhackett) [Away 7/20 - 7/31] from comment #2)
> Comment on attachment 8635272 [details] [diff] [review]
> Patch
> 
> Review of attachment 8635272 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice!  How often are we still taking a slow path because of an undersized
> template object?  It would be nifty to be able to size the result object's
> array based on the number of matches needed, for better memory utilization
> and robustness.

For normal regexps we don't anymore (e.g. never in octane-regexp). For RegExp with more than 14 submatches we still do, which I think doesn't happen often...
https://hg.mozilla.org/mozilla-central/rev/066e0b4423c9
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.