New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 699121 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 732656
issue 739473

Blocking:
issue 672303



Sign in to add a comment

cc: Reduce cost in sending compositor frame

Project Member Reported by weiliangc@chromium.org, Mar 7 2017

Issue description

There are two factors in compositor frame messages being slow:
- queueing delay
- serialization / deserialization

This focus serialization / deserialization cost.

The largest part of a compositor frame are:
- resource_list
- RenderPass's SharedQuadState list and DrawQuad list.


The plan to reduce cost for resource list to to make sure TransferableResource are POD and thus can be copied/read as entire list.

This also includes looking into why Mojo does validation for POD.


The plan to reduce cost for SQS and Quad list is either make them all POD like resource list, or use incremental update. 
 
Cc: piman@chromium.org
I mentioned this on chat but is_trivially_copyable (which POD is) is neccessary but not sufficient. We also need the types to not have any non-trivial constructors or setters. It is because those can control the internal state, meaning the object could have invalid states. But if we memcpy arbitrary stuff into there we don't want to leave the object in an invalid state. gfx::Size is one such problematic case for that reason.

Comment 2 by enne@chromium.org, Mar 7 2017

The other thought that might be worth investigating is just sending less data, by caching previous compositor frames on the other end.

I don't know that I'd say this is a fully baked design, but here's a thought experiment:  One way to do this would be to add "layer" ids.  A render pass is now a list of layer ids to draw, instead of a set of quads.  A compositor frame has a mapping of layer id => (unordered list of quads, one shared quad state).  I'll call that state a "layer group".  For the transport, various delta updates would be sent.  Add/remove/update render passes.  Add/remove/update layer group quad list/shared quad state/both.  Compositor resources would get returned after their quads were removed.

The hard part I think would be trying to generate that delta update without doing too much work.  It's not super clear to me how to easily update any of these things without generating the frame and seeing what changed / what layers are present / what quads are present.  Maybe other folks see this differently and have some insight about how to do this cheaply.  I also don't think it's possible to really know how much is cached without implementing the cache and then measuring, unfortunately.  I'd love for our system to get to a place where we can do more incremental updates, but it's not clear to me if this is more work to figure out what the incremental update is than it is to just send everything.

I think in a salamander mus world, there's less (no?) transport of compositor frames.  It seems like if you have to generate most of a frame to know what you need to send, then maybe we'd just want to generate it fully and stop there.  So, I think this is an interesting idea, but I don't know that it's something we'd need long term.  Maybe we'd still want to consider it tactically if this is a mus tadpole requirement.

(I do also think we have similar data copying issues in SurfaceAggregator that might also need an eye towards being incremental or adding indirection.  And that'll be something that will continue to be an issue even in a mus salamander world.  If this is of similar cost, maybe that's a better place to put some effort.)
Currently I'm exploring two options:
1. Bulk copy of memory for compositor frame data. Most of the compositor frame data is from resource list, quad list and shared quad state list, and those might be ok to be copied in bulk list rather than individual ones. Now working on moving color space out into a separate list since color space is not safe for bulk copying. Also looking at performance differences in doing this and how to do this in Mojo.

2. Incremental update. I think using "layer" ids to identify shared quad states is good place to start. One of the questions needs to be answered here is probably look at cluster telemetry on how often/much compositor frames different from frame to frame. Another would be investigate could we use damage tracking and property changed to generate useful diff.

For the salamander mus world, we could use same investigative methods in diff generation, but w/ regards to layers and property trees. And how much effort of doing compositor frame diff could very well depend on the timeline of projects.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b24a207b8d1e25b9cfa4ead338809d3decb6a08c

commit b24a207b8d1e25b9cfa4ead338809d3decb6a08c
Author: weiliangc <weiliangc@chromium.org>
Date: Sat Apr 01 00:55:31 2017

cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=699121
R=danakj

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2770753002
Cr-Commit-Position: refs/heads/master@{#461292}

[modify] https://crrev.com/b24a207b8d1e25b9cfa4ead338809d3decb6a08c/cc/ipc/cc_serialization_perftest.cc

Comment 5 by sadrul@chromium.org, Apr 20 2017

Cc: sadrul@chromium.org
Blocking: 601863
Labels: displaycompositor
Cc: varkha@chromium.org
Blockedon: 732656

Comment 9 by danakj@chromium.org, Jun 13 2017

Blocking: -601863 672303
Blockedon: 739473
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f09363d82bd745f01083a7c811db2d4b6775006

commit 8f09363d82bd745f01083a7c811db2d4b6775006
Author: Weiliang Chen <weiliangc@chromium.org>
Date: Wed Aug 09 18:29:13 2017

cc: Add Tracing to Compositor Frame Transport from Renderer

This adds tracing for following cases:
- serialization and deserialization of compositor frames in IPC,
- deserialization of compositor frames in Mojo,
- flow event from renderer to browser on desktop,
- instance of time elapsed between submitting compositor frame to
receiving from renderer to browser on desktop.

R=danakj@chromium.org

Bug: 699121
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6ca1adc6dc61bc28c4c241ec63a67402f2edf7cf
Reviewed-on: https://chromium-review.googlesource.com/553117
Commit-Queue: weiliangc <weiliangc@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493060}
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/cc/ipc/compositor_frame_metadata_struct_traits.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/cc/ipc/render_pass_struct_traits.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/cc/ipc/transferable_resource_struct_traits.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/components/viz/client/client_layer_tree_frame_sink.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/components/viz/service/frame_sinks/compositor_frame_sink_impl.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/components/viz/service/frame_sinks/compositor_frame_sink_impl.h
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/content/browser/renderer_host/offscreen_canvas_provider_impl_unittest.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/ui/ws/compositor_frame_sink_client_binding.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/ui/ws/compositor_frame_sink_client_binding.h
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/viz/public/cpp/compositing/compositor_frame_struct_traits.cc
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/services/viz/public/interfaces/compositing/compositor_frame_sink.mojom
[modify] https://crrev.com/8f09363d82bd745f01083a7c811db2d4b6775006/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp

Sign in to add a comment