Cache image decodes across navigations |
|
Issue descriptionWhen navigating between pages we re-decode images even if it's the same image. This is likely due to the "ClearImageCacheOnNavigation();" call. I created a demo of this behavior: https://pr.gg/decode/decode1.html In this demo a single BitmapImage.h object is created on the first page and is stored in blink's memory cache. This image is decoded. When navigating to the second page, the same BitmapImage.h object is re-used, but the image is decoded again. Using cached decodes would reduce decode costs on large websites such as amazon and google.
,
Sep 29 2017
Good point. We actually thought about this when we did the initial clearing. Local testing indicated that even if we don't clear the decoded image cache we still can't re-use these entries as the Skia Image IDs we get from Blink change after navigation/reload. Ongoing work like Khushal's changes to image IDs may make this no longer true, in which case it's worth revisiting. We could also just do the work to make these stable, if they aren't. Not saying this isn't a good thing to pursue, but there may be more needed than just removing the cache clear.
,
Sep 29 2017
The skia IDs used to be tied to the lifetime of blink::Image so that part wouldn't be affected by my recent changes. I wasn't sure about blink::Image lifetimes, but if that's cached and reused across navigations, then we should be able to reuse decodes in cc as well. I have a change up to do the latter pdr@ mentioned in #1: retain decoded images until the Image object is removed (https://chromium-review.googlesource.com/c/chromium/src/+/644317). We weren't sure if its worth it, given the additional overhead/plumbing for only purging on Image destruction . But if that helps with this use-case, then may be its another reason to try it.
,
Oct 4 2017
Small update: I ran into issues measuring the upper bound of decode caching. We need to correctly handle iframe image loads to get meaningful data from this metric, but with frames there is no longer a single "document generation id" for a given Page. Even if a unique stable id were available, I think the plumbing through the fragile dom->layout->paint->image pipeline is a nonstarter. We may be able to use another metric, Blink.MemoryCache.RevalidationPolicy.Image, to get a rough idea of the benefit of this approach. This metric has two issues: 1) overcounting of image atlases and image re-use in general. 2) complexity due to requests coming from both content and the preload scanner (see: RevalidationPolicy.Preload.Image). If we assume no image re-use: UMA data shows between 55% and 70% on android and between 60% and 85% on windows, of image loads are re-loads that could have re-used a previous decode (the range is due to issue #2 ). My intuition is that image re-use is high so the potential savings are lower. Here's a slightly different experiment we could run to decode the feasibility of this approach: 1) Add a UMA for total time spent on image decode tasks 2) Add a UMA metric for cache reuse entirely in the decode cache. 3) Do a finch experiment on Canary with ClearImageCacheOnNavigation entirely removed.
,
Dec 21 2017
Related bug: crbug.com/766653 |
|
►
Sign in to add a comment |
|
Comment 1 by pdr@chromium.org
, Sep 28 2017Owner: pdr@chromium.org
Status: Assigned (was: Untriaged)