Context for others: data is from go/slow-memory-reports

Talked to ssid@ offline who showed me how to navigate in SlowReports to get those numbers

~100MB for HttpCache does sound excessive. 

I am only aware of a refcounting change (crrev.com/54237ca8c36a7cebf34d07c467ace33b5d7ff1e9) which might affect objects lifetime. I will look into it.

+ Internals > Network > Cache

I'd suggest to not strongly assume it regressed in this version range, though there is good probability.

Yep, I agree. The refcounting change isn't in the range. https://chromium.googlesource.com/chromium/src/+log/58.0.3026.0..58.0.3029.0?pretty=fuller&n=10000 

I will try to repro locally with heap profiler.

There may be some terminology confusion. The URLRequestContext is 130MB, and it's due to its http cache. My guess is that it's an incognito browser's memory cache. 130MB sounds about right for that.

I added support for the memory-tracing the incognito cache in that range, CL: https://codereview.chromium.org/2723553002

I believe this is WAI.

Ah, thanks Josh! That explains. The largest HttpCache that I had seen before in traces is still under 1MB. Do you know why incognito http cache can be as large as 130MB? I see a couple of traces with that usage from low-end devices with (0.5-1GB RAM).  Is there an upper limit for incognito's in-memory http cache? 

I will draft a CL to log the type of cache backend.

I browsed a few sites on my Nexus 7 (2GB physical RAM) and the in-memory HttpCache can exceed 40MiB easily.

MemBackendImpl::Init() caps the amount of memory to min(50MB, 2% of physical RAM) which seems reasonable. However, the limit doesn't seem to take effect.

I see traces from low-end devices (0.5 - 1GB RAM) have HttpCache as large as 130MB. An example: https://crash.corp.google.com/browse?stbtiq=58eeb2da60000000#6?OpenedMetric=memory:chrome:browser_process:reported_by_chrome:net:effective_size_max . 130MB is definitely not 0.02 of the physical RAM.

I looked at the heap profiler data (attached here). Most of memory goes to disk_cache::MemEntryImpl::InternalWriteData(int, int, net::IOBuffer*, int, bool) which matches what we see in net/ MemoryDumpProvider.

Maybe the calculation of the cap isn't quite working as expected?

Deleted: trace_chrome_android_40MB_http_cache.json 22.3 MB

trace_chrome_android_40MB_http_cache.json 22.3 MB Download

That looks like a problem!

Handing over this to Josh who is an expert in the cache code.  Thanks, Josh! SlowReports omits the origin of URLRequestContexts. I will verify that the problem is indeed coming from the incognito/off-the-record profile.

xunjieli@, in comment #7 you mention reaching over 40MB. It's true, your trace is slightly (2.8 MB) over that. That's likely due to the memory cache being less precise about memory usage than the estimated memory usage. Can you run a bit longer and see if it grows further? Browsing just one or two more pages should make it grow significantly if the cap is broken.

I can't reproduce the larger issue of 130MBs on a 1GB device. I replaced 1GB for the value of  "base::SysInfo::AmountOfPhysicalMemory" and it's appropriately setting the cap to 20MB and staying within a couple MB of that.

It'd be good to know what the max_size_ is on the 1GB device, what the cache thinks its size is, as well as what AmountOfPhysicalMemory is returning. xunjieli@ suggested placing those in the memory report so I'll do that.

The total value of malloc-ed objects reported by Android system is always higher than the cache size. It is likely that we would notice the discrepancy here if there is miscalculation in reporting.

How long does this cache live? Does it go away immediately once all incognito tabs are closed?

Curious if this cache reacts to memory pressure notifications in any way? It may be reasonable to drop some data in that situation.

I think we do not upload the reports if incognito mode is enabled any time while collecting the trace.

Icognito's HttpCache lives as long as Incognito profile is alive. This cache doesn't react to memory pressure yet. 

I have a CL to add more details so we know whether this HttpCache comes from a regular profile or an incognito. https://codereview.chromium.org/2772283003/

jkarlin@ has a CL (https://codereview.chromium.org/2779733002/) to add details on the type of cache backend.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4da1de6f90c7149e7b2e6db4ee66a84b355988a

commit c4da1de6f90c7149e7b2e6db4ee66a84b355988a
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Mar 28 16:19:50 2017

Make url_request_context dumps include context names

Background dumps do not include URLRequestContext's names because
string attributes are not allowed. SlowReports only has background
dumps. Without the name of the context, we don't know where the contexts
come from.

This CL includes the context names in the names for the allocator dumps.

BUG= 705053 , 669108 

Review-Url: https://codereview.chromium.org/2772283003
Cr-Commit-Position: refs/heads/master@{#460116}

[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/off_the_record_profile_io_data.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/off_the_record_profile_io_data.h
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/profile_impl_io_data.h
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/net/base/sdch_manager_unittest.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/net/http/http_cache_unittest.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/net/url_request/url_request_context.cc
[modify] https://crrev.com/c4da1de6f90c7149e7b2e6db4ee66a84b355988a/net/url_request/url_request_context.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a

commit 1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a
Author: jkarlin <jkarlin@chromium.org>
Date: Tue Mar 28 19:04:38 2017

[HttpCache] Store some memcache info to memory dumps for debugging

Add some memcache variables to the memory dumps (cache max_size, amount of
physical memory, and the size the backend thinks it is) so we can debug cases
where svelte devices have large memory backends.

BUG= 705053 

Review-Url: https://codereview.chromium.org/2779733002
Cr-Commit-Position: refs/heads/master@{#460181}

[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/backend_unittest.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/blockfile/backend_impl.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/blockfile/backend_impl.h
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/disk_cache.h
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/memory/mem_backend_impl.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/memory/mem_backend_impl.h
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/simple/simple_backend_impl.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/disk_cache/simple/simple_backend_impl.h
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/http/http_cache.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/http/mock_http_cache.cc
[modify] https://crrev.com/1f2ef177650cf95c87765a0c8a8ce4a758b5ff2a/net/http/mock_http_cache.h

mmenke@ pointed out that a single large resource could cause the memory backend to get too large. This appears to be true looking at the code. After each write, the backend will evict entries if necessary to stay under the cap, but it doesn't evict any open entries (e.g., the one currently writing). 

Possible solution: Stop caching if the resource gets too big for the memory backend. We can accomplish this by failing the write call in the backend. The HTTPCacheTransaction will doom the entry and continue the transaction by reading from the network without writing to the backend.

Unless there are objections I will apply the solution to #19 in all three backends.

SimpleCache memory use for payload/stream 1 writes is only for buffering (for passing to disk threads), so it's probably less pressing, and it should already be limiting things to about 1/8th of the cache already, which seems to work out to be typically somewhere in 25MB-40MB range for Android --- ref: https://cs.chromium.org/chromium/src/net/disk_cache/simple/simple_entry_impl.cc?rcl=0ac7ceb17518903b0072836ab722384bad87e24e&l=426

(Stream 0 is in memory, though)

SimpleCache.Http.WriteResult2 should also give some idea on how often that happens.

For SimpleCache and BlockFileCache I'm more concerned with using up more disk space than is expected as opposed to memory. This is more problematic on Android than other platforms.

The mem backend concern is enough to warrant a P1. 

It's actually a little more nuanced. A single large resource is only allowed to write up to 1/8th the mem cache size. So it takes multiple open resources to trigger this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c383a4d067322844776e4029c86803748f0526d0

commit c383a4d067322844776e4029c86803748f0526d0
Author: jkarlin <jkarlin@chromium.org>
Date: Thu Mar 30 18:19:59 2017

[HTTP Cache] Prevent memory backend from exceeding its max size

Prevents the memory backend from writing additional bytes if they will cause
the cache to exceed its max size.

The HttpCache tests already verify that they can handle a failed write after
the cache has already started writing, see
HttpCache.SimpleGETWithDiskFailures2.

BUG= 705053 

Review-Url: https://codereview.chromium.org/2784803004
Cr-Commit-Position: refs/heads/master@{#460826}

[modify] https://crrev.com/c383a4d067322844776e4029c86803748f0526d0/net/disk_cache/backend_unittest.cc
[modify] https://crrev.com/c383a4d067322844776e4029c86803748f0526d0/net/disk_cache/memory/mem_backend_impl.cc
[modify] https://crrev.com/c383a4d067322844776e4029c86803748f0526d0/net/disk_cache/memory/mem_backend_impl.h
[modify] https://crrev.com/c383a4d067322844776e4029c86803748f0526d0/net/disk_cache/memory/mem_entry_impl.cc

Josh's fix is included in 59.0.3057.0. I checked SlowReports today, there is no data from >= 59.0.3057.0 yet.

Updated the dashboard. Looks like we still have cases of 70MB on http_cache in version 3062.
https://crash.corp.google.com/browse?stbtiq=081b932a10000000#6?OpenedMetric=memory:chrome:browser_process:reported_by_chrome:net:effective_size_max

On a different note, can we reduce the max size on Android devices? Do we really need 20MB of http_cache size on 1GB device?
Eg:
https://crash.corp.google.com/browse?stbtiq=0158da2b90000000#6?OpenedMetric=memory:chrome:all_processes:reported_by_chrome:net:effective_size_max

Comment 28: The 70MB case is for a user with 3GB RAM. Interestingly, the memory backend's estimate is that it's 50MB (which is its max size) but it's actually 70MB if you do a more careful estimate (e.g., memory dump). This suggests that the memory backend should be more careful in its estimate.

Comment 29: 20MB seems pretty reasonable? Much smaller and I believe you'll suffer from increased network costs and performance degradation, which users of svelte devices also care about.

In-memory cache is not hooked up with memory pressure listener. Does it make sense to drop some entries when we are under memory pressure? 

Ssid@: This in-memory cache comes from an incognito profile. Looks like SlowReports does trigger even when incognito is alive. Please check if this is WAI.

Seems like there are still some open questions here.

1) Do we still see large instances of the cache in slow reports?
2) How much did this change help? Looking at UMA data, I wasn't able to see effect: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=114d6e17116b16e32793a557fb2d2993. Is it due to UMA privacy rules for uploading incognito data?
3) Sid, could you confirm privacy around slow reports + incognito behaviour?
4) Can we answer Helen's question about memory pressure?

It would be good to figure functional questions out for M-61 which is target release for Android Go.

This should be fixed by the changes I made here and Maks' changes to shrink the strings to the required size.