Last Comment Bug 742560 - Increase Fennec's disk cache size
: Increase Fennec's disk cache size
Product: Core
Classification: Components
Component: Networking: Cache (show other bugs)
: unspecified
: x86 Android
-- normal (vote)
: mozilla14
Assigned To: Geoff Brown [:gbrown]
: Patrick McManus [:mcmanus]
Depends on:
  Show dependency treegraph
Reported: 2012-04-04 15:27 PDT by Brad Lassey [:blassey] (use needinfo?)
Modified: 2012-04-24 08:29 PDT (History)
10 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

increase max disk cache size to 20MB (981 bytes, patch)
2012-04-13 14:43 PDT, Geoff Brown [:gbrown]
mark.finkle: review+
Details | Diff | Splinter Review

Description User image Brad Lassey [:blassey] (use needinfo?) 2012-04-04 15:27:14 PDT
We currently use a smaller disk cache than other browsers, and I suspect that may be hurting us.
Comment 1 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-04-04 16:19:56 PDT
Some data on existing cache sizes:
(search for 'Galaxy Nexus: 18 MB')

Comment 2 User image Brad Lassey [:blassey] (use needinfo?) 2012-04-11 09:01:30 PDT
Geoff, do you have a strategy for picking an appropriate size here?
Comment 3 User image Geoff Brown [:gbrown] 2012-04-11 09:31:20 PDT
I think there is tension between cache effectiveness (cache it if you can / maximize cache size) and user tolerance for loss of usable storage ("firefox is a disk hog -- it's using 50% of my available storage!" / minimize use of storage).

As a starting point, I would like us to be comparable to other mobile browsers, so I am testing several browsers on several devices to get a better idea of cache sizes for other browsers.
Comment 4 User image Geoff Brown [:gbrown] 2012-04-12 17:22:58 PDT
Oh darn, it's complicated!

Max cache size of the stock browser varies across devices. I monitored the size of the stock browser cache directory on various devices while browsing and noted the approximate maximum size on each:

Nexus 1 - Android 2.2 -         6 MB
Galaxy S - Android 2.2.1 -      6 MB
Galaxy Nexus - Android 4.0.3 - 24 MB
Galaxy Tab - Android 3.1 -     35 MB

The Dolphin browser appears similar: 10 MB on Galaxy S and 30 MB on Galaxy Tab.

We should also keep in mind that other browsers typically use the Android cache directory while Fennec does not - bug 742558.

Storage space varies widely across devices: Newer devices and tablets especially have much more space - and tolerance for storage-intense apps - than older phones. Perhaps the smart-sizing feature could be expanded to accommodate mobile? That might be the best long-term solution.

In the short-term, 10 MB seems pretty reasonable, but perhaps we could make it 20 MB: see how that affects telemetry hit rates, and if it generates any complaints.
Comment 5 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-04-12 17:55:19 PDT
We could probably check for the amount of RAM on the device and adjust accordingly.
Comment 6 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-04-12 17:56:57 PDT
(In reply to Mark Finkle (:mfinkle) from comment #5)
> We could probably check for the amount of RAM on the device and adjust
> accordingly.

Which is what you suggested in comment 4. I'd rather err on the larger size to start with as well.
Comment 7 User image Geoff Brown [:gbrown] 2012-04-13 14:43:47 PDT
Created attachment 614918 [details] [diff] [review]
increase max disk cache size to 20MB

I reported bug 745340 to encourage a better solution, but anticipate that may take a while to sort out. Meanwhile, let's bump to 20 MB.
Comment 8 User image Mark Finkle (:mfinkle) (use needinfo?) 2012-04-13 19:10:41 PDT
Comment on attachment 614918 [details] [diff] [review]
increase max disk cache size to 20MB

Let's see what data we get. Reverting is easy.
Comment 9 User image Ryan VanderMeulen [:RyanVM] 2012-04-14 16:44:53 PDT
Comment 10 User image :Ms2ger (⌚ UTC+1/+2) 2012-04-15 04:37:59 PDT
Comment 11 User image Doug Turner (:dougt) 2012-04-19 11:04:18 PDT

I awhile ago we enabled disk cache to collect telemetry.  I understand that the disk cache caused startup and some jank woes.  Do you still have that data?

Reopening to sort out.
Comment 12 User image Brad Lassey [:blassey] (use needinfo?) 2012-04-19 11:06:31 PDT
closing the bug again, the patch has been pushed and this has been resolved
Comment 13 User image (dormant account) 2012-04-19 11:50:21 PDT
Our cache has some massive problems...including janking the main thread, blowing itself away on uncleanshutdown, etc. 
I thought we disabled disk cache because it provided more pain than gain...what changed?
Comment 14 User image (dormant account) 2012-04-19 11:55:33 PDT
I can't find any bug#s to do with disabling the cache, but here is an email I wrote to relevant people:

NETWORK_DISK_CACHE_OPEN is almost never 0(unlike on desktop), indicating that we are doing something expensive there.  A large number of those are 100-3000ms. No other startupy histogram metrics are similarly slow.

On the non-startup side:
Various histograms that measure time to fetch entry from disk are also on the expensive side(ie HTTP_PAGE_CACHE_READ_TIME).
HTTP_OPEN_TO_FIRST_FROM_CACHE is not obviously better than HTTP_OPEN_TO_FIRST_FROM_RECEIVED...infact the median time is worse.

Note it seems like the recent landing of bug 648429 has shifted HTTP_OPEN_TO_FIRST_FROM_CACHE to the left(ie less time spent reading). This effect is quite obvious on fennec, not so obvious in desktop firefox.
Comment 15 User image Doug Turner (:dougt) 2012-04-19 11:58:57 PDT
I think it was our mistake not disabling the disk cache on m-c after we collected this data.  We should disable the disk cache given this data you collected.
Comment 16 User image Geoff Brown [:gbrown] 2012-04-19 13:25:12 PDT
Bug 645848 enabled disk cache for fennec. 

Bug 717031 disabled it on aurora only, for FF11, citing 3 bugs: 707436, 713480 and 716293. 707436 and 716293 are resolved/fixed; 713480 applies only to a disabled feature.

NETWORK_DISK_CACHE_OPEN has not improved much, but HTTP_PAGE_OPEN_TO_FIRST_FROM_CACHE is now substantially better than HTTP_PAGE_OPEN_TO_FIRST_RECEIVED (for 13.0, median of approx 100ms vs approx 700 ms.)
Comment 17 User image (dormant account) 2012-04-19 14:57:33 PDT
I can't argue about this because the telemetry dashboard is not reporting population samples atm :( bug 747192
Comment 18 User image JP Rosevear [:jpr] 2012-04-24 07:23:10 PDT
Should we re-open this bug until bug 747192 is resolved?
Comment 19 User image Doug Turner (:dougt) 2012-04-24 08:29:03 PDT
Hey jp,

We spent a few weeks (a few years ago) looking for a major perf regression that turned out to be the disk cache. I think we need *overwhelming* data that suggests that enabing the disk cache is a good idea.  Right now, we do not have that.  If we do, please point me at it.

The disk cache is a major feature that hasn't proved itself yet.  This late in the game, it is best to cut it.  There will be future releases in which we can completely verify that enabling this feature is a good idea.

I will let you and brad sort out which bugs to open or reopen.

Note You need to log in before you can comment on or make changes to this bug.