Title: [195430] trunk/Source/WebCore
- Revision
- 195430
- Author
- akl...@apple.com
- Date
- 2016-01-21 18:00:42 -0800 (Thu, 21 Jan 2016)
Log Message
CGImageSource sometimes retains temporary SharedBuffer data indefinitely, doubling memory cost.
<https://webkit.org/b/153325>
Reviewed by Anders Carlsson.
After a resource has finished downloading, and has been cached to disk cache,
we mmap() the disk cached version so we can throw out the temporary download buffer.
Due to the way CGImageSource works on Mac/iOS, it's not possible to replace the data
being decoded once the image has been fully decoded once. When doing the replacement,
we'd end up with the SharedBuffer wrapping the mmap() data, and the CGImageSource
keeping the old SharedBuffer::DataBuffer alive, effectively doubling the memory cost.
This patch adds a CachedResource::didReplaceSharedBufferContents() callback that
CachedImage implements to throw out the decoded data. This is currently the only way
to make CGImageSource drop the retain it holds on the SharedBuffer::DataBuffer.
The downside of this approach is that we'll sometimes incur the cost of one additional
image decode after an image downloads and is cached for the first time.
I put a FIXME in there since we could do better with a little help from CGImageSource.
* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::didReplaceSharedBufferContents):
* loader/cache/CachedImage.h:
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::tryReplaceEncodedData):
* loader/cache/CachedResource.h:
(WebCore::CachedResource::didReplaceSharedBufferContents):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (195429 => 195430)
--- trunk/Source/WebCore/ChangeLog 2016-01-22 00:47:00 UTC (rev 195429)
+++ trunk/Source/WebCore/ChangeLog 2016-01-22 02:00:42 UTC (rev 195430)
@@ -1,3 +1,34 @@
+2016-01-21 Andreas Kling <akl...@apple.com>
+
+ CGImageSource sometimes retains temporary SharedBuffer data indefinitely, doubling memory cost.
+ <https://webkit.org/b/153325>
+
+ Reviewed by Anders Carlsson.
+
+ After a resource has finished downloading, and has been cached to disk cache,
+ we mmap() the disk cached version so we can throw out the temporary download buffer.
+
+ Due to the way CGImageSource works on Mac/iOS, it's not possible to replace the data
+ being decoded once the image has been fully decoded once. When doing the replacement,
+ we'd end up with the SharedBuffer wrapping the mmap() data, and the CGImageSource
+ keeping the old SharedBuffer::DataBuffer alive, effectively doubling the memory cost.
+
+ This patch adds a CachedResource::didReplaceSharedBufferContents() callback that
+ CachedImage implements to throw out the decoded data. This is currently the only way
+ to make CGImageSource drop the retain it holds on the SharedBuffer::DataBuffer.
+ The downside of this approach is that we'll sometimes incur the cost of one additional
+ image decode after an image downloads and is cached for the first time.
+
+ I put a FIXME in there since we could do better with a little help from CGImageSource.
+
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::didReplaceSharedBufferContents):
+ * loader/cache/CachedImage.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::tryReplaceEncodedData):
+ * loader/cache/CachedResource.h:
+ (WebCore::CachedResource::didReplaceSharedBufferContents):
+
2016-01-21 Beth Dakin <bda...@apple.com>
Add the ability to update WebKitAdditions to WK2
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (195429 => 195430)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2016-01-22 00:47:00 UTC (rev 195429)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2016-01-22 02:00:42 UTC (rev 195430)
@@ -436,6 +436,16 @@
CachedResource::finishLoading(data);
}
+void CachedImage::didReplaceSharedBufferContents()
+{
+ if (m_image) {
+ // Let the Image know that the SharedBuffer has been rejigged, so it can let go of any references to the heap-allocated resource buffer.
+ // FIXME(rdar://problem/24275617): It would be better if we could somehow tell the Image's decoder to swap in the new contents without destroying anything.
+ m_image->destroyDecodedData(true);
+ }
+ CachedResource::didReplaceSharedBufferContents();
+}
+
void CachedImage::error(CachedResource::Status status)
{
checkShouldPaintBrokenImage();
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (195429 => 195430)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2016-01-22 00:47:00 UTC (rev 195429)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2016-01-22 02:00:42 UTC (rev 195430)
@@ -127,6 +127,8 @@
void addIncrementalDataBuffer(SharedBuffer&);
+ void didReplaceSharedBufferContents() override;
+
typedef std::pair<LayoutSize, float> SizeAndZoom;
typedef HashMap<const CachedImageClient*, SizeAndZoom> ContainerSizeRequests;
ContainerSizeRequests m_pendingContainerSizeRequests;
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (195429 => 195430)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-01-22 00:47:00 UTC (rev 195429)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-01-22 02:00:42 UTC (rev 195430)
@@ -807,9 +807,8 @@
if (m_data->size() != newBuffer.size() || memcmp(m_data->data(), newBuffer.data(), m_data->size()))
return;
- if (m_data->tryReplaceContentsWithPlatformBuffer(newBuffer)) {
- // FIXME: Should we call checkNotify() here to move already-decoded images to the new data source?
- }
+ if (m_data->tryReplaceContentsWithPlatformBuffer(newBuffer))
+ didReplaceSharedBufferContents();
}
#endif
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (195429 => 195430)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2016-01-22 00:47:00 UTC (rev 195429)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2016-01-22 02:00:42 UTC (rev 195430)
@@ -267,6 +267,8 @@
void setDecodedSize(unsigned);
void didAccessDecodedData(double timeStamp);
+ virtual void didReplaceSharedBufferContents() { }
+
// FIXME: Make the rest of these data members private and use functions in derived classes instead.
HashCountedSet<CachedResourceClient*> m_clients;
ResourceRequest m_resourceRequest;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
