WebP Image Encoding is delaying compositor frames on the engine. |
||||
Issue descriptionThis trace was collected on the engine for google.com. It can be seen that the serialization of the commit messages is taking ~400 ms. This is due to the WebP encoding for each image referenced in the SkPicture. The long term solution would be with asynchronous image loading so the image encodes don't block the commit message. In the short term we should use the SHA1 of the raw data and avoid re-encoding the images on the engine in subsequent messages.
,
Apr 14 2016
Yup. If a page has lot of images, this overhead gets added to all of them. Since the sending of images has to be synchronous, not sure we can do anything right now...
,
Apr 14 2016
Assume this is already on your powerful machine. Can we look at the encoding code to see anything we can do there? Also do we have a bug for async encoding?
,
Apr 14 2016
Thanks for the tip. Looks like there is a configuration that the WebPEncoder is using to tweak the time taken by the encoding algorithm. Dropping it down to the lowest value has reduced the total deserialization time to ~70 ms. Attaching a trace. I'll just upload that change.
,
Apr 14 2016
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2aab581ec1f14450be730478ee715cc084544af commit f2aab581ec1f14450be730478ee715cc084544af Author: khushalsagar <khushalsagar@chromium.org> Date: Thu Apr 14 20:59:30 2016 blimp: Reduce WebP encoding time on the engine. The WebP Encoding during commit serialization blocks the render thread for close to 300 ms and significantly adds on to the time taken to push a frame to the client. Use the config that ensures the lowest encoding time. This reduced the encode time for an image on google.com from ~215 ms to ~53ms. BUG= 603353 Review URL: https://codereview.chromium.org/1889763002 Cr-Commit-Position: refs/heads/master@{#387421} [modify] https://crrev.com/f2aab581ec1f14450be730478ee715cc084544af/blimp/engine/renderer/engine_image_serialization_processor.cc
,
Apr 14 2016
A separate bug has been opened to move the encode work off the render thread when asynchronous image transfer is supported. (crbug/603643)
,
Apr 14 2016
,
Apr 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a2796c212636511499f47597e76865d7577cd1c8 commit a2796c212636511499f47597e76865d7577cd1c8 Author: nyquist <nyquist@chromium.org> Date: Mon Apr 18 20:58:17 2016 Initial version of Blimp BlobCache. Currently, every time there is a commit from engine to client, all pictures are sent again, leading to poor performance and high bandwidth usage. This CL adds a cache on the client side that never clears out any received resources. This means that the engine can depend on that the client has all resources it has previously sent, and uses this fact to only send down a unique identifier (SHA1 hash of the image) and the the width and height in place of the image itself. This is wrapped in the new proto BlobCacheImageMetadata. If this would be a new image for the client, the proto also includes the payload of the image. This payload will be removed when the BlobChannel is fully integrated. The identifier is calculated based on the source image, to ensure that no encode is required on the engine when the same image is seen again. To ensure that the cache is used for all images, this changes the behavior of the serializer to also encode images that were previously encoded as WebP, to ensure that they become cacheable. BUG= 597807 , 600719 , 603353 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1867653002 Cr-Commit-Position: refs/heads/master@{#388013} [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/client/feature/compositor/decoding_image_generator.cc [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/BUILD.gn [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/blob_cache.h [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/id_util.cc [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/id_util.h [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/in_memory_blob_cache.cc [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/in_memory_blob_cache.h [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/blob_cache/in_memory_blob_cache_unittest.cc [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/compositor/webp_decoder.cc [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/proto/BUILD.gn [add] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/common/proto/blob_cache.proto [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/engine/BUILD.gn [modify] https://crrev.com/a2796c212636511499f47597e76865d7577cd1c8/blimp/engine/renderer/engine_image_serialization_processor.cc
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138 commit 5344a6eebfbd65e6a86f2274c69e74ac6ccfe138 Author: nyquist <nyquist@chromium.org> Date: Mon Apr 25 23:29:24 2016 [blimp] Change to use SHA-256 instead of SHA-1 for unique ids Blimp currently uses SHA-1 to uniquely identify resources. There was no specific reason for this other than the goal of having a unique ID, so this changes it to instead use SHA-256 from //crypto, since it is in general better. This does however change the length of the hash from 20 bytes to 32 bytes. This concern was raised as a drive-by comment on https://codereview.chromium.org/1867653002/ This CL also adds functionality to verify that a specific BlobId has the correct length, and removes all references to SHA1 outside of the //blimp/common/blob_cache/id_util.[cc|h] files. In addition, this CL adds a test to ensure that there is certainty that this unique ID only ever changes intentionally. BUG= 597807 , 600719 , 603353 Review URL: https://codereview.chromium.org/1912153004 Cr-Commit-Position: refs/heads/master@{#389608} [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/BUILD.gn [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/DEPS [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/blob_cache/id_util.cc [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/blob_cache/id_util.h [add] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/blob_cache/id_util_unittest.cc [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/blob_cache/in_memory_blob_cache.cc [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/common/compositor/webp_decoder.cc [modify] https://crrev.com/5344a6eebfbd65e6a86f2274c69e74ac6ccfe138/blimp/engine/renderer/engine_image_serialization_processor.cc
,
Dec 9 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by klo...@chromium.org
, Apr 14 2016