Page MenuHomePhabricator

Limit the filesystem cache size of Picasso library.
Closed, ResolvedPublic2 Estimated Story Points

Description

The Picasso library (which we use to load images from the network) automatically caches the images in local storage. This is generally a nice feature, but it seems to cache the images without any bounds. We should investigate the possibility of hooking into Picasso's caching logic, and trim the cache when it exceeds a certain limit.

Event Timeline

Dbrant raised the priority of this task from to Needs Triage.
Dbrant updated the task description. (Show Details)
Dbrant subscribed.
KLans_WMF set Security to None.

Per Bernd's link above, Picasso will simply use OkHttp's disk cache, if configured, so limiting its size looks like a matter of adjusting line 15 of OkHttpConnectionFactory.java:

private static final long HTTP_CACHE_SIZE = 16 * 1024 * 1024;

The question is, how much smaller do we want it to be? Is there an error or something that I can reliably trigger and then try to eliminate by adjusting the cache size downward?

@Mholloway I guess you want to browse to a lot of pages and see if the app cache stops growing at some point.
It would be good to (at least temporarily) add some debug logging of the OkHttp Cache sizes. That should help get a better picture of it.

I did some testing/logging. Under the hood, OkHttp's Cache is a DiskLruCache with some added optimization functionality, and it definitely maxes out at the value set in HTTP_CACHE_SIZE (currently 16MB).

See my writeup in the task above.

tl;dr: I don't think this limit applies to the series of events causing the crashes.

I did some testing/logging. Under the hood, OkHttp's Cache is a DiskLruCache with some added optimization functionality, and it definitely maxes out at the value set in HTTP_CACHE_SIZE (currently 16MB).

Based on the above comment, I'm fairly convinced that this is no longer an issue. My guess is that the reports we've gotten about running out of storage space were from devices that were already critically low on storage, and wouldn't have benefited from additional constraints on our caching.