DiscardableSharedMemoryManager should have lower limits or smarter purging on Desktop |
||||||
Issue descriptionRepro steps: 1) Load facebook.com [timeline view] 2) Scroll down for a long time. Observations: There are ~100MB of malloc memory that isn't freed. Taking a heap dump shows that some of this memory is being created in cc::GpuImageDecodeCache::DecodeImageIfNecessary. GpuImageDecodeCache appears to rely on the discardable functionality in the function GpuImageDecodeCache::DecodeImageIfNecessary. Header comment: """ // CPU-side decoded data is stored in software // discardable memory and is only locked for short periods of time (until the // upload completes). """ However, DiscardableSharedMemory is only implemented on Android. """ // Returns SUCCESS on platforms which do not support discardable pages. DiscardableSharedMemory::LockResult LockPages(const SharedMemory& memory, size_t offset, size_t length) { #if defined(OS_ANDROID) SharedMemoryHandle handle = memory.handle(); if (handle.IsValid()) { int pin_result = ashmem_pin_region(handle.GetHandle(), offset, length); if (pin_result == ASHMEM_WAS_PURGED) return DiscardableSharedMemory::PURGED; if (pin_result < 0) return DiscardableSharedMemory::FAILED; } #endif return DiscardableSharedMemory::SUCCESS; } """
,
Feb 8 2018
I remembered we do OnPurge when the tab is backgrounded. So kSuspendedMaxItemsInCacheForGpu of 0 should free all the unlocked cache then.
,
Feb 12 2018
I think this is working as intended. Note that the discardable memory system, while not "true" discardable on non-Android systems, does have limits and limit enforcement. Any time an allocated discardable memory segment is fully unlocked, we set a cross-process flag indicating this: https://cs.chromium.org/chromium/src/base/memory/discardable_shared_memory.cc?rcl=52575f295595a93be8f836161ea064adb2126f0e&l=325 On the service (browser) side, any time we allocate new discardable memory, hit memory pressure (and maybe a few other cases), we purge any unlocked memory until we're within an acceptable limit. This does mean that, barring any sort of purging / pressure, Discardable memory will grow to the OS specified limit (see here: https://cs.chromium.org/chromium/src/components/discardable_memory/service/discardable_shared_memory_manager.cc?rcl=52575f295595a93be8f836161ea064adb2126f0e&l=157). These appears to be the lesser of 512MB or 1/4 of system memory on Mac. If we're using too much memory in these cases, it probably means that the Discardable system itself should introduce some additional limiting (timeout? lower limits?). Assigning to you to make the call whether this seems OK, or if we should pursue further work here. Feel free to assign back.
,
Feb 12 2018
Here's the service side purging logic, forgot to link in last post: https://cs.chromium.org/chromium/src/components/discardable_memory/service/discardable_shared_memory_manager.cc?rcl=52575f295595a93be8f836161ea064adb2126f0e&l=542
,
Feb 12 2018
Memory pressure signals are in general pretty busted on desktop. e.g. the macOS OS signals fire at very different times than their Windows counterparts, and fire way too late. +sebmarchand is currently investigating trying to create a reasonable memory pressure signal for windows. I suspect [confirm please?] that the current limits were chosen fairly arbitrarily. 512MB or 1/4 system memory seems quite large for macOS and Windows. If I had to choose arbitrary numbers on my own, I'd suggest 256MB or 1/16 system memory. In general, a short spike in memory usage seems like not-a-big-deal, but we probably don't want to keep around all that memory [since the memory pressure signals are unreliable]. Maybe some timeout of X-minutes since last access would be a reasonable server-side way to purge DiscardableSharedMemory on Desktop.
,
Feb 12 2018
There's been some discussions about this on issue 794738 and I think that we all agree that the current pressure signal (on Windows at least) is pretty bad. The threshold is either 200MB or 400MB depending on the total amount of memory you have, but in both cases it seem to be pretty inefficient. Windows try to ensure that there's always a few hundreds MB of "free" memory available, and so even when the system is thrashing we rarely hit these limits... I ran some experiments on some low-end hardware and I've seen some cases where the amount of free memory slightly increased as soon as the system started thrashing. See this graph: https://docs.google.com/spreadsheets/d/1n98MYGYbXMXcaCJV94yOLwZ2BaIDWhMcUa60YsklHj8/edit?usp=sharing#gid=2068274126 , in this experiment I've used an extension to automatically cycle through a set of 8 tabs on a cheap Windows laptop (2GB of RAM), you can see at t~=40s that the amount of free physical memory suddenly increases to ~400MB, despite the system being in a really bad state. But it's not always like this... In this test https://docs.google.com/spreadsheets/d/1n98MYGYbXMXcaCJV94yOLwZ2BaIDWhMcUa60YsklHj8/edit?usp=sharing#gid=710385501 I've kept Chrome idle for 10 mins (with 8 tabs opened), then I've cycled through these tabs for 10 minutes and finally I've returned to an idle state. You can see that the amount of free memory significantly decrease when the thrashing starts (from 600MB to 200MB), which cause the memory pressure signal to trigger for a short period of time and then the amount of free memory goes back to 400MB (at this point the system is still thrashing). These experiments should probably be re-done properly, but I think that it demonstrate that the current pressure signal is pretty bad. I'm still trying to find a better memory pressure signal, the hard-page fault rate would be really nice to have but it's really expensive to measure... The decrease in free physical memory associated with an increase of Chrome's total WS might be a pretty good pressure indicator.
,
Feb 12 2018
reveman@, are you still a good owner for DiscardableSharedMemoryManager internals issues, or can you help triage? TL;DR: The issue we're hitting is that, on Desktop, it appears that DiscardableSharedMemoryManager will just keep building up unlocked memory until we hit the 512MB or 1/4 system memory limit. At this point, it appears that memory usage will never go down (unless we hit a memory pressure signal, which isn't reliable on many OSes). It feels like we should either lower these limits (512MB of discardable is quite a lot to use on a 2GB machine), or add some sort of time-based purging. WDYT?
,
Feb 13 2018
We might be able to lower the limit now that we have better image caching logic. The 512MB limit was chosen before we had that. Maybe try reducing it to 128MB as currently used on Android?
,
Feb 14 2018
ericrk: Does that sg to you?
,
Feb 26 2018
I think that lowering to 128MB will likely have a negative impact on Software Raster image caching - it's easy for a site to use more than 128MB of images, and a 128MB discardable limit will mean that we're constantly discarding things.The Software cache is currently set up to use up to 256MB of memory on 4GB+ machines before hitting a degraded-performance mode. I think the underlying discardable limits should at least match this to avoid regressions. I sort of feel like a time-based purging mechanism would make more sense, as this allows for temporary spikes if we need the memory (being locked frame-over-frame), but will purge down to a reasonable limit if we go more idle. Alternately, we could say that consumers of discardable are responsible for time-based purging... in which case we could probably leave things as-is. This would sort of change the intended use pattern for discardable though? reveman@, wdyt?
,
Feb 26 2018
Time based purging logic for discardable memory seems reasonable. Don't we have some form of time based purging logic in Chrome already these days? I think the plan used to be that when we have some form of central memory coordinator in chrome, then we'd be able to use the signals from that to control the amount of discardable memory we keep alive. I'm not sure what the status is but it looks like there's some MemoryCoordinatorClient interface that is used by the tab manager code. Maybe this is the right place to create some time based purging logic for disacrdable memory?
,
Mar 21 2018
ericrk: Ping, is it reasonable to put in some simple time based purging logic now?
,
Apr 9 2018
Issue 810490 has been merged into this issue.
,
Apr 21 2018
Sorry for the delay here: My understanding is that MemoryCoordinator/MemoryCoordinatorClient isn't enabled except for the very limited Purge/Suspend mechanisms. I'm also not sure if this is the right place to put time-based purging logic, as the memory coordinator is centralized in the browser and communicates with clients over (1-way) IPCs, making it a bit high-overhead for time-based purging which could happen process-locally. Maybe all we really need is a helper API to simplify the various places we implement time-based purging? I'll see if there's something simple we can put in now. Depending on complexity, this may be better handled by someone else, as I haven't really worked directly in the discardable memory impl (only used it as a consumer). |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by keishi@chromium.org
, Feb 8 2018