Bug 122954 – Try to keep AppCache memory caches in control

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[Xabier Rodríguez Calvar]

[AppCache] Trying to keep memory caches in control

[Xabier Rodríguez Calvar]

Created attachment 214441 [details]
Patch

Attempt to keep the memory cache under control. This is a proof of concept and suggestions are very welcome. Changelog will be added when patch is ready.

[Build Bot]

Comment on attachment 214441 [details]
Patch

Attachment 214441 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/3474135

[Build Bot]

Comment on attachment 214441 [details]
Patch

Attachment 214441 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/3480051

[Xabier Rodríguez Calvar]

Created attachment 214454 [details]
Patch

AtFixed compile issue

[Alexey Proskuryakov]

Could you please explain your approach? It's hard to see how code proves a concept when the concept is kept secret :)

[Xabier Rodríguez Calvar]

(In reply to comment #5)
> Could you please explain your approach? It's hard to see how code proves a concept when the concept is kept secret :)

Sure , sorry for that :)

The idea is having an upper limit for the memory used by the app cache in case we have several caches in memory and as they are in the database anyway we can reload the resource from disk to memory easily when needed.

What the patch does is setting some control points, specially when a cache is loaded or stuff is added to it. Then it checks the size of the caches in memory and if it's above the limit, at a first step it tries to dump all caches but the last one and if that doesn't reach the limit, it waits for some time and  then it dumps everything.

[Alexey Proskuryakov]

Yes, this is certainly a huge inefficiencies in the current appcache implementation.

I don't think that hardcoding a limit is a great answer though. Keeping even a single application in memory doesn't make sense when it's large.

We need something much more like a regular resource cache, just backed by appcache database and not the network. That would respect system low memory notifications too.

[Xabier Rodríguez Calvar]

(In reply to comment #7)
> Yes, this is certainly a huge inefficiencies in the current appcache implementation.
> 
> I don't think that hardcoding a limit is a great answer though. Keeping even a single application in memory doesn't make sense when it's large.
> 
> We need something much more like a regular resource cache, just backed by appcache database and not the network. That would respect system low memory notifications too.

With the approach I am trying, no app is stored in memory after certain amount of time, because it is evicted by the timer.

[Xabier Rodríguez Calvar]

(In reply to comment #8)
> (In reply to comment #7)
> > Yes, this is certainly a huge inefficiencies in the current appcache implementation.
> > 
> > I don't think that hardcoding a limit is a great answer though. Keeping even a single application in memory doesn't make sense when it's large.
> > 
> > We need something much more like a regular resource cache, just backed by appcache database and not the network. That would respect system low memory notifications too.
> 
> With the approach I am trying, no app is stored in memory after certain amount of time, because it is evicted by the timer.

With this I mean if there is any way that we can work out this patch and get it applied, knowing that we need to fix it better, of course, but as an improvement instead of a permanent solution?

[Xabier Rodríguez Calvar]

Created attachment 215643 [details]
Patch

Done some improvements for the moment the cache is created and added cache eviction for background.

[Build Bot]

Comment on attachment 215643 [details]
Patch

Attachment 215643 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/19598732

[Build Bot]

Comment on attachment 215643 [details]
Patch

Attachment 215643 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/19598747

[Build Bot]

Comment on attachment 215643 [details]
Patch

Attachment 215643 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/19628700

[Xabier Rodríguez Calvar]

Created attachment 216412 [details]
Patch

Fixed Mac build.

[Xabier Rodríguez Calvar]

(In reply to comment #7)
> I don't think that hardcoding a limit is a great answer though. Keeping even a single application in memory doesn't make sense when it's large.

It takes into account the cache only for the first pass, after the timeout that cache should be purged too.

> We need something much more like a regular resource cache, just backed by appcache database and not the network. That would respect system low memory notifications too.

Is there any way we could get landed even it is not the perfect solution? I think it is better than nothing.

[Brady Eidson]

Comment on attachment 216412 [details]
Patch

Assuming that patches for review since 2013 are stale, r-

[Anne van Kesteren]

AppCache is now disabled and will be removed in bug 219391.