Allow to hoist MRegExp when "exec" is called

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P1
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla67
Points:
---

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

It looks like we could easily allow hoisting of MRegExp when RegExp.prototype.exec is called when we handle CallRegExpMethodIfWrapped in IsRegExpHoistableCall.

CallRegExpMethodIfWrapped is the only function preventing to hoist RegExps when RegExp.prototype.exec is called.

That means we're now able to hoist MRegExp for RegExp.prototype.test (already works without this patch) and RegExp.prototype.exec. The other RegExp/String built-ins seem to be more difficult to handle correctly. For example we can't hoist a RegExp in String.prototype.search (tested with "".search(/./)) because of the calls to

  • RegExpCreate
  • FlatStringSearch
  • RegExpSearchSlowPath
  • and MRegExpInstanceOptimizable

(So we should probably remove this line, shouldn't we?)

Similarly RegExps can't be hoisted in String.prototype.match (tested with "".match(/./)) because of

  • RegExpCreate
  • FlatStringMatch
  • RegExpMatchSlowPath
  • RegExpGlobalMatchOpt
  • and MRegExpInstanceOptimizable.

This change improves the following µ-benchmark from 520ms to 460ms for me:

function f() {
    var r = 0;
    var t = dateNow()
    for (var i = 0; i < 5000000; ++i){
        if (/b{2,}/.exec("aaaaabaaabbbaaaa")) {
            r++;
        }
    }
    return [dateNow() - t, r];
}
Attachment #9044680 - Flags: review?(jdemooij)
Comment on attachment 9044680 [details] [diff] [review]
bug1528772.patch

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

::: js/src/jit/IonAnalysis.cpp
@@ +2300,5 @@
>      return IsExclusiveFirstArg(call, def);
>    }
>  
> +  if (name == runtime->names().RegExp_prototype_Exec ||
> +      name == runtime->names().CallRegExpMethodIfWrapped) {

The idea is that if we have a RegExpObject we never get to CallRegExpMethodIfWrapped anyway so it doesn't affect behavior, right?
Attachment #9044680 - Flags: review?(jdemooij) → review+
Priority: -- → P1

(In reply to Jan de Mooij [:jandem] from comment #2)

The idea is that if we have a RegExpObject we never get to
CallRegExpMethodIfWrapped anyway so it doesn't affect behavior, right?

Yes, and because it shouldn't matter when calling CallRegExpMethodIfWrapped if the input is a hoisted or cloned RegExp, because CallRegExpMethodIfWrapped doesn't modify any state.

Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cd8d6fd4931
Support hoisting RegExp when "exec" is called. r=jandem

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.