Bug 56842 - Performance regression: Blocking inline scripts on external sheets
Summary: Performance regression: Blocking inline scripts on external sheets
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 45072
Blocks: 8852
  Show dependency treegraph
 
Reported: 2011-03-22 10:39 PDT by Tony Gentilcore
Modified: 2011-06-06 21:37 PDT (History)
12 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Gentilcore 2011-03-22 10:39:21 PDT
http://trac.webkit.org/changeset/74995/ seems to be causing significant page load time regressions for several Google properties. More data is still coming in, but certain Google properties load 10-20% slower after this behavior change. The regression was noticed in Chrome 10, and I've isolated it to this change in local builds.

As explained in https://bugs.webkit.org/show_bug.cgi?id=51684#c9, this change was made for better web compat. So it may not be possible or a good idea to simply roll back even though the actual perf hit for some sites was much bigger than anticipated. The purpose of this bug is to find a way to mitigate this perf regression.

Here are some ideas:

1. When a script is blocked on a stylesheet, we could allow it to be compiled while still waiting on the stylesheet. Currently we block both compilation and execution. As far as I can tell, this is an orthogonal optimization that would be perfectly safe but probably wouldn't recover most of the regression.
2. Rather than blocking scripts on stylesheet loads, we allow them to execute, but then block their execution when they call out for layout. Since this involves hitting the event loop in a new place, this could be tricky or impossible to get right. However, the win would be nice as I suspect this would allow most scripts to not block on stylesheets at all.
3. Currently we block scripts on styles whether the script is in the head or body. Unless I'm missing something, this blocking behavior should only be required for scripts in the body as layout isn't applicable while parsing the head. If this change works, it would recover the regression from at least one case that I'm aware of.
4. Currently the preload scanner doesn't operate while parsing the head. This makes it especially bad to block on a script in the head. If we can fix #3, this would be moot. I also suspect this would help in many other cases. (filed as bug 45072)

Thoughts? Other ideas?
Comment 1 Eric Seidel (no email) 2011-03-22 10:53:33 PDT
(In reply to comment #0)

> 2. Rather than blocking scripts on stylesheet loads, we allow them to execute, but then block their execution when they call out for layout. Since this involves hitting the event loop in a new place, this could be tricky or impossible to get right. However, the win would be nice as I suspect this would allow most scripts to not block on stylesheets at all.

Isn't this how our behavior used to work?

> 3. Currently we block scripts on styles whether the script is in the head or body. Unless I'm missing something, this blocking behavior should only be required for scripts in the body as layout isn't applicable while parsing the head. If this change works, it would recover the regression from at least one case that I'm aware of.

Possibly.  But you'd still get incorrect results if you tried to crawl the CSSOM during that script.
Comment 2 Tony Gentilcore 2011-03-22 11:02:52 PDT
(In reply to comment #1)
> (In reply to comment #0)
> 
> > 2. Rather than blocking scripts on stylesheet loads, we allow them to execute, but then block their execution when they call out for layout. Since this involves hitting the event loop in a new place, this could be tricky or impossible to get right. However, the win would be nice as I suspect this would allow most scripts to not block on stylesheets at all.
> 
> Isn't this how our behavior used to work?

Oh, did this change w/ the html5 parser? Are there any bugs that explain the rationale for the change?

> 
> > 3. Currently we block scripts on styles whether the script is in the head or body. Unless I'm missing something, this blocking behavior should only be required for scripts in the body as layout isn't applicable while parsing the head. If this change works, it would recover the regression from at least one case that I'm aware of.
> 
> Possibly.  But you'd still get incorrect results if you tried to crawl the CSSOM during that script.

Yeah, that is unfortunate because it is presumably a very uncommon operation. Do you think there's any way we could detect calling out to the cssom and safely block in only that case? If not, I'm not sure how to weigh breaking that assumption vs taking a perf hit in so many cases where it isn't necessary.

BTW, do any other approaches happen to come to mind?
Comment 3 Tony Gentilcore 2011-03-22 11:13:44 PDT
BTW, here's a test case. It loads in ~2s without the change and ~4s with the change.
http://stevesouders.com/cuzillion/?c0=hc1hfff2_0_f&c1=hb0hfff0_0_f&c2=bi1hfff2_0_f&t=1300817486
Comment 4 Eric Seidel (no email) 2011-03-22 11:15:20 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > 
> > > 2. Rather than blocking scripts on stylesheet loads, we allow them to execute, but then block their execution when they call out for layout. Since this involves hitting the event loop in a new place, this could be tricky or impossible to get right. However, the win would be nice as I suspect this would allow most scripts to not block on stylesheets at all.
> > 
> > Isn't this how our behavior used to work?
> 
> Oh, did this change w/ the html5 parser? Are there any bugs that explain the rationale for the change?

I guess I'm mixing words.  We always have lazily caused a layout when a script accesses a property requiring layout.

We've never actually blocked the operation while waiting for a load.

> > > 3. Currently we block scripts on styles whether the script is in the head or body. Unless I'm missing something, this blocking behavior should only be required for scripts in the body as layout isn't applicable while parsing the head. If this change works, it would recover the regression from at least one case that I'm aware of.
> > 
> > Possibly.  But you'd still get incorrect results if you tried to crawl the CSSOM during that script.
> 
> Yeah, that is unfortunate because it is presumably a very uncommon operation. Do you think there's any way we could detect calling out to the cssom and safely block in only that case? If not, I'm not sure how to weigh breaking that assumption vs taking a perf hit in so many cases where it isn't necessary.

We could certainly detect attempts to access the stylesheet list when not all sheets have loaded.  However we have no way to block execution.  We don't run inner runloops in WebCore, and as far as I know we have no way to exit out of a running script and resume it later (but maybe that's possible these days with our debugger support.
Comment 5 Antti Koivisto 2011-03-22 13:36:45 PDT
Maybe these pages get hit hard because they have been specifically optimized for the old behavior? I haven't seen signs of significant regressions from this in the web in general. It should be possible to adapt the pages for the new behavior too.
Comment 6 James Robinson 2011-03-22 13:44:52 PDT
(In reply to comment #5)
> Maybe these pages get hit hard because they have been specifically optimized for the old behavior? I haven't seen signs of significant regressions from this in the web in general. It should be possible to adapt the pages for the new behavior too.

I suspected it's more likely that these pages have always loaded slower than they could have in other browsers and the fact that WebKit has changed behavior caused the performance difference between the behaviors to be highlighted.