1081175 - (latin1strings) Degraded regular expression performance (infinite loop?)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[James Keane]

Attachment: Degraded regular expression

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.101 Safari/537.36

Steps to reproduce:

Google Caja CSS lexer causes a browser lock up on FIrefox 33 for sufficiently large css files.

See related Caja issue: https://code.google.com/p/google-caja/issues/detail?id=1941


Actual results:

Using the provided example, the time required to lex the bootstrap css (on my machine):

Chrome 38: ~50ms
Firefox 32: ~4ms
Firefox 33 forced unicode: ~8ms
Firefox 33 without forced unicode: _never completes_.


Expected results:

Firefox 33 should have similar performance as Firefox 32.

Comment 1

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]: Serious web compat regression...

Comment 2

[James Keane]

I have reduced the test case to the absolute minimum.

It is definitely a performance regression as increasing the input size shows exponential increase in time.

Comment 3

[James Keane]

Attachment: Reduced example

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Shell testcase

Thanks for the bug report and the great testcase!

It looks like this is an issue in the regex engine, will investigate more.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

One-line fix for a bug introduced when irregexp was imported (bug 976446). We were appending to a vector instead of replacing the old elements, like the original code does.

The problem is in Latin1-only code though, so it was not a problem until Latin1 strings were added to SpiderMonkey.

Comment 6

[Jan de Mooij [:jandem]]

[Tracking Requested - why for this release]:

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc10fcd30554

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8503597 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Older bug exposed by bug 998392.
[User impact if declined]: Very slow websites/apps, hangs.
[Describe test coverage new/current, TBPL]: Patch adds a testcase.
[Risks and why]: Low risk - fix is a one-liner and it matches the code upstream.
[String/UUID change made/needed]: None.

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8503597 [details] [diff] [review]
Patch

We're too lage for Firefox 33, which ships on Tuesday. Let's take this in Firefox 34 after it lands on m-c.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dc10fcd30554

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8503597 [details] [diff] [review]
Patch

Aurora+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b82fa46ca35a

Comment 13

[Kohei Yoshino [:kohei]]

Nominating for 33.1 as the patch is very small and low risk.

Comment 14

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/Firefox/Releases/33/Site_Compatibility#JavaScript

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Sorry but we only take critical issues for 33.1...
This will wait for 34.