30-50ms blocked in commit is because it's waiting on the pending tree to finish rasterizing so it can activate.  It's a commit pipeline stall because of the decode time.

The background image isn't being cached by our image decode system in cc because it's a background-image and that's a current hole in how we detect images.  It *should* be cached in Skia, however.  It's strange to me that it's not, but maybe there's cache contention or it's doing something that's preventing it from being cached.

fmalita/vmpstr: Do you know why this image wouldn't be cached in Skia?

Some background images are converted to draw image calls, so it's possible that we're capturing them as well. Off the top of my head, we don't cache things if they are too big, so maybe this is hitting that limit. I'll take a look.

enne@ Should I file a separate bug for the commit stall? We shouldn't block main like that, it's just as bad as doing a sync IPC, it's blocking network request handling, low memory notifications, the preload scanner and more.

esprehn: you should talk to sunnyps@ about it and file a bug.  This is related to the "begin main frame before activation work".  We used to not send the begin main frame and start a commit until activation happened, to leave the main thread free.  However, this decreases the amount of pipelining you can do.  That said, if you send the begin main frame too early, then the main thread stalls.  Maybe it's possible we could use the history of how long historical activation times have been to know when we should send the begin main frame message.

Can we make the commit async? We just need to make sure not to progress through the paint code until the commit has finished I think. But JS, style and layout could run.

On the image front, it seems that how this animation is happening is by varying the src_rect and upscaling it to some size. However, it turns out that each of the src_rects and upscale size is different each frame. So, although the original image is decoded in memory, the upscaling using a high filter quality is taking up a bulk of the work and it can't be reused since the values are different. Here are three examples (visible in the SoftwareImageDecodeController::DecodeImage slices):

id[302] src_rect[0,0 1280x365] target_size[2530x721], then
id[302] src_rect[0,0 1280x388] target_size[2530x767], then
id[302] src_rect[0,0 1280x394] target_size[2530x779]

As far as SIDC knows these are distinct cache entries, although the id is the same. I'm going to grab a trace with cc.debug as well, since it might make it a bit more clear where the time is spent.

I'm not fond of adding a heuristic for this.

Re: making commit async, there are two ways of doing this:
1. main thread sends commit message to compositor thread, blocks updates to layer tree / shared state until commit completes (what ojan is proposing I think)
2. main thread sends commit ready message to compositor thread, compositor performs synchronous commit i.e. compositor thread blocks for main thread

skyostil/alexclarke: for option 2 can blink scheduler guarantee that we don't block compositor thread for a long time?

Yeah, I think it would be good to clear up the page lifecycle to avoid a stall.  

Even without MFBA, ProxyMain::BeginMainFrame::commit() is a synchronous IPC and can block several ms, as in the attached trace.

Deleted: trace_leaves-complexity-100-linux.json.gz 2.8 MB

trace_leaves-complexity-100-linux.json.gz 2.8 MB Download

I propose we split this patch into two; One to investigate the images, one for async commit. I've filed  crbug.com/664251  to track the image component.

s/patch/bug/

vmiura: BeginMainFrame::commit taking a long time on complicated pages is a separate issue and has existed for a very long time.  There's a tradeoff between doing extra work to make the commit asynchronous and just doing it synchronously when it's usually fast.  I think we're already pretty committed (<_<) to making it asynchronous in a future mus world.  Personally, I don't think it's a high priority to change this in the short term.

The difference with MFBA is that now BeginMainFrame is blocked on activation of the previous frame, so with very slow decodes and raster it becomes ten times worse, which seems like a much more real problem.

At any rate, I think it should be a different bug as well if you want to talk about that.

enne: I agree this is a much bigger problem.  I just assumed the solution could be the same.

I guess I think the historic timing approach seems a bit brittle, and would like to understand the async commit options.

I think we need both the activation time heuristic and an async commit of some sort. Otherwise, even with an async commit, we will end up doubling an already large latency.