Closed
Bug 792797
Opened 12 years ago
Closed 12 years ago
jank when opening blogspot webpage hangs browser UI
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: timbugzilla, Unassigned)
References
()
Details
(Keywords: perf, Whiteboard: [snappy])
Attachments
(2 files, 1 obsolete file)
10.26 KB,
text/plain
|
Details | |
5.70 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
Many blogspot.com blogs use the "Magazine" layout now. When opening these webpages there is a lot of jank, which makes the UI hang/become unresponsive for several seconds.
The effect is most pronounced on a low power system such as an AMD E-350 based laptop, and is also very noticable on more powerful systems (Intel E8500 system profiled here). The site loads more quickly and does not "jank" on the Chrome browser.
I've included a link to a profile here from the built in profiler. Most of the jank period seems to be spent in RegEx JS functions.
http://people.mozilla.com/~bgirard/cleopatra/?report=711677693f4fee4788544f96ffabb50772bf59c3
About:support is attached as a txt file.
Reporter | ||
Comment 1•12 years ago
|
||
This can be reproduced with a fresh profile (no add ons) and in safe mode.
Reporter | ||
Comment 2•12 years ago
|
||
Here's a profile taken during the jank/hang on an AMD E-350 based laptop:
http://people.mozilla.com/~bgirard/cleopatra/?report=f57e88426b6a64ce94d426062a646b9b63d0b9a3
Comment 3•12 years ago
|
||
Hmm I get
"Error in worker: Exception: SyntaxError: JSON.parse: expected ',' or '}' after property value in object (http://people.mozilla.com/~bgirard/cleopatra/js/parserWorker.js:244)"
end eventually
"About to start next task..."
when trying to load the Profiles.
Besides of that See also Bug 791627 where a similar Perf Issue has beend reported concerning an other View mode.
Reporter | ||
Comment 4•12 years ago
|
||
Here's a profile that you can access (others seem to be corrupted):
http://people.mozilla.com/~bgirard/cleopatra/?report=2cd6ccb3f7d8dc9f3db8190450dcd2fdbd60a476
Most of the Jank period is spent in the YARR interpreter.
Reporter | ||
Comment 5•12 years ago
|
||
Hmm, on Chome I am not seeing any errors during page loading, but on FF nightly I get a lot of entries in the error console such as the following:
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Unknown property '-moz-box-shadow'. Declaration dropped.
Source File: http://aordisco.blogspot.co.nz/
Line: 1132
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Unknown property 'box-sizing'. Declaration dropped.
Source File: http://aordisco.blogspot.co.nz/
Line: 1201
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Unknown property '-moz-border-radius'. Declaration dropped.
Source File: http://aordisco.blogspot.co.nz/
Line: 1376
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Unknown pseudo-class or pseudo-element '-webkit-scrollbar'. Ruleset ignored due to bad selector.
Source File: http://aordisco.blogspot.co.nz/
Line: 1530
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Unknown pseudo-class or pseudo-element '-webkit-scrollbar-button'. Ruleset ignored due to bad selector.
Source File: http://aordisco.blogspot.co.nz/
Line: 1537
Timestamp: 22/09/2012 1:30:05 p.m.
Warning: Error in parsing value for 'background-image'. Declaration dropped.
Source File: http://aordisco.blogspot.co.nz/
Line: 1667
Timestamp: 22/09/2012 1:30:07 p.m.
Warning: Unknown property 'mso-pagination'. Declaration dropped.
Source File: http://aordisco.blogspot.co.nz/
Line: 0
Comment 6•12 years ago
|
||
(In reply to timbugzilla from comment #5)
Those are unrelated.
Moving to JS for Investigation.
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → JavaScript Engine
Ever confirmed: true
Keywords: perf
Product: Firefox → Core
Reporter | ||
Comment 7•12 years ago
|
||
Is there any more info needed on this bug to move forward with resolving it?
Whiteboard: [snappy]
Reporter | ||
Comment 8•12 years ago
|
||
These (relatively common) pages still hang the browser. Any ideas for making progress on this?
Reporter | ||
Comment 9•12 years ago
|
||
This performance bug with blogspot sites been reported again as bug 826219.
https://bugzilla.mozilla.org/show_bug.cgi?id=826219
Reporter | ||
Comment 11•12 years ago
|
||
Had a look at this with the new built-in profiler in today's Nightly build.
The jank seems to take place in "magazine.js" (magazine.js:1, a JS:EvaluateString operation), which calls a function in "common.js" (l.match @ common.js:138) and spends all its time doing a regexp match method.
Comment 12•12 years ago
|
||
It would by handy to know if since bug 829758 landed, the situation changed/improved?
Reporter | ||
Comment 13•12 years ago
|
||
No improvement with the 02/06 nightly I'm afraid.
Reporter | ||
Comment 14•12 years ago
|
||
An observation is that memory use spikes up about 120MB while opening the page, then drops back to normal.
Comment 15•12 years ago
|
||
1) Is there something known about on which systems this happens, as me and Sean cannot reproduce this.
2) Is there a regression range? Or do you know since when this problem started occurring?
Comment 16•12 years ago
|
||
I cannot reproduce the bad behavior locally, but I would appreciate testing of this patch by someone who can. This permits the Yarr interpreter to handle interrupts, which should remove browser unresponsiveness but not change execution time.
Reporter | ||
Comment 17•12 years ago
|
||
I'd be happy to test a try build.
(In reply to Sean Stangl from comment #16)
> Created attachment 711084 [details] [diff] [review]
> Handle operation callbacks within the Yarr interpreter
>
> I cannot reproduce the bad behavior locally, but I would appreciate testing
> of this patch by someone who can. This permits the Yarr interpreter to
> handle interrupts, which should remove browser unresponsiveness but not
> change execution time.
Reporter | ||
Comment 18•12 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #15)
> 1) Is there something known about on which systems this happens, as me and
> Sean cannot reproduce this.
>
It is particularly noticeable on a low powered laptop (AMD E-350 based). It is also noticeable on an Intel E8500 based system. I can reproduce it with win7 x86, win7 x64, and win8 x64 OS's.
> 2) Is there a regression range? Or do you know since when this problem
> started occurring?
I think that it's always been this way with the "magazine" style layouts on blogspot.
Comment 19•12 years ago
|
||
Try run for ddf015925b17 is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=ddf015925b17
Results (out of 9 total builds):
success: 8
warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sean.stangl@gmail.com-ddf015925b17
Reporter | ||
Comment 20•12 years ago
|
||
The try build is slightly more responsive. Now if I click on another tab, I don't get the "program not responsive" message in the window titlebar, and the tab switches about halfway through the "spinning gears" animation. Before tab switching would wait until after "spinning gears" animation.
Comment 21•12 years ago
|
||
I can reproduce this at home on a slow computer. The patch in Comment 16 does help, but the real problem is this horrible code in common.js:139:
> if (a instanceof RegExp && 0 === c.search(a))
> return c.match(a)[0]
Apparently this |a| is a regular expression that captures an unholy amount of subexpressions and is therefore uncompilable. So, not only is this code executing the costly regular expression multiple times, but also it generates the full matchset for the expression (i.e., the data structure that takes up hundreds of megs) then immediately discards it, extracting only the first result.
There are several things in play here:
1) str_search() can currently only use MatchOnly mode when the code is compilable. We should write MatchOnly support for the interpreter, so that it can bail out early after the first match is found.
2) str_match() is written in terms of DoMatch(), which requires updating the RegExpStatics -- in this case, a huge data structure. It should be lazified.
3) The code |str.match(r)[0]| is probably fairly common. We could try detecting it.
Comment 22•12 years ago
|
||
(In reply to timbugzilla from comment #20)
> The try build is slightly more responsive. Now if I click on another tab, I
> don't get the "program not responsive" message in the window titlebar, and
> the tab switches about halfway through the "spinning gears" animation.
> Before tab switching would wait until after "spinning gears" animation.
Thanks. Since the patch makes the responsiveness better, let's try to land it now (hopefully promoting to aurora) and address the issues in Comment 21 in follow-up bugs.
Reporter | ||
Comment 23•12 years ago
|
||
Thanks for sorting this out!
If you are interested in another google property that janks while loading, I've filed a bug for jank when opening google groups pages. An Evaluate String operation seems to make up most of the jank.
https://bugzilla.mozilla.org/show_bug.cgi?id=793359
Comment 24•12 years ago
|
||
This is the patch that was included in the Try run, which reduces jank noticeably.
Attachment #711084 -
Attachment is obsolete: true
Attachment #711451 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #711451 -
Flags: review?(dvander) → review+
Comment 25•12 years ago
|
||
Comment 26•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 27•12 years ago
|
||
(In reply to Sean Stangl from comment #22)
> Thanks. Since the patch makes the responsiveness better, let's try to land
> it now (hopefully promoting to aurora) and address the issues in Comment 21
> in follow-up bugs.
Do you have the bug numbers for the follow up bug reports ?
Comment 28•12 years ago
|
||
(In reply to Matthias Versen [:Matti] from comment #27)
> (In reply to Sean Stangl from comment #22)
> > Thanks. Since the patch makes the responsiveness better, let's try to land
> > it now (hopefully promoting to aurora) and address the issues in Comment 21
> > in follow-up bugs.
>
> Do you have the bug numbers for the follow up bug reports ?
I just filed Bug 841615 for MatchOnly str_match(). The other suggestions are most likely not going to be worked on given their complexity and low impact, so filing bugs on them would be disingenuous. I would suggest fixing str_match() first, then coming back to evaluate whether other options are necessary.
It might be worthwhile to file a bug for MatchOnly interpretation on the WebKit tracker, though, as they are the upstream for the regexp engine (YARR) and may have interest in making the change themselves.
You need to log in
before you can comment on or make changes to this bug.
Description
•