Bug 111443 – [Qt][WK1] MemoryCache is not cleaned by default

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Allan Sandfeld Jensen]

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.

[Allan Sandfeld Jensen]

Created attachment 191502 [details]
Patch

[Arunprasad]

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.

[Allan Sandfeld Jensen]

(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.

[Arunprasad]

(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. :)

[Allan Sandfeld Jensen]

(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.

[Arunprasad]

> 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.

[Jocelyn Turcotte]

Comment on attachment 191502 [details]
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.

[Allan Sandfeld Jensen]

(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.

[Allan Sandfeld Jensen]

Committed r148951: <http://trac.webkit.org/changeset/148951>