Closed
Bug 1081175
Opened 10 years ago
Closed 10 years ago
(latin1strings) Degraded regular expression performance (infinite loop?)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: james.keane, Assigned: jandem)
Details
(Keywords: dev-doc-complete, regression, site-compat)
Attachments
(3 files, 1 obsolete file)
1.14 KB,
text/html
|
Details | |
853 bytes,
application/x-javascript
|
Details | |
1.58 KB,
patch
|
bhackett1024
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•10 years ago
|
Summary: (latin1strings) Degraded regular expression performance → (latin1strings) Degraded regular expression performance (infinite loop?)
Comment 1•10 years ago
|
||
[Tracking Requested - why for this release]: Serious web compat regression...
Status: UNCONFIRMED → NEW
tracking-firefox33:
--- → ?
Ever confirmed: true
Flags: needinfo?(jdemooij)
Keywords: regression
Updated•10 years ago
|
Version: unspecified → 33 Branch
Reporter | ||
Comment 2•10 years ago
|
||
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.
Reporter | ||
Comment 3•10 years ago
|
||
Attachment #8503234 -
Attachment is obsolete: true
Assignee | ||
Comment 4•10 years ago
|
||
Thanks for the bug report and the great testcase! It looks like this is an issue in the regex engine, will investigate more.
Assignee | ||
Comment 5•10 years ago
|
||
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)
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 6•10 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox32:
--- → unaffected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
Assignee | ||
Updated•10 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Updated•10 years ago
|
Attachment #8503597 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc10fcd30554
Assignee | ||
Comment 8•10 years ago
|
||
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.
Attachment #8503597 -
Flags: approval-mozilla-beta?
Attachment #8503597 -
Flags: approval-mozilla-aurora?
Comment 9•10 years ago
|
||
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-
Updated•10 years ago
|
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dc10fcd30554
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 11•10 years ago
|
||
Comment on attachment 8503597 [details] [diff] [review] Patch Aurora+
Attachment #8503597 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•10 years ago
|
||
Nominating for 33.1 as the patch is very small and low risk.
Keywords: site-compat
Comment 14•10 years ago
|
||
https://developer.mozilla.org/en-US/Firefox/Releases/33/Site_Compatibility#JavaScript
Keywords: dev-doc-complete
Comment 15•10 years ago
|
||
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.
Description
•