Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(229)

Issue 2763933002: memory coordinator: Purge memory under memory pressure (Closed)

Created:
11 months ago by bashi
Modified:
10 months, 3 weeks ago
CC:
chromium-reviews, jam, darin-cc_chromium.org, mlamouri+watch-content_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

memory coordinator: Purge memory under memory pressure - On WARNING condition, request a backgrounded child process to purge memory - On CRITICAL condition, try to discard a tab first. If it failed, try to purge memory from all child processes. It it failed, try to purge memory from the browser process - Use somewhat conservative heuristics for purging - Browser: suppress purging for 2 mins after a purge - Renderer: after purging, suppress the next purging until the renderer goes foreground -> background then remains backgrounded for 30 secs BUG=696844 Review-Url: https://codereview.chromium.org/2763933002 Cr-Commit-Position: refs/heads/master@{#460622} Committed: https://chromium.googlesource.com/chromium/src/+/b9058f285cf483b7c49fe46e554f55f54ffeb956

Patch Set 1 #

Patch Set 2 : update #

Patch Set 3 : tweak #

Patch Set 4 : Tweak #

Patch Set 5 : Use int64_t #

Total comments: 26

Patch Set 6 : comments #

Patch Set 7 : discard tab #

Total comments: 5

Patch Set 8 : comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+247 lines, -5 lines) Patch
M content/browser/memory/memory_condition_observer.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M content/browser/memory/memory_coordinator_impl.h View 1 2 3 4 5 6 4 chunks +25 lines, -0 lines 0 comments Download
M content/browser/memory/memory_coordinator_impl.cc View 1 2 3 4 5 6 6 chunks +76 lines, -4 lines 0 comments Download
M content/browser/memory/memory_coordinator_impl_unittest.cc View 1 2 3 4 5 6 2 chunks +104 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 5 6 7 2 chunks +21 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 2 chunks +18 lines, -0 lines 0 comments Download

Messages

Total messages: 44 (28 generated)
bashi
I've been doing local experiments on my ChromeOS and Android devices to figure out a ...
11 months ago (2017-03-23 02:58:06 UTC) #4
haraken
On 2017/03/23 02:58:06, bashi (slow til Mar 23) wrote: > I've been doing local experiments ...
11 months ago (2017-03-23 05:19:16 UTC) #5
bashi
Added tests. Could you take another look?
11 months ago (2017-03-24 07:50:03 UTC) #13
haraken
Mostly looks good. https://codereview.chromium.org/2763933002/diff/80001/chrome/browser/memory/tab_manager.cc File chrome/browser/memory/tab_manager.cc (right): https://codereview.chromium.org/2763933002/diff/80001/chrome/browser/memory/tab_manager.cc#newcode369 chrome/browser/memory/tab_manager.cc:369: return false; Does this mean that ...
11 months ago (2017-03-24 11:55:39 UTC) #16
bashi
Regarding the question about tab discarding, I'll address in the next patch set. Maybe I ...
10 months, 3 weeks ago (2017-03-28 03:15:31 UTC) #17
bashi
PTAL? As chatted offline I changed the CL to call DiscardTab() every time OnCriticalCondition() is ...
10 months, 3 weeks ago (2017-03-28 05:48:36 UTC) #20
haraken
LGTM
10 months, 3 weeks ago (2017-03-28 06:56:11 UTC) #21
bashi
kinuko@: Could you review content/ ?
10 months, 3 weeks ago (2017-03-28 07:16:21 UTC) #23
bashi
isherman@: Could you review histograms.xml?
10 months, 3 weeks ago (2017-03-28 07:19:27 UTC) #25
kinuko
lgtm https://codereview.chromium.org/2763933002/diff/120001/content/renderer/render_thread_impl.cc File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/2763933002/diff/120001/content/renderer/render_thread_impl.cc#newcode2237 content/renderer/render_thread_impl.cc:2237: void RenderThreadImpl::RecordPurgeMemory(RendererMemoryMetrics before) { nit: please move this ...
10 months, 3 weeks ago (2017-03-28 08:07:00 UTC) #28
bashi
https://codereview.chromium.org/2763933002/diff/120001/content/renderer/render_thread_impl.cc File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/2763933002/diff/120001/content/renderer/render_thread_impl.cc#newcode2237 content/renderer/render_thread_impl.cc:2237: void RenderThreadImpl::RecordPurgeMemory(RendererMemoryMetrics before) { On 2017/03/28 08:07:00, kinuko wrote: ...
10 months, 3 weeks ago (2017-03-28 09:37:17 UTC) #31
Ilya Sherman
Metrics LGTM, thanks.
10 months, 3 weeks ago (2017-03-28 18:16:17 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2763933002/140001
10 months, 3 weeks ago (2017-03-29 23:20:59 UTC) #37
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder ...
10 months, 3 weeks ago (2017-03-30 01:23:21 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2763933002/140001
10 months, 3 weeks ago (2017-03-30 01:25:22 UTC) #41
commit-bot: I haz the power
10 months, 3 weeks ago (2017-03-30 01:56:22 UTC) #44
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/b9058f285cf483b7c49fe46e554f...

Powered by Google App Engine
This is Rietveld 408576698