792797 - jank when opening blogspot webpage hangs browser UI

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[timbugzilla]

Attachment: about:support as txt file

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.

Comment 1

[timbugzilla]

This can be reproduced with a fresh profile (no add ons) and in safe mode.

Comment 2

[timbugzilla]

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

[(mostly gone) XtC4UaLL [:xtc4uall]]

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.

Comment 4

[timbugzilla]

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.

Comment 5

[timbugzilla]

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

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to timbugzilla from comment #5)
Those are unrelated.

Moving to JS for Investigation.

Comment 7

[timbugzilla]

Is there any more info needed on this bug to move forward with resolving it?

Comment 8

[timbugzilla]

These (relatively common) pages still hang the browser. Any ideas for making progress on this?

Comment 9

[timbugzilla]

This performance bug with blogspot sites been reported again as bug 826219. 

https://bugzilla.mozilla.org/show_bug.cgi?id=826219

Comment 11

[timbugzilla]

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

[Hannes Verschore [:h4writer]]

It would by handy to know if since bug 829758 landed, the situation changed/improved?

Comment 13

[timbugzilla]

No improvement with the 02/06 nightly I'm afraid.

Comment 14

[timbugzilla]

An observation is that memory use spikes up about 120MB while opening the page, then drops back to normal.

Comment 15

[Hannes Verschore [:h4writer]]

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

[Sean Stangl [:sstangl]]

Attachment: 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.

Comment 17

[timbugzilla]

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.

Comment 18

[timbugzilla]

(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

[Mozilla RelEng Bot]

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/[e-mail]-ddf015925b17

Comment 20

[timbugzilla]

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

[Sean Stangl [:sstangl]]

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

[Sean Stangl [:sstangl]]

(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.

Comment 23

[timbugzilla]

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

[Sean Stangl [:sstangl]]

Attachment: Handle interrupts in the Yarr interpreter.

This is the patch that was included in the Try run, which reduces jank noticeably.

Comment 25

[Sean Stangl [:sstangl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad5221d0562

Comment 26

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7ad5221d0562

Comment 27

[Matthias Versen [:Matti]]

(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

[Sean Stangl [:sstangl]]

(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.