The utility of purgeable memory for cached resources has diminished over time. Currently, WebKit uses file backed resources when possible. Since this is read only memory, it is in essence "purgeable". Having the PurgeableBuffer code adds additional complexity that we don't need. e.g. on my Mac, I am not seeing any entry for "WebCore purgeable data" when I run mmap against the web processes that I have running. It is used on iOS, however even there much of the purgeable memory we create gets replaced by file backed memory once we get the notification from CFNetwork.
Created attachment 236589 [details] Patch for bots
Attachment 236589 [details] did not pass style-queue: ERROR: Source/WebCore/loader/cache/MemoryCache.h:90: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.h:92: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 236589 [details] Patch for bots Attachment 236589 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5709250206629888 New failing tests: editing/pasteboard/drag-selected-image-to-contenteditable.html css3/masking/mask-repeat-space-padding.html http/tests/local/link-stylesheet-load-order-preload.html http/tests/cache/subresource-expiration-2.html css3/viewport-percentage-lengths/viewport-percentage-lengths-anonymous-block.html editing/selection/drag-to-contenteditable-iframe.html css3/viewport-percentage-lengths/viewport-percentage-lengths-percent-size-child.html http/tests/loading/redirect-methods.html http/tests/cache/post-redirect-get.php css3/background/background-repeat-space-content.html http/tests/cache/history-only-cached-subresource-loads.html fast/dom/StyleSheet/detached-style.html http/tests/cache/display-image-unset-allows-cached-image-load.html http/tests/cache/iframe-304-crash.html fast/filter-image/filter-image-animation.html fast/dom/StyleSheet/detached-style-pi-2.xhtml fast/dom/StyleSheet/detached-style-2.html fast/files/xhr-response-blob.html http/tests/cache/post-with-cached-subresources.php http/tests/cache/history-only-cached-subresource-loads-max-age-https.html http/tests/cache/subresource-fragment-identifier.html http/tests/cache/subresource-multiple-instances.html http/tests/cache/subresource-expiration-1.html editing/pasteboard/4989774.html editing/pasteboard/4947130.html fast/canvas/canvas-scale-drawImage-shadow.html http/tests/cache/cached-main-resource.html fast/css/font-face-download-error.html editing/pasteboard/4641033.html fast/dom/StyleSheet/detached-style-pi.xhtml
Created attachment 236598 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 236607 [details] Patch
Comment on attachment 236607 [details] Patch r=me
Committed r172736: <http://trac.webkit.org/changeset/172736>
Re-opened since this is blocked by bug 136060
This patch caused 14% PLT3 regression on Yosemite. See the email I sent internally.
14% PLT regression by removing this code. That is awesome! (And frankly a bit unbelievable. I'll take a look at it tomorrow)
Problem with the patch was that I removed the call to adjustSize() in MemoryCache::evict() - // If the resource was purged, it means we had already decremented the size when we made the - // resource purgeable in makeResourcePurgeable(). So adjust the size if we are evicting a - // resource that was not marked as purgeable. - if (!MemoryCache::shouldMakeResourcePurgeableOnEviction() || !resource->isPurgeable()) - adjustSize(resource->hasClients(), -static_cast<int>(resource->size())); We need to keep the call to adjustSize().
Created attachment 236828 [details] Patch
(In reply to comment #11) > Problem with the patch was that I removed the call to adjustSize() in MemoryCache::evict() > > - // If the resource was purged, it means we had already decremented the size when we made the > - // resource purgeable in makeResourcePurgeable(). So adjust the size if we are evicting a > - // resource that was not marked as purgeable. > - if (!MemoryCache::shouldMakeResourcePurgeableOnEviction() || !resource->isPurgeable()) > - adjustSize(resource->hasClients(), -static_cast<int>(resource->size())); > > We need to keep the call to adjustSize(). Props to Mr. Kling for spotting this.
Comment on attachment 236828 [details] Patch r=me
Committed r172790: <http://trac.webkit.org/changeset/172790>