Over to cmumford for further triage.

Thx for the report Erik. This looks like it may be Blob related and not specific to leveldb. I'm setting the component and also setting back to untriaged so that this can go through our normal triage process. My guess is that it will be assigned to somebody more familiar with Blobs than I, but if not I'll jump in and see what I can learn.

This sounds like just a lack of memory reporting. I.e. BlobStorageContext which keeps track of all blobs that are still alive doesn't have any integration with the memory reporting infra. It indeed seems like it might be a good idea to make BlobStorageContext/BlobStorageRegistry a MemoryDumpProvider and start tracking memory used by blobs.

There doesn't seem anything wrong by itself with the blob system having 600MB of blobs in memory. By default on 64-bit desktops we let the blob system use upto 2GB of memory.

Perfect, thanks for the fast analysis. :)

The interface for setting up a new MDP is pretty straight forward, see https://cs.chromium.org/chromium/src/base/trace_event/memory_dump_provider.h?type=cs&q=memorydumpprovider&sq=package:chromium&l=19.

Happy to provider pointers if your team needs any assistance on that front. Adding some Android people, since this is presumably another potential large consumer of memory that might affect Android.

BlobStorageContext/BlobStorageRegistry can potentially use up to 2GB of memory on Desktop. We should make it a MemoryDumpProvider so that Chrome can accurately track major consumers of memory.

On android we'll use at most 1% of RAM (as reported by base::SysInfo::AmountOfPhysicalMemory), so it shouldn't really be a major consumer there (unless 1% already counts as major). But yeah, actually tracking this usage better is something that we definitely should do.

Do you have a link to a good primer for blobs? I'd like to get a better understanding of what these are before diving into a more detailed conversation. :)

https://chromium.googlesource.com/chromium/src/+/HEAD/storage/browser/blob/README.md tries to be that primer. Some of it is a bit outdated (I neglected to update it after mojofying blobs), but most of it should still be accurate.

Thanks for the link. I perused through it, the FileAPI spec, and the Mozilla link, but I'm still having trouble understanding the use case for large blobs. Could you describe some common examples?

The link says that we'll allow 2GB of blobs in RAM on a 4GB device. Throwing in memory usage for the kernel, integrated GPU, chrome itself [browser, GPU browser, multiple renderers, etc.] and we're probably already approaching the memory limits of the system. macOS and Windows implement compression, which allows real memory usage to be closer to ~6-8GB, but performance degrades pretty sharply due to thrashing. It's potentially the case that when we do allow 2GB of blob RAM on a 4GB device, the user experience will be significantly worse than if we had a lower limit [e.g. 100 MB]. 

All this being said, since I don't know the typical use cases for large blobs, I have no way of evaluating the performance trade offs. Maybe we could chat over VC?

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

commit 4d3f5a677626f40be17db36ecf5d71965b58d1fa
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Tue Feb 27 04:33:32 2018

[Blobs] Add minimal MemoryDumpProvider integration.

The Blob system can allocate a lot of memory, so we should integrate
with MemoryDumpProvider infra to account for this usage.

Bug:  813046 
Change-Id: Ifabb469c36afb68df3031fe6660a76a97274950b
Reviewed-on: https://chromium-review.googlesource.com/938761
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539402}
[modify] https://crrev.com/4d3f5a677626f40be17db36ecf5d71965b58d1fa/components/download/internal/background_service/in_memory_download_unittest.cc
[modify] https://crrev.com/4d3f5a677626f40be17db36ecf5d71965b58d1fa/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/4d3f5a677626f40be17db36ecf5d71965b58d1fa/storage/browser/blob/blob_storage_context.h

I'll defer to dmurph@ for any input he might have on how we ended up with the current limits. But yeah, VC sounds good to me if we include him.

Summary from VC:

Memory pressure signals don't really work, so assume they are unreliable / don't fire.

Plan:
* In our LRU cache data, store the time the ShareableBlobDataItem was accessed.
* Schedule a periodic 'sweep' of the blob storage that evicts items that haven't been accessed in a long time (hours?)

IMO, the cost of paging in memory from disk is relatively small compared to the cost of holding memory unnecessarily [which causes a global performance hit]. Does 10 minutes sound like a reasonable threshold?

[if the threshold is too low, will we have metrics that will tell us this so we can adjust?]

I'm making memory pressure signals on Android more reliable in issue 813909, so please don't remove memory pressure listener (if there is any). I think ChromeOS also fires memory pressure signals.

Seb is working on memory pressure signals for Windows: https://bugs.chromium.org/p/chromium/issues/detail?id=771478

And no one is working on it for macOS:
https://bugs.chromium.org/p/chromium/issues/detail?id=713463

> IMO, the cost of paging in memory from disk is relatively small compared to the cost of holding memory unnecessarily [which causes a global performance hit]. Does 10 minutes sound like a reasonable threshold?

Definitely agree that hours is probably too long. Not sure we want to go too low either though, because currently we don't have any way to page a blob back in to memory. Once we evict data it just stays on disk, and anytime we need to read the data we read it from disk. Not sure how bad that is/when performance of reading blobs actually matters, but certainly something to keep in mind when figuring out the best delay.

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

commit 62435902f2cd60535f7e56e752243269cd9c4277
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Sat Mar 03 02:56:25 2018

[Blobs] Rename memory dump to site_storage/blob_storage_*.

Bug:  813046 
Change-Id: Ie8775a418059b5c69dd47ea06668101e69f6ee72
Reviewed-on: https://chromium-review.googlesource.com/939714
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540730}
[modify] https://crrev.com/62435902f2cd60535f7e56e752243269cd9c4277/storage/browser/blob/blob_storage_context.cc

Mek: sounds like the next step here is regularly evicting things from blob storage.

Is that correct? Should this be considered a P2?

Yes, agreed that that is the next step. Should probable actually have a separate bug for this, since this bug is about the lack of reporting. Actually improving on the usage seems like a mostly entirely separate issue.

Mek: can you file the appropriate bug?

Filed issue 824826, and closing this one.