Bug 74798 - REGRESSION(r92782): asynchronize and throttle memory cache prune with timer
Summary: REGRESSION(r92782): asynchronize and throttle memory cache prune with timer
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-17 18:27 PST by Wang Zhongping
Modified: 2011-12-21 00:14 PST (History)
4 users (show)

See Also:


Attachments
Proposed patch (5.16 KB, patch)
2011-12-17 18:29 PST, Wang Zhongping
no flags Details | Formatted Diff | Diff
slideshow testcase (13.65 MB, application/octet-stream)
2011-12-21 00:14 PST, Wang Zhongping
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wang Zhongping 2011-12-17 18:27:21 PST
In slideshow of images in large number, webkit memory cache capacity can be exceed quite easily. Starting from 65859 patch (https://bugs.webkit.org/show_bug.cgi?id=65859), memory cache is notified upon decoded data access after each image is drawn. Those notifications result in lots of memory cache prune requests in high frequency. The painting of images are slowed down, because memory cache prune is performed synchronously and it won't return until prune is done.

This patch is to make memory cache prune asynchronous through a Timer and add throttle support to those follow-up requests after timer is scheduled and before it's fired. Timer is scheduled with a delay, either fixed or variable, it's open to design. In this patch, a prune-request-#-per-second related delay is used, to adapt workload change. This patch improves performance by allowing reasonably larger memory cache peak when workload is higher, but it also promises to prune cache timely once timer is fired.
Comment 1 Wang Zhongping 2011-12-17 18:29:03 PST
Created attachment 119746 [details]
Proposed patch
Comment 2 Mark Rowe (bdash) 2011-12-17 18:46:46 PST
Traditionally the XYZ in the “REGRESSION(XYZ)” prefix in a bug title refers to a revision number. Putting a bug number in it with no indication that is what the number is can be very confusing.
Comment 3 Alexey Proskuryakov 2011-12-18 21:56:45 PST
r92782 is Chromium code, and changing cross-platform code to fix an issue that apparently only occurs in one port is something that needs an explanation.

How does this change affect other ports? Can it be tested with an automated regression test?
Comment 4 Wang Zhongping 2011-12-19 01:47:22 PST
Thanks for your questions. In fact, Chormium change in r92782 has already existed in almost all of the other ports, per ChangeLog of r92782

      "This causes us to call CachedImage::didDraw() which calls CachedResource::didAccessDecodedData() which updates 
        the LRU for decoded image data and potentially prunes the cache. This is already done by almost all of the other ports."

r92782 is to make Chromium behave the same as other ports, so this performance regression issue not only exists in 
Chromium but also exist in other ports (such as cairo, openvg, qt). This patch will also benefit other ports. It can be 
tested with automated regression test.
Comment 5 Alexey Proskuryakov 2011-12-20 17:29:42 PST
Thank you for the explanation.

> It can be tested with automated regression test.

Please add a test to the patch. We want all testable fixes to come with regression tests. This is particularly important for loader code.
Comment 6 Wang Zhongping 2011-12-21 00:14:06 PST
Created attachment 120154 [details]
slideshow testcase

This is the testcase from which we find the regression. It has been simplified otherwise it can't be uploaded as attachment due to large size (>20M). Here is a brief description: it consists of 10 rounds of slideshow of 150 images. Round 1 is for warm-up. Round 2-4 is to measure performance when drawing images in full window size, round 5-7 is in raw image size, and round 8-10 is in fixed size (800*600). Result metrics indicates total time consumed in each round by T(1) to T(10), and 4 additional metrics M(1) to M(4) which each indicates average time consumption per image in previous four groups of rounds.