It even seems janky on Mac m56 Canary.

Adding more compositing folks who might be able to comment on frame scheduling.

The animation is done with 2 transitions. One transitions the margin-top of the topmost bar from 0 to -65, thus hiding it, while the other transitions the width,visibility and  opacity of the "G" on the second row to make it appear to grow in/out from the left. The upshot of all this is a lot of flexbox layout changes, I think.

Maybe the cost is in the width animation on the "G".

That "G" is SVG, so resizing will re-raster, which means re-layout because SVG-as-Image does layout for raster.

cc folks for helping triaging Internals>Compositing>Rasterization.

This looks like it's using software raster, as there are not GpuRasterBuffer::Playback traces and are instead OneCopyRasterBuffer::Playback traces.

I think the expanded viewport trigger went into m54 (https://codereview.chromium.org/2161663002), however there's still a content veto for too many paths: https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkPictureAnalyzer.cpp?l=19

I'm not sure what you mean by "descheduling the frame pumping".  Frames in which process/thread? It looks like the browser is ticking regularly.  The compositor thread is mostly waiting on pending trees for rasterization, which backs up the begin frames being sent to the main thread, if that's what you mean.  If there wasn't a rasterization bottleneck in the pipeline, then I think frames would tick more regularly.

This seems like an optimization issue to me, and it's not clear we can optimize if the problem is the animating width on SVG-as-image.

Rasterization is only one of the issues I mentioned in the original post. It's spending a lot of time in UpdateLayerTree, which has nothing to do with animating width on SVGs, does it?

That at least should be fixable. It might depend on SlimmingPaint v2, but it shouldn't be a fundamental cost.

By "descheduling the frame pumping" I meant that the raster threads were doing a lot of work that descheduled the main thread rendering pipeline (presumably because all the cores were busy rastering?).

ojan@ Can you force Ganesh in about:flags and take another trace? I'm very curious how saturating the cores with raster work compares to doing the paths on the GPU, even if it's slower, since it means the main thread doesn't get descheduled as badly. :)

Separately the UpdateLayerTree and synchronizedPaint taking 6ms each (that's 12ms of just rendering pipeline if I understand you right?) is pretty terrible. Someone on the Paint team should look at that in a profiler, it's super easy! Just follow the guide in:
http://x20web/~esprehn/chrome_profiling.md.html

The animation seems much better by forcing Ganesh on.  Removing the Ganesh veto is on our OKRs, but I'm surprised we veto to Software Raster on Nexus 5x instead of switching to MSAA mode.  senorblanco@ would you mind looking into the veto?  Is MSAA black-listed on that driver?

schenney@: Based on #1/#7 it sounds like there's a surprising amount of paint invalidation going on.  Is the time spent painting the SVG, or is the rest of the content getting re-laid out?

Running this on a 5X reveals:

"Adreno 4xx support for EXT_multisampled_render_to_texture is buggy on Android 7.0".

This is https://bugs.chromium.org/p/chromium/issues/detail?id=612474. That will definitely prevent MSAA usage on a 5X on 7.0, so we veto-to-software instead. I seem to recall that this driver bug only affected 7.0 (not pre-7.0), and was going to be fixed and unblacklisted in a followup 7.x release. aelias/boliu, do you know if there's been any progress on that?

So story for adreno 4xx and 5xx is a bit different.

4xx (like nexus5x) is fine on android 6 (M), but android 7 (first version of N) broke this, and we didn't notice it in time to have the fix in 7. It will be fixed in android 7.1, whenever that goes out.

5xx, we noticed crashes from both the pixel phones (android 7) and samsung s7 (android 6). We did notice the crash in time so that the issue is fixed in the initial N release on the pixel, so we only blacklisted android 6

There is no work to do on chrome side. When 7.1 goes out, 4xx will become unblacklisted.

#11: Sigh...

Can we block this issue on the SSAA-path-renderer?

Re: #11: thanks for the update!

Re: #12: it's not clear yet whether the tessellating SSAA path renderer will outperform software raster on Android (it may, but I have no data yet). I think tiling MSAA is still a better long-term solution on Android, so if possible we should just wait for 7.1.

If there is an invalidation issue here, I think we should investigate that rather than just papering over the problem with GPU raster. (If not, we can just wait for Android 7.1 and/or tess SSAA). Bouncing to schenney@ to see if he wants to investigate that.

Commit bb1c8b9d3e4acb60fb489f878963e2ccbb936b61 looks like it had the intended
effect of reducing paint time quite a lot - up to 25%.

Sending back to senorblanco for the path raster tasks.

 Issue 720214  has been merged into this issue.

The paths veto has been removed, and the edge-AA tessellating path renderer now kicks in for a portion of the Google logo. Other parts exceed the 10-verb-limit and are presumably drawn using the default software-and-upload path renderer. I see no appreciable improvement in jank by removing the verb limit, and performance is about the same regardless of which path renderer is used. I doubt that path rendering is impacting this page much at all.

I note that it is equally janky on Safari and Firefox, but quite smooth on IE11 (maybe due to missing web platform features?)

All that said, I don't plan any further work here, so I'm bouncing it back to Chris to see if something can be done about layerization.

Attaching a trace from the Pixel XL phone.  I am seeing two raster issues that need investigation.

1) The first raster task when initiating the animation is > 23ms.

What is taking so much time in Skia?  Software path rendering?  Edge-AA?  Generating glyph cache?

2) The GPU thread has long calls to GLES2DecoderImpl::ClearLevel which total > 20ms.  These are from glDrawElements   These should be avoided by fully clearing all render surfaces, and fully initializing texture levels before use.

The pattern of three clears in a row, could mean we have uninitialized mip-maps.

Deleted: trace_google-blog.json 5.5 MB

trace_google-blog.json 5.5 MB View Download

Re comment #19

I'm testing on Pixel Xl with URL: https://blog.google/topics/connected-workspaces/chrome-os-joins-forces-vmware-accelerate-adoption-chromebooks-enterprise/.  I think the following is happening:

1) When the page is idle for 1 second we purge Ganesh's resource cahce and the GLES2 command buffer resources.  Subsequently the next header animation has relatively slow raster tasks as it's re-building caches.  

The attached trace shows the header animation 3 times.

* 1st scroll is after idling, and is slow with > 20ms Raster tasks and GPU thread time / frame.
* 2nd time is soon after the 1st, the frame rate is steady.  Raster tasks average ~5ms / frame, and GPU thread spends 6~7ms / frame.
* 3rd time after a second of idling, you can see us purging resources at time 6,121.537 ms.  Then the animation behaves like the 1st time.

So a question is, are we maybe being too aggressive at purging resources on foreground tabs on this kind of /high memory/ device?

2) The GLES2DecoderImpl::ClearLeve are happening when Ganesh initializes some atlas (glyph cache?).

It's a 1024x2048x16bit texture.  Ganesh allocates the texture, and uploads a small area with glTexSubImage2D and the rest of the texture is undefined memory.  Subsequently when drawing with the texture, the GLES2 Command Buffer clears the undefined areas to black, in a kind of slow way (malloc up to 1MB, clear it to zero, glTexSubImage2D to uninitialized parts of the texture).  It would be better if Ganesh cleared the atlas first with glClear.

Deleted: trace_google-blog-resource-purging.json 23.6 MB

trace_google-blog-resource-purging.json 23.6 MB Download

Ganesh currently does not create FBOs for the glyph atlases because they're never rendered to. Could the command buffer clear using glClear rather than uploading buffers full of zeros? It seems like we should never use the texsubimage2d code for renderable formats including for WebGL.

Yeah, I think it may be possible for us to clear it with glClear though I think it would be less efficient than doing it upfront, as we have to do it in multiple parts.  On a tiler especially I think clearing it first before it has any content would be the most memory bandwidth efficient.

The above sounds good. Additionally, I'll send out a patch today to use the new Skia cache clearing API, which I think will preserve the glyph atlas... May still want to do a full clear on low-end, but maybe even that is ok now that we use smaller textures there.

That's a good point about partial vs full clear.

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/45e5068a6d10f4e4fd4658824310f8871f02ccf7

commit 45e5068a6d10f4e4fd4658824310f8871f02ccf7
Author: Brian Salomon <bsalomon@google.com>
Date: Tue May 23 18:07:25 2017

Add a flag to GrSurfaceFlags that requires the texture to be cleared upon creation. 

Bug: chromium:656320

Change-Id: I940bfa24540516ab83a2ed52f761b96eb6ad19f1
Reviewed-on: https://skia-review.googlesource.com/17391
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/GrSurfaceProxy.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/include/gpu/GrTypes.h
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/gl/GrGLCaps.h
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/vk/GrVkGpu.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/include/gpu/gl/GrGLFunctions.h
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/tests/GrSurfaceTest.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/gl/GrGLGpu.h
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/GrResourceProvider.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/gl/GrGLAssembleInterface.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/gl/GrGLGpu.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/include/private/GrSurfaceProxy.h
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/src/gpu/GrGpu.cpp
[modify] https://crrev.com/45e5068a6d10f4e4fd4658824310f8871f02ccf7/include/gpu/gl/GrGLInterface.h

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/a9e795eab5f59a52d96b8fdc39351452835f5eb9

commit a9e795eab5f59a52d96b8fdc39351452835f5eb9
Author: Brian Salomon <bsalomon@google.com>
Date: Tue May 23 19:03:01 2017

Revert "Add a flag to GrSurfaceFlags that requires the texture to be cleared upon creation. "

This reverts commit 45e5068a6d10f4e4fd4658824310f8871f02ccf7.

Reason for revert: :'(

Original change's description:
> Add a flag to GrSurfaceFlags that requires the texture to be cleared upon creation. 
> 
> Bug: chromium:656320
> 
> Change-Id: I940bfa24540516ab83a2ed52f761b96eb6ad19f1
> Reviewed-on: https://skia-review.googlesource.com/17391
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> 

TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug: chromium:656320

Change-Id: I8a4f71537e45f3c4cf37b10b2dc8ee38fe6959ba
Reviewed-on: https://skia-review.googlesource.com/17765
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/GrSurfaceProxy.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/include/gpu/GrTypes.h
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/gl/GrGLCaps.h
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/vk/GrVkGpu.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/include/gpu/gl/GrGLFunctions.h
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/tests/GrSurfaceTest.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/gl/GrGLGpu.h
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/GrResourceProvider.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/gl/GrGLAssembleInterface.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/gl/GrGLGpu.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/include/private/GrSurfaceProxy.h
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/src/gpu/GrGpu.cpp
[modify] https://crrev.com/a9e795eab5f59a52d96b8fdc39351452835f5eb9/include/gpu/gl/GrGLInterface.h

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/8785df15fe5a57651597d138d3c5aea0ffe3f661

commit 8785df15fe5a57651597d138d3c5aea0ffe3f661
Author: Brian Salomon <bsalomon@google.com>
Date: Wed May 24 16:04:45 2017

Clear atlas textures at creation in Chrome

Bug: chromium:656320
Change-Id: Ia65274aa733f199be188579821e745920493aefc
Reviewed-on: https://skia-review.googlesource.com/17824
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/8785df15fe5a57651597d138d3c5aea0ffe3f661/include/gpu/GrCaps.h
[modify] https://crrev.com/8785df15fe5a57651597d138d3c5aea0ffe3f661/src/gpu/GrDrawOpAtlas.cpp
[modify] https://crrev.com/8785df15fe5a57651597d138d3c5aea0ffe3f661/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/8785df15fe5a57651597d138d3c5aea0ffe3f661/src/gpu/GrCaps.cpp

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/1c214313248a4b5a69af14539608c54fb67c2bf8

commit 1c214313248a4b5a69af14539608c54fb67c2bf8
Author: Brian Salomon <bsalomon@google.com>
Date: Thu May 25 20:03:07 2017

Revert "Clear atlas textures at creation in Chrome"

This reverts commit 8785df15fe5a57651597d138d3c5aea0ffe3f661.

Reason for revert: Chrome bug
Bug:  chromium:726226 

Original change's description:
> Clear atlas textures at creation in Chrome
> 
> Bug: chromium:656320
> Change-Id: Ia65274aa733f199be188579821e745920493aefc
> Reviewed-on: https://skia-review.googlesource.com/17824
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> 

TBR=bsalomon@google.com,robertphillips@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: chromium:656320

Change-Id: Ibc4dd7f184866b58481f2dc7e7e88da9169e7adc
Reviewed-on: https://skia-review.googlesource.com/17988
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/1c214313248a4b5a69af14539608c54fb67c2bf8/include/gpu/GrCaps.h
[modify] https://crrev.com/1c214313248a4b5a69af14539608c54fb67c2bf8/src/gpu/GrDrawOpAtlas.cpp
[modify] https://crrev.com/1c214313248a4b5a69af14539608c54fb67c2bf8/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/1c214313248a4b5a69af14539608c54fb67c2bf8/src/gpu/GrCaps.cpp

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/89ca3a3b0210355a40bc0bc0ffd0104f91b67997

commit 89ca3a3b0210355a40bc0bc0ffd0104f91b67997
Author: Brian Salomon <bsalomon@google.com>
Date: Thu May 25 20:28:48 2017

Revert "Clear atlas textures at creation in Chrome"

This reverts commit 8785df15fe5a57651597d138d3c5aea0ffe3f661.

Reason for revert: Chrome bug
Bug:  chromium:726226 

Original change's description:
> Clear atlas textures at creation in Chrome
>
> Bug: chromium:656320
> Change-Id: Ia65274aa733f199be188579821e745920493aefc
> Reviewed-on: https://skia-review.googlesource.com/17824
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
>

TBR=bsalomon@google.com,robertphillips@google.com
Bug: chromium:656320

No-Tree-Checks: true
No-Try: true
No-Presubmit: true
Change-Id: Ibc4dd7f184866b58481f2dc7e7e88da9169e7adc
Reviewed-On: https://skia-review.googlesource.com/17988
Reviewed-By: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-on: https://skia-review.googlesource.com/17991

[modify] https://crrev.com/89ca3a3b0210355a40bc0bc0ffd0104f91b67997/include/gpu/GrCaps.h
[modify] https://crrev.com/89ca3a3b0210355a40bc0bc0ffd0104f91b67997/src/gpu/GrDrawOpAtlas.cpp
[modify] https://crrev.com/89ca3a3b0210355a40bc0bc0ffd0104f91b67997/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/89ca3a3b0210355a40bc0bc0ffd0104f91b67997/src/gpu/GrCaps.cpp

Re-assign to other labels when all rasterization optimization seem done, or at least when other improvements might be easier to achieve.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

senorblanco, bsalomon: is there more work to be done on the raster front here? If yes, can you assign to whoever should do this?  If no, can you re-add the dropped labels from #32 and give to the paint team?

I'll try repushing the Skia clear changes.