Bug 913806 (cache2enable)

Turn HTTP cache v2 on by default on all products

RESOLVED FIXED in mozilla32

Status

()

defect
RESOLVED FIXED
6 years ago
10 months ago

People

(Reporter: mayhemer, Assigned: mayhemer)

Tracking

(Depends on 2 bugs, Blocks 5 bugs, {feature})

Firefox Tracking Flags

(relnote-firefox 32+)

Details

(Whiteboard: [cache2])

Attachments

(1 attachment, 2 obsolete attachments)

No description provided.
Depends on: 913807
Depends on: 836851
Depends on: 913808
Depends on: 913809
Depends on: 913813
Depends on: 913814
Depends on: 913817
Depends on: 913818
Depends on: 913820
Depends on: 913822
Depends on: 913823
Blocks: 913824
Blocks: 913828
Depends on: 913819
Blocks: 913809
No longer depends on: 913809
Blocks: http_cache
Depends on: 914644
Depends on: 914824
Depends on: 915296
Depends on: 917423
Blocks: 877301
Depends on: 920573
Depends on: 920606
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
Depends on: 922081
(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.
Depends on: 922659
Depends on: 922671
Depends on: 922123
Alias: cache2enable
Depends on: 923688
Keywords: feature
Depends on: 934610
Depends on: 926070
Posted patch v1 (obsolete) — Splinter Review
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Depends on: 937601
Depends on: 938186
Depends on: 939567
Depends on: 939574
Depends on: 940747
Depends on: 940749
Depends on: 941698
No longer depends on: 939574
No longer depends on: 913817
No longer depends on: 938186
No longer depends on: 920606
No longer depends on: 923688
No longer depends on: 914644
No longer depends on: 926070
No longer depends on: 913818
No longer depends on: 940747
No longer depends on: 940749
No longer depends on: 939574
No longer depends on: 913817
No longer depends on: 938186
No longer depends on: 920606
No longer depends on: 923688
No longer depends on: 914644
No longer depends on: 913817
No longer depends on: 920606
No longer depends on: 923688
No longer depends on: 938186
No longer depends on: 939574
No longer depends on: 914644
Depends on: 942835
Depends on: 945945
Depends on: 949175
Depends on: 949250
Depends on: 949251
Depends on: 950134
Depends on: 950164
No longer depends on: 913820
Depends on: 951713
Depends on: 956374
Depends on: 948566
Depends on: 924112
Depends on: 957155
Depends on: 957213
Depends on: 957228
Depends on: 957230
Depends on: 957243
Depends on: 957251
Depends on: 957707
Depends on: 958317
Depends on: 923688
No longer depends on: 958317
Depends on: cache2tests
No longer depends on: 957251
No longer depends on: 957243
No longer depends on: 957228
No longer depends on: 957213
No longer depends on: 957155
No longer depends on: 948566
Depends on: 959761
Depends on: 960902
Depends on: 963703
Depends on: 964039
No longer depends on: 913822
Depends on: 966114
Depends on: 967310
No longer depends on: 913819
Depends on: 967862
Depends on: 968106
No longer depends on: 923688
Depends on: 968593
No longer depends on: 966114
No longer depends on: 967310
Depends on: 968606
Depends on: 969160
No longer depends on: 969160
Depends on: 971975
Depends on: 975207
Depends on: 975255
No longer depends on: 967693
See Also: → 967693
Depends on: 976147
Depends on: 976157
Depends on: 976171
Depends on: 976217
Depends on: 976866
Depends on: 977766
Depends on: 975367
Depends on: 980527
Depends on: 916052
Depends on: 982598
Depends on: 986179
Depends on: 987814
Depends on: 987829
Depends on: 913819
Depends on: 988011
Depends on: 988063
No longer depends on: 913819
No longer depends on: 988011
Depends on: 988318
Depends on: 992974
Depends on: 994606
Depends on: 988489
Depends on: 913819
Depends on: 999383
No longer depends on: 975207
No longer depends on: 976217
No longer depends on: 988063
No longer depends on: 994606
Depends on: 1000338
No longer depends on: 1000338
Depends on: 1005079
Depends on: 1005475
Depends on: 1006181
Depends on: 1006263
Blocks: 1009122
Depends on: 1009500
Depends on: 1010153
Depends on: 1010221
Posted patch enable the cache (obsolete) — Splinter Review
Just merged to apply.
Attachment #829369 - Attachment is obsolete: true
Attachment #8423245 - Flags: review?(jduell.mcbugs)
Attachment #8423245 - Attachment is patch: true
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 on attachment 8423245 [details] [diff] [review]
enable the cache

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

Assuming gum results look good...
Attachment #8423245 - Flags: review?(jduell.mcbugs) → review+
No longer depends on: 913823
Attachment #8423245 - Attachment is obsolete: true
Attachment #8423445 - Flags: review?(jduell.mcbugs)
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.
Attachment #8423445 - Flags: review?(jduell.mcbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/d61ae091de9c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Depends on: 1012190
Duplicate of this bug: 648232
Duplicate of this bug: 837274
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.
Depends on: 1013281
Depends on: 1013285
Depends on: 1013333
(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.
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?
Flags: needinfo?(honzab.moz)
(In reply to Avi Halachmi (:avih) from comment #15)

browser.cache.use_new_backend_temp = false and keep browser.cache.use_new_backend = 0
Flags: needinfo?(honzab.moz)
(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?
(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.
Depends on: 1027612
relnote-firefox: --- → ?
(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?
Flags: needinfo?(honzab.moz)
Blocks: 1030975
Did not :)  Filed bug 1030975.
Flags: needinfo?(honzab.moz)
Depends on: 1032992
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
Flags: needinfo?(honzab.moz)
(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!
Flags: needinfo?(honzab.moz)
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
Flags: needinfo?(honzab.moz)
(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
Flags: needinfo?(honzab.moz)
Thanks. I used your blog post. More interesting than the doc ;)
Depends on: 1036939
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.
Depends on: 1053517
Depends on: 1065478
Depends on: 1066757
Depends on: 1066726
Depends on: 1066970
See Also: → 1060082
Depends on: 1127874
Depends on: 1131092
Duplicate of this bug: 717761
Is there any reason why this is not enabled by default after 2 years?
(In reply to kamil from comment #28)
> Is there any reason why this is not enabled by default after 2 years?

It is.
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?
(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.
(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?
(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.
You need to log in before you can comment on or make changes to this bug.