| Summary: | Try to keep AppCache memory caches in control | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Xabier Rodríguez Calvar <calvaris> | ||||||||||
| Component: | WebCore Misc. | Assignee: | Xabier Rodríguez Calvar <calvaris> | ||||||||||
| Status: | RESOLVED WONTFIX | ||||||||||||
| Severity: | Normal | CC: | andersca, annevk, ap, beidson, buildbot, commit-queue, darin, japhet, kling, koivisto, rniwa, sam | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Xabier Rodríguez Calvar
2013-10-17 04:51:51 PDT
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.
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 Comment on attachment 214441 [details] Patch Attachment 214441 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/3480051 Created attachment 214454 [details]
Patch
AtFixed compile issue
Could you please explain your approach? It's hard to see how code proves a concept when the concept is kept secret :) (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. 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. (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. (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? Created attachment 215643 [details]
Patch
Done some improvements for the moment the cache is created and added cache eviction for background.
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 Comment on attachment 215643 [details] Patch Attachment 215643 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/19598747 Comment on attachment 215643 [details] Patch Attachment 215643 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/19628700 Created attachment 216412 [details]
Patch
Fixed Mac build.
(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. Comment on attachment 216412 [details]
Patch
Assuming that patches for review since 2013 are stale, r-
AppCache is now disabled and will be removed in bug 219391. |