111443 2013-03-05 08:29:36 -0800 [Qt][WK1] MemoryCache is not cleaned by default 2013-04-23 01:32:19 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 allan.jensen allan.jensen ararunprasad arurajku hausmann oldest_to_newest 847705 0 allan.jensen 2013-03-05 08:29:36 -0800 By default the MemoryCache does not remove dead resources, this only happens if setDeadDecodedDataDeletionInterval() has been called. In WK2 we do this in WebProcess based on the CacheModel, but in WK1 we only do it based on an undocumented dynamic property calle _q_deadDecodedDataDeletionInterval. By default we should enable cleaning of the memoryCache as long as it is enabled following the setting of WK2. 847709 1 191502 allan.jensen 2013-03-05 08:33:17 -0800 Created attachment 191502 Patch 847735 2 ararunprasad 2013-03-05 09:12:56 -0800 Is it really needed? Because QWebPage has a dynamic property(_q_deadDecodedDataDeletionInterval) to tune this as mentioned by you. Also if we have a room for dead decoded data in the MemoryCache why should we delete the dead decoded data? Incase of cache pressure first it will remove all the dead decoded data from the MemoryCache. 847751 3 allan.jensen 2013-03-05 09:37:45 -0800 (In reply to comment #2) > Is it really needed? Because QWebPage has a dynamic property(_q_deadDecodedDataDeletionInterval) to tune this as mentioned by you. Also if we have a room for dead decoded data in the MemoryCache why should we delete the dead decoded data? Incase of cache pressure first it will remove all the dead decoded data from the MemoryCache. Note we do not have memory pressure events in QtWebKit. Only Mac uses those. Keeping dead things out of memory means that we are less of a burden to the operating system, and less likely to have something we actually need swapped out. Plus it makes everything else faster if the operating system has more free memory. 847763 4 ararunprasad 2013-03-05 09:53:40 -0800 (In reply to comment #3) > (In reply to comment #2) > > Is it really needed? Because QWebPage has a dynamic property(_q_deadDecodedDataDeletionInterval) to tune this as mentioned by you. Also if we have a room for dead decoded data in the MemoryCache why should we delete the dead decoded data? Incase of cache pressure first it will remove all the dead decoded data from the MemoryCache. > > Note we do not have memory pressure events in QtWebKit. Only Mac uses those. Keeping dead things out of memory means that we are less of a burden to the operating system, and less likely to have something we actually need swapped out. Plus it makes everything else faster if the operating system has more free memory. Yes. We don't have memory pressure events. Suppose WebCore requests a new resource then MemoryCache will be audited to ensure whether it has enough space, if it don't have then it starts purging the things from DeadDecoded resource to Live Resources which is used long back(based on LRU). It is not a kind of event, but it is like a checking for the room before adding new resources. :) 849997 5 allan.jensen 2013-03-07 05:39:29 -0800 (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > Is it really needed? Because QWebPage has a dynamic property(_q_deadDecodedDataDeletionInterval) to tune this as mentioned by you. Also if we have a room for dead decoded data in the MemoryCache why should we delete the dead decoded data? Incase of cache pressure first it will remove all the dead decoded data from the MemoryCache. > > > > Note we do not have memory pressure events in QtWebKit. Only Mac uses those. Keeping dead things out of memory means that we are less of a burden to the operating system, and less likely to have something we actually need swapped out. Plus it makes everything else faster if the operating system has more free memory. > > Yes. We don't have memory pressure events. Suppose WebCore requests a new resource then MemoryCache will be audited to ensure whether it has enough space, if it don't have then it starts purging the things from DeadDecoded resource to Live Resources which is used long back(based on LRU). It is not a kind of event, but it is like a checking for the room before adding new resources. :) True, but imagine you have a large memory cache setup the way we do WK2. 128 Mbyte total with a maxDeadbytes of 64MByte. Every time the cache reached 128, it will be pruned to 64, and then slowly grow again back to 128Mbyte. If this patch was applied it woul instead be pruned every minute to try to hit the target of 64mbyte cache in use. 850007 6 ararunprasad 2013-03-07 05:55:42 -0800 > True, but imagine you have a large memory cache setup the way we do WK2. 128 Mbyte total with a maxDeadbytes of 64MByte. Every time the cache reached 128, it will be pruned to 64, and then slowly grow again back to 128Mbyte. If this patch was applied it woul instead be pruned every minute to try to hit the target of 64mbyte cache in use. Yeah!. So by default it is 1 minute, if the library user wants then they can override using _q_deadDecodedDataDeletionInterval. 879058 7 191502 jturcotte 2013-04-22 03:34:10 -0700 Comment on attachment 191502 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191502&action=review > Source/WebKit/qt/WebCoreSupport/InitWebCoreQt.cpp:108 > + WebCore::memoryCache()->setDeadDecodedDataDeletionInterval(60); If you can think of any way to avoid hard-coding this value in two different files it would be nice. I can't think of any simple way so r=me in any case. 879064 8 allan.jensen 2013-04-22 03:48:38 -0700 (In reply to comment #7) > (From update of attachment 191502 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191502&action=review > > > Source/WebKit/qt/WebCoreSupport/InitWebCoreQt.cpp:108 > > + WebCore::memoryCache()->setDeadDecodedDataDeletionInterval(60); > > If you can think of any way to avoid hard-coding this value in two different files it would be nice. > I can't think of any simple way so r=me in any case. Ideally we would expose an API to set the cacheModel, just like WK2 and the other ports does. 879675 9 allan.jensen 2013-04-23 01:32:19 -0700 Committed r148951: <http://trac.webkit.org/changeset/148951> 191502 2013-03-05 08:33:17 -0800 2013-04-22 03:34:09 -0700 Patch bug-111443-20130305172834.patch text/plain 2571 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ0NjI3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L3F0 L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCmluZGV4IDkzN2ZhOTE5NWVl ZDRiOGE3YTc0MDY5NmRhNjE0N2Q0MjE5NDAwMTUuLmU1YTUyNDE3NDAxNzVhNmU4ZjAyMWY5YzA3 MTdiOTNkMjAxMDA2MmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCisr KyBiL1NvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTMtMDMt MDUgIEFsbGFuIFNhbmRmZWxkIEplbnNlbiAgPGFsbGFuLmplbnNlbkBkaWdpYS5jb20+CisKKyAg ICAgICAgW1F0XVtXSzFdIE1lbW9yeUNhY2hlIGlzIG5vdCBjbGVhbmVkIGJ5IGRlZmF1bHQKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMTQ0MworCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEVuYWJsZSBkZWxl dGlvbiBvZiBkZWFkIHJlc291cmNlcyBpbiB0aGUgbWVtb3J5IGNhY2hlIGFzIGxvbmcgYXMgdGhl IE1lbW9yeUNhY2hlIGlzIGVuYWJsZWQuCisKKyAgICAgICAgVGhpcyBjYW4gYmUgZGlzYWJsZWQg b3IgdHVuZWQgdXNpbmcgdGhlIGR5bmFtaWMgcHJvcGVydHkgICJfcV9kZWFkRGVjb2RlZERhdGFE ZWxldGlvbkludGVydmFsIi4KKyAgICAgICAgVGhlIGRlZmF1bHQgaXMgc2V0IHRvIG1hdGNoIHRo YXQgb2YgbW9zdCBvdGhlciBwb3J0cyBhbmQgV0syIHdpdGggY2FjaGUgaW4gV2ViQnJvd3Npbmcg bW9kZS4KKworICAgICAgICAqIEFwaS9xd2Vic2V0dGluZ3MuY3BwOgorICAgICAgICAoUVdlYlNl dHRpbmdzOjpzZXRPYmplY3RDYWNoZUNhcGFjaXRpZXMpOgorICAgICAgICAqIFdlYkNvcmVTdXBw b3J0L0luaXRXZWJDb3JlUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6aW5pdGlhbGl6ZVdlYkNv cmVRdCk6CisKIDIwMTMtMDMtMDMgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KIAog ICAgICAgICBVbnJldmlld2VkIGF0dGVtcHRlZCBidWlsZCBmaXguIEFkZHMgYmFjayBzb21lIGlu Y2x1ZGVzIHJlbW92ZWQgaW4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvcXQvQXBpL3F3ZWJz ZXR0aW5ncy5jcHAgYi9Tb3VyY2UvV2ViS2l0L3F0L0FwaS9xd2Vic2V0dGluZ3MuY3BwCmluZGV4 IDBhZmUyZmQxYmI5N2UzYTc2YTM2NDkxN2I2ZDAxOGI5MzJjMDFhZDIuLmNiMjA0ODdjNDQ5ZjZh MjQyMGY1MGU4OGI4MTJkY2RlYjA3YmE5YmYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvcXQv QXBpL3F3ZWJzZXR0aW5ncy5jcHAKKysrIGIvU291cmNlL1dlYktpdC9xdC9BcGkvcXdlYnNldHRp bmdzLmNwcApAQCAtODkzLDYgKzg5Myw3IEBAIHZvaWQgUVdlYlNldHRpbmdzOjpzZXRPYmplY3RD YWNoZUNhcGFjaXRpZXMoaW50IGNhY2hlTWluRGVhZENhcGFjaXR5LCBpbnQgY2FjaGVNCiAgICAg V2ViQ29yZTo6bWVtb3J5Q2FjaGUoKS0+c2V0Q2FwYWNpdGllcyhxTWF4KDAsIGNhY2hlTWluRGVh ZENhcGFjaXR5KSwKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHFNYXgoMCwg Y2FjaGVNYXhEZWFkKSwKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHFNYXgo MCwgdG90YWxDYXBhY2l0eSkpOworICAgIFdlYkNvcmU6Om1lbW9yeUNhY2hlKCktPnNldERlYWRE ZWNvZGVkRGF0YURlbGV0aW9uSW50ZXJ2YWwoZGlzYWJsZUNhY2hlID8gMCA6IDYwKTsKIH0KIAog LyohCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L3F0L1dlYkNvcmVTdXBwb3J0L0luaXRXZWJD b3JlUXQuY3BwIGIvU291cmNlL1dlYktpdC9xdC9XZWJDb3JlU3VwcG9ydC9Jbml0V2ViQ29yZVF0 LmNwcAppbmRleCA2YTAyMmFjNDNiNGNhZmFkYmQzNDRhM2EwNDE0NGU2YWRjNGZmMWI2Li43ZDFm NjAwMThkYWIzNzk3ZGM5NTFjY2Q5MDE0ZTk2ODgxNDE3YjVlIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L3F0L1dlYkNvcmVTdXBwb3J0L0luaXRXZWJDb3JlUXQuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQvcXQvV2ViQ29yZVN1cHBvcnQvSW5pdFdlYkNvcmVRdC5jcHAKQEAgLTM0LDYgKzM0LDcg QEAKICNpbmNsdWRlICJDaHJvbWVDbGllbnRRdC5oIgogI2luY2x1ZGUgIkltYWdlLmgiCiAjaW5j bHVkZSAiSW5pdGlhbGl6ZUxvZ2dpbmcuaCIKKyNpbmNsdWRlICJNZW1vcnlDYWNoZS5oIgogI2lu Y2x1ZGUgIk5vdEltcGxlbWVudGVkLmgiCiAjaW5jbHVkZSAiUGFnZS5oIgogI2luY2x1ZGUgIlBs YXRmb3JtU3RyYXRlZ2llc1F0LmgiCkBAIC0xMDMsNiArMTA0LDggQEAgUV9ERUNMX0VYUE9SVCB2 b2lkIGluaXRpYWxpemVXZWJDb3JlUXQoKQogI2lmIFVTRShRVEtJVCkKICAgICBJbml0V2ViQ29y ZVN5c3RlbUludGVyZmFjZSgpOwogI2VuZGlmCisgICAgaWYgKCFXZWJDb3JlOjptZW1vcnlDYWNo ZSgpLT5kaXNhYmxlZCgpKQorICAgICAgICBXZWJDb3JlOjptZW1vcnlDYWNoZSgpLT5zZXREZWFk RGVjb2RlZERhdGFEZWxldGlvbkludGVydmFsKDYwKTsKIAogICAgIGluaXRpYWxpemVkID0gdHJ1 ZTsKIH0K