Title: [122215] trunk/Source/WebCore
- Revision
- 122215
- Author
- [email protected]
- Date
- 2012-07-10 05:10:47 -0700 (Tue, 10 Jul 2012)
Log 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 <[email protected]> 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (122214 => 122215)
--- trunk/Source/WebCore/ChangeLog 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/ChangeLog 2012-07-10 12:10:47 UTC (rev 122215)
@@ -1,3 +1,56 @@
+2012-07-10 Huang Dongsung <[email protected]>
+
+ 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 <[email protected]>
RTL calendar picker for <input type=date> is too narrow and clipped
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (122214 => 122215)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2012-07-10 12:10:47 UTC (rev 122215)
@@ -428,6 +428,17 @@
setDecodedSize(decodedSize() + delta);
}
+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)
{
if (!image || image != m_image)
@@ -445,13 +456,7 @@
if (!image || image != m_image)
return false;
- CachedResourceClientWalker<CachedImageClient> w(m_clients);
- while (CachedImageClient* c = w.next()) {
- if (c->willRenderImage(this))
- return false;
- }
-
- return true;
+ return !likelyToBeUsedSoon();
}
void CachedImage::animationAdvanced(const Image* image)
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (122214 => 122215)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2012-07-10 12:10:47 UTC (rev 122215)
@@ -73,6 +73,8 @@
virtual void allClientsRemoved();
virtual void destroyDecodedData();
+ virtual bool likelyToBeUsedSoon();
+
virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);
virtual void error(CachedResource::Status);
virtual void setResponse(const ResourceResponse&);
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (122214 => 122215)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2012-07-10 12:10:47 UTC (rev 122215)
@@ -211,6 +211,9 @@
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; }
void decreasePreloadCount() { ASSERT(m_preloadCount); --m_preloadCount; }
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.cpp (122214 => 122215)
--- trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2012-07-10 12:10:47 UTC (rev 122215)
@@ -235,6 +235,12 @@
if (elapsedTime < cMinDelayBeforeLiveDecodedPrune)
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
// list in m_allResources.
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (122214 => 122215)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2012-07-10 12:06:37 UTC (rev 122214)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2012-07-10 12:10:47 UTC (rev 122215)
@@ -2698,7 +2698,11 @@
// 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());
}
int RenderObject::maximalOutlineSize(PaintPhase p) const
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
