913806 - (cache2enable) Turn HTTP cache v2 on by default on all products

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Comment 1

[fxtech]

could you let cachev2 use the path specified on browser.cache.disk.parent_directory  ? it would help to avoid backupping useless data during my daily disk backup

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to fxtech from comment #1)
> could you let cachev2 use the path specified on
> browser.cache.disk.parent_directory  ? it would help to avoid backupping
> useless data during my daily disk backup

File bug 922081 on it.

Comment 3

[Honza Bambas (:mayhemer)]

Attachment: v1

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: enable the cache

Just merged to apply.

Comment 5

[Honza Bambas (:mayhemer)]

And here is the current Gum push: https://tbpl.mozilla.org/?tree=Gum&rev=46221d0b440e

(please ignore the Linux x64 ASAN JP timeouts, those are known and not run on m-c/m-i)

Comment 6

[Jason Duell]

Comment on attachment 8423245 [details] [diff] [review]
enable the cache

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

Assuming gum results look good...

Comment 7

[Honza Bambas (:mayhemer)]

Attachment: enable the new cache using the _temp pref

Comment 8

[Jason Duell]

Comment on attachment 8423445 [details] [diff] [review]
enable the new cache using the _temp pref

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

FYI: the reason we're using the _temp pref is so that in case we need to back out, people who opted-in to new_cache=1 won't have that pref unset.  If we stick we'll get rid of the _temp pref and make new_cache=1 the default.

Comment 9

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d61ae091de9c

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d61ae091de9c

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

When the new cache was turned on we saw a ~15MB bump in RSS memory usage and ~11MB bump in Explicit memory usage:
https://areweslimyet.com/mobile/

It looks like the Explicit jump is due to network/cache2/disk-storage() which wasn't reported before turning on, but is now ~10MB. Is this memory that just wasn't reported before with the old cache?

I don't know the specific cause of the ~15MB RSS jump.

Comment 14

[Honza Bambas (:mayhemer)]

(In reply to Mark Finkle (:mfinkle) from comment #13)
> When the new cache was turned on we saw a ~15MB bump in RSS memory usage and
> ~11MB bump in Explicit memory usage:

Filed bug 1013333 on this.

Comment 15

[Avi Halachmi (:avih)]

Looking at the patch, it's not clear to me how to use prefs to switch the new cache off.

It adds a new pref:
browser.cache.use_new_backend_temp = true

It keeps the old pref with the same value:
browser.cache.use_new_backend = 0

How do I switch it off now for the sake of some testing?

Comment 16

[Honza Bambas (:mayhemer)]

(In reply to Avi Halachmi (:avih) from comment #15)

browser.cache.use_new_backend_temp = false and keep browser.cache.use_new_backend = 0

Comment 17

[Avi Halachmi (:avih)]

(In reply to Honza Bambas (:mayhemer) from comment #16)
> (In reply to Avi Halachmi (:avih) from comment #15)
> 
> browser.cache.use_new_backend_temp = false and keep
> browser.cache.use_new_backend = 0

Thanks.

I'm guessing that at some stage in the future browser.cache.use_new_backend_temp will go away and be ignored, and browser.cache.use_new_backend = 0 or 1 will control if it's the new or old cache?

Comment 18

[Honza Bambas (:mayhemer)]

(In reply to Avi Halachmi (:avih) from comment #17)
> (In reply to Honza Bambas (:mayhemer) from comment #16)
> > (In reply to Avi Halachmi (:avih) from comment #15)
> > 
> > browser.cache.use_new_backend_temp = false and keep
> > browser.cache.use_new_backend = 0
> 
> Thanks.
> 
> I'm guessing that at some stage in the future
> browser.cache.use_new_backend_temp will go away and be ignored, and
> browser.cache.use_new_backend = 0 or 1 will control if it's the new or old
> cache?

My idea is to remove _temp before merge to aurora.  _new should go away with the old cache code which is not gonna happen this release.

Comment 19

[Florian Bender]

(In reply to Honza Bambas (:mayhemer) from comment #18)
> My idea is to remove _temp before merge to aurora.  _new should go away with
> the old cache code which is not gonna happen this release.

(Where) Did this happen?

Comment 20

[Honza Bambas (:mayhemer)]

Did not :)  Filed bug 1030975.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Honza, I am working on the release notes for 32 and 33 and this bug has been marked. Do you confirm that this feature is enabled by default in 32? Thanks

Comment 22

[Honza Bambas (:mayhemer)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #21)
> Honza, I am working on the release notes for 32 and 33 and this bug has been
> marked. Do you confirm that this feature is enabled by default in 32? Thanks

Yes, it sticks in 32!

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Thanks Honza.
I added "New HTTP caching (v2) enabled by default" to the release notes.

I have a few more questions for you:
* Do you have some documentations on this? Blog post, MDN? I would like to add a link for advanced users.
* Does it impact also Firefox Android?
* Are you happy about the wording?

Thanks

Comment 24

[Honza Bambas (:mayhemer)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Thanks Honza.
> I added "New HTTP caching (v2) enabled by default" to the release notes.
> 
> I have a few more questions for you:
> * Do you have some documentations on this? Blog post, MDN? I would like to
> add a link for advanced users.

You can use:
http://www.janbambas.cz/new-firefox-http-cache-enabled/
https://developer.mozilla.org/en-US/docs/HTTP_Cache

> * Does it impact also Firefox Android?

It does and it should be a positive impact :)

> * Are you happy about the wording?

Yep, thanks.

> 
> Thanks

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Thanks. I used your blog post. More interesting than the doc ;)

Comment 26

[Ed Morley [:emorley]]

Note we're not actually running our tests with the new cache enabled - and there are currently leaks if it is switched on for them:
https://tbpl.mozilla.org/?tree=Try&rev=a1ea7549c4fd

Bug 1053517 is an absolutely blocker for leaving this on release IMO.

Comment 28

[kamil]

Is there any reason why this is not enabled by default after 2 years?

Comment 29

[Honza Bambas (:mayhemer)]

(In reply to kamil from comment #28)
> Is there any reason why this is not enabled by default after 2 years?

It is.

Comment 30

[marvinhk]

this may sound stupid, am I having the benefit of new cache having prefs as followed:
user_pref("browser.cache.use_new_backend", 0);
user_pref("browser.cache.use_new_backend_temp", true);

or I have to flip browser.cache.use_new_backend = 1 as well?

Comment 31

[Honza Bambas (:mayhemer)]

(In reply to marvinhk from comment #30)
> this may sound stupid, am I having the benefit of new cache having prefs as
> followed:
> user_pref("browser.cache.use_new_backend", 0);
> user_pref("browser.cache.use_new_backend_temp", true);
> 
> or I have to flip browser.cache.use_new_backend = 1 as well?

You have it right.  the _temp pref is a leftover from times we were testing.  The prefs will be gone when we remove the old cache code altogether.

Comment 32

[jwms]

(In reply to Honza Bambas (:mayhemer) from comment #31)
> (In reply to marvinhk from comment #30)
> > this may sound stupid, am I having the benefit of new cache having prefs as
> > followed:
> > user_pref("browser.cache.use_new_backend", 0);
> > user_pref("browser.cache.use_new_backend_temp", true);
> > 
> > or I have to flip browser.cache.use_new_backend = 1 as well?
> 
> You have it right.  the _temp pref is a leftover from times we were testing.
> The prefs will be gone when we remove the old cache code altogether.

For clarification, the browser.cache.use_new_backend pref doesn't do anything anymore since we've been on the new one for several years, correct?

Comment 33

[Honza Bambas (:mayhemer)]

(In reply to jwms from comment #32)
> For clarification, the browser.cache.use_new_backend pref doesn't do
> anything anymore since we've been on the new one for several years, correct?

Yes, use_new_backend_temp one has wight over use_new_backend.