Changeset 122215 in webkit

Timestamp:

Jul 10, 2012 5:10:47 AM (12 years ago)

Author:

kseo@webkit.org

Message:

Don't destroy the decoded data of an image if WebKit is about to render the image.
​https://bugs.webkit.org/show_bug.cgi?id=90721

Patch by Huang Dongsung <​luxtella@company100.net> on 2012-07-10
Reviewed by Antti Koivisto.

When the cache capacity of the MemoryCache is exceeded, the decoded data of all
the CachedImages are destroyed. Even the images inside the viewport are
destroyed. However, if the images need to be rendered again due to scoll events
or animation, they must be decoded again. As an extreme case, if there is an
animation with an image when MemoryCache is almost full, the image must be
decoded every frame. This slows down animation and needlessly consumes CPU
cycles.

Therefore, it is better to not destory the decoded data of an image if the image
is inside the viewport because there is high chance that the image needs to be
rendered again soon. This patch reduces the unnecessary repetition of image decoding
on low memory, and also relieves the memory fragmentation because it avoids reallocation
of image frames.

In addition, there is another positive side effect. Currently,
CachedImageClient::willRenderImage() is used only to determine if GIF animation needs
to be paused or not in CachedImage::shouldPauseAnimation(). This patch makes
GIF animation outside the viewort be paused.

This is also a prerequisite for parallel image decoders. Because parallel image
decoders decode an image asynchronously, clients cannot render the image at the time
when the request is made. Clients can draw the image later after receiving image
decoding complete notification. However, there is a problem because MemoryCache can
destroy the decoded data before clients actually render the image. So parallel image decoders
must prevent the decoded data from being destroyed if the image will be rendered
soon.

This patch may consume a little more memory, but furtunately the peak memory usage
is almost the same.

No new tests - no new testable functionality.

• loader/cache/CachedImage.cpp:

(WebCore::CachedImage::likelyToBeUsedSoon):
(WebCore):
(WebCore::CachedImage::shouldPauseAnimation):

• loader/cache/CachedImage.h:

(CachedImage):

• loader/cache/CachedResource.h:

(CachedResource):
(WebCore::CachedResource::likelyToBeUsedSoon):

• loader/cache/MemoryCache.cpp:

(WebCore::MemoryCache::pruneLiveResourcesToSize):

• rendering/RenderObject.cpp:

(WebCore::RenderObject::willRenderImage):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedImage.cpp (modified) (2 diffs)
• loader/cache/CachedImage.h (modified) (1 diff)
• loader/cache/CachedResource.h (modified) (1 diff)
• loader/cache/MemoryCache.cpp (modified) (1 diff)
• rendering/RenderObject.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-10  Huang Dongsung  <luxtella@company100.net>
+
+        Don't destroy the decoded data of an image if WebKit is about to render the image.
+        https://bugs.webkit.org/show_bug.cgi?id=90721
+
+        Reviewed by Antti Koivisto.
+
+        When the cache capacity of the MemoryCache is exceeded, the decoded data of all
+        the CachedImages are destroyed. Even the images inside the viewport are
+        destroyed.  However, if the images need to be rendered again due to scoll events
+        or animation, they must be decoded again. As an extreme case, if there is an
+        animation with an image when MemoryCache is almost full, the image must be
+        decoded every frame. This slows down animation and needlessly consumes CPU
+        cycles.
+
+        Therefore, it is better to not destory the decoded data of an image if the image
+        is inside the viewport because there is high chance that the image needs to be
+        rendered again soon. This patch reduces the unnecessary repetition of image decoding
+        on low memory, and also relieves the memory fragmentation because it avoids reallocation
+        of image frames.
+
+        In addition, there is another positive side effect. Currently,
+        CachedImageClient::willRenderImage() is used only to determine if GIF animation needs
+        to be paused or not in CachedImage::shouldPauseAnimation(). This patch makes
+        GIF animation outside the viewort be paused.
+
+        This is also a prerequisite for parallel image decoders. Because parallel image
+        decoders decode an image asynchronously, clients cannot render the image at the time
+        when the request is made. Clients can draw the image later after receiving image
+        decoding complete notification. However, there is a problem because MemoryCache can
+        destroy the decoded data before clients actually render the image. So parallel image decoders
+        must prevent the decoded data from being destroyed if the image will be rendered
+        soon.
+
+        This patch may consume a little more memory, but furtunately the peak memory usage
+        is almost the same.
+
+        No new tests - no new testable functionality.
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::likelyToBeUsedSoon):
+        (WebCore):
+        (WebCore::CachedImage::shouldPauseAnimation):
+        * loader/cache/CachedImage.h:
+        (CachedImage):
+        * loader/cache/CachedResource.h:
+        (CachedResource):
+        (WebCore::CachedResource::likelyToBeUsedSoon):
+        * loader/cache/MemoryCache.cpp:
+        (WebCore::MemoryCache::pruneLiveResourcesToSize):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willRenderImage):
+
 2012-07-10  Kent Tamura  <tkent@chromium.org>
 

trunk/Source/WebCore/loader/cache/CachedImage.cpp

@@ -429 +429 @@
 }
 
+bool CachedImage::likelyToBeUsedSoon()
+{
+    CachedResourceClientWalker<CachedImageClient> walker(m_clients);
+    while (CachedImageClient* client = walker.next()) {
+        if (client->willRenderImage(this))
+            return true;
+    }
+
+    return false;
+}
+
 void CachedImage::didDraw(const Image* image)
 {
@@ -446 +457 @@
         return false;
     
-    CachedResourceClientWalker<CachedImageClient> w(m_clients);
-    while (CachedImageClient* c = w.next()) {
-        if (c->willRenderImage(this))
-            return false;
-    }
-
-    return true;
+    return !likelyToBeUsedSoon();
 }
 

trunk/Source/WebCore/loader/cache/CachedImage.h

@@ -74 +74 @@
     virtual void destroyDecodedData();
 
+    virtual bool likelyToBeUsedSoon();
+
     virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);
     virtual void error(CachedResource::Status);

trunk/Source/WebCore/loader/cache/CachedResource.h

@@ -212 +212 @@
     void setOwningCachedResourceLoader(CachedResourceLoader* cachedResourceLoader) { m_owningCachedResourceLoader = cachedResourceLoader; }
     
+    // MemoryCache does not destroy the decoded data of a CachedResource if the decoded data will be likely used.
+    virtual bool likelyToBeUsedSoon() { return false; }
+
     bool isPreloaded() const { return m_preloadCount; }
     void increasePreloadCount() { ++m_preloadCount; }

trunk/Source/WebCore/loader/cache/MemoryCache.cpp

@@ -236 +236 @@
                 return;
 
+            // Check to see if the current resource are likely to be used again soon.
+            if (current->likelyToBeUsedSoon()) {
+                current = prev;
+                continue;
+            }
+
             // Destroy our decoded data. This will remove us from 
             // m_liveDecodedResources, and possibly move us to a different LRU 

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -2699 +2699 @@
     // If we're not in a window (i.e., we're dormant from being put in the b/f cache or in a background tab)
     // then we don't want to render either.
-    return !document()->inPageCache() && !document()->view()->isOffscreen();
+    if (document()->inPageCache() || document()->view()->isOffscreen())
+        return false;
+
+    // If a renderer is outside the viewport, we won't render.
+    return viewRect().intersects(absoluteClippedOverflowRect());
 }