454001 - use a smaller figure for browser.cache.memory.capacity

Status: RESOLVED INCOMPLETE

(Core :: Networking: Cache, defect)

Description

[Jo Hermans]

see bug 430061 comment 28

Since bug 105344, the size of the memory cache is calculated based on the amount of physical RAM. In my case, it's 18MB for 1GB of RAM. That cache is filled in about 30 minutes or so, and then older data is removed from the cache to make room for new data. The size of the cache was always difficult to determine, a big cache always met a lot of resistance for people that complained about the memory usage of Gecko based browsers.

But now we don't store images in the memory cache anymore (bug 430061), so you only see 2 or 3 MB of data inside for a normal browsing session. I tried to fill the whole 18MB in an afternoon, but couldn't (*). Maybe it's time to bring back the capacity to a smaller number ? A smaller number would trigger the expiration code when ther limit is reached, so that finally less memory is used. Maybe we don't even need the automatic calibration at all anymore.
 I'm now using browser.cache.memory.capacity set to 4096 (which is what Netscape 4 was using, I think), but even then it was difficult to reach it, unless you start loading huge pages like large bugzilla listings.

(*) quite often, I don't see any pages added to the cache at all. I don't know yet why, possible some cache-control header or such.

Comment 1

[Alfred Kayser]

Note, the memory cache is now only used for objects that are not allowed to be stored on disk (such as https objects and objects with 'Cache-Control: no-store', and even 'Cache-Control: no-cache, no-store').

(no-cache, but still in the memory cache????)

Comment 2

[Alfred Kayser]

By the way, even if the limit is 18MB, but in reality you are only able to get about 3MB in the memory cache, only 3MB is really used for it.

Comment 3

[Jo Hermans]

(In reply to comment #2)
> By the way, even if the limit is 18MB, but in reality you are only able to get
> about 3MB in the memory cache, only 3MB is really used for it.

Why do you think that ? It's currently 12076 KiB for me.

Comment 4

[Jo Hermans]

Note that it's currently possible that you don't see the memory section in about:cache at all, for instance immediately after you start up the application. The reason is that it's not printed when it's not initialized yet, which can happen since it's currently not used much anymore (see comment 1). I was a bit surprised when I first saw this.

Comment 5

[Alfred Kayser]

Not initialized because it is not used is perfect, but may be 'about:cache' should reflect this ("Memory cache not initialized"). B.t.w. it seems that the offline cache is initialized even if not used at all.

Comment 6

[Alfred Kayser]

Created bug 456574 for the postponing of the creation of the offline cache.

Comment 7

[Alfred Kayser]

Make that bug 456547.

Comment 8

[Benjamin Smedberg]

Joe, can you take this?

Comment 9

[Steve Chapel]

Attachment: patch to limit memory cache use to 8 MB

This patch limits the default amount of memory used for the memory cache to 8 MB. This should be large enough to hold all cached data during most sessions, but save up to 24 MB of RAM for sessions that make extensive use of the cache. The rounding factor is also larger to help make up for the fact that reported system RAM is often quite a bit lower than installed RAM (probably due to integrated graphics using the RAM).

Comment 10

[Alfred Kayser]

With Bug 531801: Should cache SSL content to disk even without Cache-Control: public there is even less demand for the memory cache.

Comment 11

[Jo Hermans]

I would limit it to even less. Note that you often find much less in the cache, even after surfing for hours. The memory cache is hardly used these days. Maybe a fixed number of 1MB ?

Comment 12

[Alfred Kayser]

Why not use a simple if/ifelse statement to achieve:
+ *   32 MB  2 MB
+ *   64 MB  4 MB
+ *  128 MB  6 MB
+ *  256 MB  8 MB
instead of the complex arithmtic, which is errorprone:
     double x = log(kBytesD)/log(2.0) - 14;
     if (x > 0) {
+        capacity = (PRInt32)(x * x / 3.0 + x + 0.999); // 0.999 for rounding
+        if (capacity > 8)
+            capacity = 8;
         capacity   *= 1024;
     } else {
         capacity    = 0;

just do:
if (x < 32) capacity = 2;
else if (x < 64) capacity = 4
else if (x < 128) capacity = 6
else capacity = 8

Comment 13

[Steve Chapel]

My patch doesn't limit the memory cache to just 1 MB because I don't want to cause a performance problem for users that do make extensive use of the memory cache, for example, if they browse large https pages.

My patch doesn't use a simple if/else because the arithmetic expression has already been tested, and an if/else will not degrade as gracefully as the expression, for example, if some memory is consumed by integrated graphics. With the if/else given, someone with 128 MB of RAM and integrated graphics would get 4 MB of cache instead of 6 MB. With my patch, they would get 5 or 6 MB of cache.

Comment 14

[Steve Chapel]

What we could do instead to address the concerns is something like:

if (x > 256)
    capacity = 4;
else if (x > 128)
    capacity = 2;
else
    capacity = 1;

This would result in the following cache allocations:

  RAM    Cache
  ---    -----
  128 MB  1 MB
  256 MB  2 MB
  512 MB  4 MB

This would be the case even if the integrated graphics occupied nearly half the RAM. Compared to the current cache allocations:

  RAM     Cache
  ---     -----
   32 MB   2 MB
   64 MB   4 MB
  128 MB   6 MB
  256 MB  10 MB
  512 MB  14 MB

the patch would always cut cache memory use to half (or less) of the current value. With a cache size of 4 MB on nearly every modern computer, it should not significantly impact performance (I hope) by failing to cache enough https pages. On a 512 MB machine, 4 MB is less than 1% of the RAM, so reducing cache memory use even further would have little impact on whether Firefox is causing memory problems for other programs.

Comment 15

[Jo Hermans]

(In reply to comment #13)
> My patch doesn't limit the memory cache to just 1 MB because I don't want to
> cause a performance problem for users that do make extensive use of the memory
> cache, for example, if they browse large https pages.

By default, they're not stored in the memory cache anymore (bug 531801) ... Only content that sets no-store is now left there, which is rather rare (private banking data, etc ... should use it). 

Try browsing for an entire day, and then see how much memory is actually used in the memory cache. I'm currently at 127 KB after 4 hours (!!!), and that's mostly gmail. And even if you see some content, then you're probably wondering why it's still begin stored after all these hours (especially if it's private data, which might be the major reason why the no-store header would be used).

I originally filed the bug when the images were removed form the cache, so a much smaller cache was needed (https-pages never got pushed out of the cache, because it was too large). Now, it's even less used as before. We can decrease the cache even more (hoping that at least some data might be pushed out after a while). Or maybe we can change it to a simple timer-based expiration ? Then we don't need the LRU-SP algorithm (with its soft and hard limits) anymore either, which is overkill for the current usage.

On the other hand, maybe there's an application that still uses the memory-cache a lot, for instance to avoid any disk cache at all. Maybe Thunderbird or one of the mobile apps ?

Comment 16

[Jason Duell]

I believe the mobile guys are turning off the disk cache and only using memory cache, so I'd want them to sign off on whatever we do here.  cc-ing dougt.

Comment 17

[Doug Turner (:dougt)]

we have turned off the disk cache due to performance reasons and were interested in getting performance numbers of using the disk cache with e10s.

Comment 18

[Johnny Stenback (:jst)]

Not blocking 2.0 on this.

Comment 20

[Jason Duell]

Bug 602611 suggests we cache recently-used items in memory and move them to disk later.  This would increase our demand for the memory cache.  I don't think we know what the right amount for that would be yet (and it's likely to be platform dependent).

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 426994 [details] [diff] [review]
patch to limit memory cache use to 8 MB

Moving this to jduell since he seems to better understand the current issues :-)

Comment 22

[Jason Duell]

Comment on attachment 426994 [details] [diff] [review]
patch to limit memory cache use to 8 MB

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

I'm going to punt on reviewing this for now, as the landscape here will change with bug 602611.  I.e. we'll use the memory cache a lot more. Note that that doesn't necessarily mean "keep a big RAM cache": we'll want enough to hold a working set of "very recently used, very likely to be used again soon" resources.   We'll want to gather some data to figure out the right size.

In the meantime, note that AFAICT the memory cache does not preallocate space, i.e. we only use up as much space as we have entries, at least according to about:memory (you need 'verbose' to see it).  So this bug is NOT about "we're wasting many MB of RAM on empty space we don't use", but rather "we let some very stale, mainly HTTPS-no-store entries sit around in the memory cache if you keep the browser open for a long time."  Which is less serious, but still worth fixing, perhaps, if bug 602611 doesn't happen soon enough.

Comment 23

[Joe Drew (not getting mail)]

It's not clear to me why I have this bug :)

Comment 24

[Patrick McManus [:mcmanus]]

fresh data in a new bug if this is believed to be actionable