RegExp.prototype.split may mis-optimize when RegExp.prototype.flags is deleted and Object.prototype.flags is added and then changed.

RESOLVED FIXED in Firefox 52

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

Trunk
mozilla53
Points:
---

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox50 unaffected, firefox51 unaffected, firefox52 fixed, firefox53 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
from bug 1322319.

js::RegExpPrototypeOptimizableRaw checks flags getter in prototype chain, but it should check own property.
(Assignee)

Updated

2 years ago
See Also: → bug 1322319
(Assignee)

Comment 1

2 years ago
Created attachment 8818145 [details] [diff] [review]
Check own flags property in RegExpPrototypeOptimizableRaw.

Added GetOwnGetterPure that checks own getter, and used it in js::RegExpPrototypeOptimizableRaw to check flags getter.

Also, removed NativeObject* parameter from NativeGetGetterPureInline since it's not used.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8818145 - Flags: review?(hv1989)
Comment on attachment 8818145 [details] [diff] [review]
Check own flags property in RegExpPrototypeOptimizableRaw.

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

Thanks for the testcase!
Attachment #8818145 - Flags: review?(hv1989) → review+
(Assignee)

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/98106f49ee61a352c18bd127554f375cddbc6f66
Bug 1323108 - Check own flags property in RegExpPrototypeOptimizableRaw. r=h4writer

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/98106f49ee61
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Updated

2 years ago
Blocks: 1263340
(Assignee)

Updated

2 years ago
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
status-firefox52: --- → affected
status-firefox-esr45: --- → unaffected
(Assignee)

Comment 5

2 years ago
Created attachment 8820508 [details] [diff] [review]
(mozilla-aurora) Check own flags property in RegExpPrototypeOptimizableRaw. r=h4writer

Approval Request Comment
> [Feature/Bug causing the regression]
bug 1263340

> [User impact if declined]
wrong behavior in JavaScript

> [Is this code covered by automated tests?]
Yes

> [Has the fix been verified in Nightly?]
Yes

> [Needs manual test from QE? If yes, steps to reproduce]
No

> [List of other uplifts needed for the feature/fix]
Bug 1322319 needs to be uplifted *before* this.

> [Is the change risky?]
No

> [Why is the change risky/not risky?]
It reduces the target of optimization, and fallback to normal path

> [String changes made/needed]
None
Attachment #8820508 - Flags: review+
Attachment #8820508 - Flags: approval-mozilla-aurora?
I'm seeing the following slowdowns on a browser build:

== Change summary for alert #4544 (as of December 17 2016 22:31 UTC) ==

Regressions:

197%  webaudio 0.2 Stereo Panning with Automation linux64 opt     357.08 -> 1062.17
 52%  kraken 1.1 audio-dft linux64 opt                            135.44 -> 205.69
 49%  massive 1.2 box2d-throughput-f32 linux64 opt                31473.8 -> 15982.12
 47%  massive 1.2 box2d-throughput linux64 opt                    28089.86 -> 15000.19
 41%  ss 1.0.2 math-partial-sums linux64 opt                      8.89 -> 12.53
 30%  unity-webgl 0.1 Particles linux64 opt                       121242.5 -> 84468.33
 28%  octane 2.0.1 Box2D linux64 opt                              48992.42 -> 35516
 26%  jetstream 1.0 box2d linux64 opt                             274.17 -> 202.8
  8%  massive 1.2 summary linux64 opt                             4358.91 -> 4024.49
  6%  kraken 1.1 summary linux64 opt                              1082.99 -> 1143.88

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=4544

The regression range is:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6f7b9683818ae3b5bc2ddb33460e7c75b6aec27c&tochange=98106f49ee61a352c18bd127554f375cddbc6f66

I'm not sure this regression is caused by this bug, but it looks like the only one I see that could have caused it. Can you take a look and see if you can reproduce this regression locally using treeherder builds and find out if this bug or another caused this?
Flags: needinfo?(arai.unmht)
(Assignee)

Comment 7

2 years ago
locally, I don't confirm any regression on that range.
I've tested with automation build for 98106f49ee61 and 95bb4ca0e110 and 6f7b9683818a, for linux64 opt, and also with mozregression with [2016-12-01,2016-12-20] range.
tested on Linux 3.16.0-4-amd64 #1 SMP Debian 3.16.36-1+deb8u2 (2016-10-19) x86_64 GNU/Linux
Flags: needinfo?(arai.unmht)
(Assignee)

Comment 8

2 years ago
forgot to note that I've tested Kraken, sunspider and Octane on browser.
Comment on attachment 8820508 [details] [diff] [review]
(mozilla-aurora) Check own flags property in RegExpPrototypeOptimizableRaw. r=h4writer

fix wrong behaviour in javascript, followup to bug 1322319, take in aurora52
Attachment #8820508 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 10

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/eee45ae5b76b
status-firefox52: affected → fixed
You need to log in before you can comment on or make changes to this bug.