Layout time for top 10k sites

[Eric Seidel]

This is a continuation of Tony's thread, focused on Layout.

I'm sending this to blink-dev instead of just the Google Blink-layout
folks in case others are interested.



I profiled [3] 5 of the top 10 sites listed as "slowest layout" in
Tony's top 10k:
https://docs.google.com/a/chromium.org/document/d/1ca_Q7xePmCRqaYnHe7vkpCmKNFNLdDXvzgtUPt9iG8w/edit#

They all have a very long delay (unclear entirely why) until they send onload.

During that time, they do a zillion layouts. [4]

I believe most of these layouts are caused by marquee/animation emulation:

http://icon.55bbs.com/homepage/js/index20120411_ys.js?v=1.5 see
rollText.run [1]
http://img1.cache.netease.com/cnews/js/ntes_ui/ntes_ui_slide_0.3.0_min.js
see _setPos [2]

Many of these layouts only layout the few elements invalidated.

But others do a full FrameView::layout(). (I'm not yet sure why?
Maybe the layout timer?)

When we do a FrameView::layout() that's when we get in trouble. We
get in trouble for 2 reasons:

1. We're calling updateCompositingLayersAfterLayout, which causes
eager compositing layer updates (even if the compositor may not need
them yet).

2. We're calling updateLayerPositionsAfterLayout, which unfortunately
uses clippedOverflowRectForRepaint to recursively walk up the
rendering tree. During layout() clippedOverflowRectForRepaint uses
layoutState() to quickly compute the value, but
updateLayerPositionsAfterLayout happens after layout() is done, and
doesn't get to use the layoutState() cache.

There is a 3rd bug implied from the above... why the heck are we doing
full FrameView::layout() calls in the first place? I believe that may
be the deferedLayoutTimer in action. Elliot/James have mentioned
before that we should only ever be doing compositor or JS-driven
layouts. It's not clear what is driving these full-page layouts, the
Inspector doesn't tell me.


I'm interested in your thoughts and feedback.

-eric


1. run: function(id, delay) {
var oLiFirst = this.liArr[id][0];
var liMarTop = oLiFirst.style.marginTop;
var liTopNum = parseInt(liMarTop);
var c = Math.abs(liTopNum);
if (c < parseInt(this.oUlH[id])) {
c++;
oLiFirst.style.marginTop = "-" + c + "px"
} else {
if (Math.abs(liTopNum) == parseInt(this.oUlH[id])) {
clearInterval(this.go[id]);
this.oParentUl[id].removeChild(oLiFirst);
this.oParentUl[id].appendChild(oLiFirst);
this.liArr[id][this.liArr[id].length -
1].style.marginTop = "0px";
this.timeout[id] = setTimeout(function(obj, id,
childtags, delay) {
return function() {
obj.start(id, childtags, delay)
}
}(this, id, this.childNode[id], delay), delay)
}
}
}

2. _setPos: function(e) {
var t = this;
t._bPos = e;
t._body.style.left = -t._bPos + "px";
if (t._handle) {
t._hPos = t.bhRate ? e / t.bhRate : 0;
t._handle.addCss(t._fix.pos + ":" + t._hPos + "px")
}
}

3. Profiling instructions, care of Tony:
https://docs.google.com/a/chromium.org/document/d/1_Zh6mLgGOhH_ccnahn-gTVD91xazQ4sG8asDbEgaSpY/edit

4. https://code.google.com/p/chromium/issues/detail?id=261439

[Elliott Sprehn]

My understanding is that "full document layout" just means we chose the document (or the document element) as the layoutRoot. Layout roots are chosen based on common ancestor stuff. You should look at what FrameView::scheduleRelayoutOfSubtree is actually doing.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2Bbb4fkeyqP--FkCOXzzfiFeDUO6ssaMM7pRU6%2BvCz5oDamk5w%40mail.gmail.com.

[Eric Seidel]

After looking at these profiles, I went off questing after
compositor-scheduled layouts as part of:
https://codereview.chromium.org/19782002/

Unfortunately when testing today, I found that my original deduction
that these were scheduling many layouts is wrong.

The change above *does* reduce the number of layouts on these sites
(for sina.com.cn it goes from 250 layouts to 96), but it doesn't
really make them spend much less time in layout.


I collected new data using the new _num statistic in loading_timeline:
https://docs.google.com/a/chromium.org/document/d/1IFMzdljBmDwG5--D6LIxN2q48h9K2o5VTqKhzXKFuUE/edit

You'll see that all of these sites actually just have a few
(relatively speaking) long (multi-second) layouts.

[Elliott Sprehn]

Scheduling fewer layouts means less tasks to execute in the SharedTimer heap though and fewer sets of invalidations and paints, so I still think it should be better for performance even if we still spend the same amount of time in ::layout().

[Eric Seidel]

It probably is. But I don't have good data to demonstrate that my
schedule-layouts-from-the-compositor change is a win, and w/o
compelling data I'm not ready to sign myself up for multiple weeks of
fixing the inevitable regressions it will cause. :)

I've moved back to profiling the top 10k sites and will work on
smaller bugs for a while.

[Eric Seidel]

After a long bout with various (mis-behaving) linux profiling tools, I
have finally found the culprit.

All of the top-10-worst-layout sites are spending all their time loading fonts:
https://code.google.com/p/chromium/issues/detail?id=266214

Only one of them even has anything else interesting in the profile. :)
We're spending sometimes 90% of our time under libfontconfig.

Hopefully this is a linux-only bug.

[Darin Fisher]

Correct me if I'm mistaken, but a side-effect of scheduling layouts from the compositor should be that they won't run in background tabs.  I'm presuming this since rAF doesn't fire in background tabs.  That alone might be worth the effort :-)

-Darin

[Eric Seidel]

Yes, there are definitely large potential benefits to finishing that
schedule-layouts-from-the-compositor patch. Unfortunately it's also
very likely to have a long tail of regressions (I already spent nearly
2 weeks just to get that far). Once I realized that my initial
analysis was wrong, and it would not actually help fix slowness of
Layout on these top-10k sites, I decided it was not worth committing
to landing it now.

I'll come back to the patch. But there are lower hanging fruit to fix
in layout first. :)

[Darin Fisher]

Makes sense :-)