Closed Bug 1081175 Opened 5 years ago Closed 5 years ago
(latin1strings) Degraded regular expression performance (infinite loop?)
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.
Summary: (latin1strings) Degraded regular expression performance → (latin1strings) Degraded regular expression performance (infinite loop?)
[Tracking Requested - why for this release]: Serious web compat regression...
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.
Thanks for the bug report and the great testcase! It looks like this is an issue in the regex engine, will investigate more.
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8503597 - Flags: review?(bhackett1024)
[Tracking Requested - why for this release]:
Attachment #8503597 - Flags: review?(bhackett1024) → review+
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 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.
Attachment #8503597 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8503597 [details] [diff] [review] Patch Aurora+
Attachment #8503597 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Nominating for 33.1 as the patch is very small and low risk.
Sorry but we only take critical issues for 33.1... This will wait for 34.
You need to log in before you can comment on or make changes to this bug.