<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[195430] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/195430">195430</a></dd>
<dt>Author</dt> <dd>akling@apple.com</dd>
<dt>Date</dt> <dd>2016-01-21 18:00:42 -0800 (Thu, 21 Jan 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreloadercacheCachedImagecpp">trunk/Source/WebCore/loader/cache/CachedImage.cpp</a></li>
<li><a href="#trunkSourceWebCoreloadercacheCachedImageh">trunk/Source/WebCore/loader/cache/CachedImage.h</a></li>
<li><a href="#trunkSourceWebCoreloadercacheCachedResourcecpp">trunk/Source/WebCore/loader/cache/CachedResource.cpp</a></li>
<li><a href="#trunkSourceWebCoreloadercacheCachedResourceh">trunk/Source/WebCore/loader/cache/CachedResource.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (195429 => 195430)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2016-01-21 Andreas Kling <akling@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):
+
</ins><span class="cx"> 2016-01-21 Beth Dakin <bdakin@apple.com>
</span><span class="cx">
</span><span class="cx"> Add the ability to update WebKitAdditions to WK2
</span></span></pre></div>
<a id="trunkSourceWebCoreloadercacheCachedImagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (195429 => 195430)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -436,6 +436,16 @@
</span><span class="cx"> CachedResource::finishLoading(data);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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();
+}
+
</ins><span class="cx"> void CachedImage::error(CachedResource::Status status)
</span><span class="cx"> {
</span><span class="cx"> checkShouldPaintBrokenImage();
</span></span></pre></div>
<a id="trunkSourceWebCoreloadercacheCachedImageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (195429 => 195430)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -127,6 +127,8 @@
</span><span class="cx">
</span><span class="cx"> void addIncrementalDataBuffer(SharedBuffer&);
</span><span class="cx">
</span><ins>+ void didReplaceSharedBufferContents() override;
+
</ins><span class="cx"> typedef std::pair<LayoutSize, float> SizeAndZoom;
</span><span class="cx"> typedef HashMap<const CachedImageClient*, SizeAndZoom> ContainerSizeRequests;
</span><span class="cx"> ContainerSizeRequests m_pendingContainerSizeRequests;
</span></span></pre></div>
<a id="trunkSourceWebCoreloadercacheCachedResourcecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (195429 => 195430)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -807,9 +807,8 @@
</span><span class="cx"> if (m_data->size() != newBuffer.size() || memcmp(m_data->data(), newBuffer.data(), m_data->size()))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- if (m_data->tryReplaceContentsWithPlatformBuffer(newBuffer)) {
- // FIXME: Should we call checkNotify() here to move already-decoded images to the new data source?
- }
</del><ins>+ if (m_data->tryReplaceContentsWithPlatformBuffer(newBuffer))
+ didReplaceSharedBufferContents();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebCoreloadercacheCachedResourceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (195429 => 195430)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -267,6 +267,8 @@
</span><span class="cx"> void setDecodedSize(unsigned);
</span><span class="cx"> void didAccessDecodedData(double timeStamp);
</span><span class="cx">
</span><ins>+ virtual void didReplaceSharedBufferContents() { }
+
</ins><span class="cx"> // FIXME: Make the rest of these data members private and use functions in derived classes instead.
</span><span class="cx"> HashCountedSet<CachedResourceClient*> m_clients;
</span><span class="cx"> ResourceRequest m_resourceRequest;
</span></span></pre>
</div>
</div>
</body>
</html>