Closed Bug 1456871 Opened 2 years ago Closed 2 years ago

Consider increasing disk cache size

Categories

(Core :: Networking: Cache, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: michal, Assigned: michal)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

Maximum disk cache size set by smart size code is now 350MB. 1/8 of the size can be easily eaten by a single JS entry containing byte code in alt-data (see https://bugzilla.mozilla.org/show_bug.cgi?id=1448476#c14). We experienced some performance issues with the old cache when we set size to 1GB but the new cache should handle bigger size without any issue (except clearing the cache during shutdown, see bug 1356853).
I agree, and to be honest I've already changed that in my working profile a long ago to exactly 1GB, no issues.

Regarding the delete on shutdown, I think it might be reasonable to make cache size smaller when that option is on.  No one will notice and we will shutdown faster.
Attached patch patchSplinter Review
Assignee: nobody → michal.novotny
Attachment #8979744 - Flags: review?(honzab.moz)
Comment on attachment 8979744 [details] [diff] [review]
patch

Review of attachment 8979744 [details] [diff] [review]:
-----------------------------------------------------------------

are the number chosen just arbitrarily?  if yes, how do you plan to monitor any impact?

::: netwerk/cache2/CacheFileIOManager.cpp
@@ +4198,2 @@
>      avail10MBs = 50;
>    }

was this code always wrong?  if avail10MBs > 700 => avail10MBs = 700.  and then definitely avail10MBs > 50.  but maybe I'm just missing how this code has to work...
Attachment #8979744 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #3)
> are the number chosen just arbitrarily?  if yes, how do you plan to monitor
> any impact?

I just guessed them. My feeling was that using 1GB when there is 100GB free space is very conservative, so now we use 1GB when free space is 25GB. OTOH I reduced the cache size when free space is lower than 0.5GB. I don't think this needs to be monitored.

> ::: netwerk/cache2/CacheFileIOManager.cpp
> @@ +4198,2 @@
> >      avail10MBs = 50;
> >    }
> 
> was this code always wrong?  if avail10MBs > 700 => avail10MBs = 700.  and
> then definitely avail10MBs > 50.  but maybe I'm just missing how this code
> has to work...

This is IMO correct, it works so that we enter all subsequent if-statements. E.g. if there is 8GB free space we take 2.5% from 1GB, then we enter the next one and take 7.5% from 6.5GB and finally we add 30% from 0.5GB (on desktop).
Keywords: checkin-needed
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6657c5618f0
Consider increasing disk cache size, r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b6657c5618f0
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.