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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by bashi
Modified:
4 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
Trybot results:  chromium_presubmit   win_clang   win_chromium_x64_rel_ng   win_clang   win_chromium_compile_dbg_ng   mac_chromium_rel_ng   win_chromium_rel_ng   ios-simulator   mac_chromium_compile_dbg_ng   ios-device-xcode-clang   ios-simulator-xcode-clang   ios-device   linux_chromium_rel_ng   linux_chromium_tsan_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_asan_rel_ng   chromeos_daisy_chromium_compile_only_ng   chromium_presubmit   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_android   linux_android_rel_ng   cast_shell_linux   android_cronet   android_n5x_swarming_rel   android_compile_dbg   android_arm64_dbg_recipe   android_clang_dbg_recipe   linux_chromium_rel_ng   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   ios-device   chromeos_amd64-generic_chromium_compile_only_ng   android_clang_dbg_recipe   android_compile_dbg   linux_chromium_rel_ng   linux_chromium_tsan_rel_ng   chromeos_daisy_chromium_compile_only_ng   android_n5x_swarming_rel   ios-device-xcode-clang   android_cronet   linux_chromium_asan_rel_ng   cast_shell_android   linux_chromium_chromeos_rel_ng   mac_chromium_rel_ng   mac_chromium_compile_dbg_ng   cast_shell_linux   ios-simulator-xcode-clang   linux_android_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_chromeos_ozone_rel_ng   chromium_presubmit   ios-simulator   android_arm64_dbg_recipe 
Commit queue not available (can’t edit this change).

Messages

Total messages: 44 (28 generated)
bashi
I've been doing local experiments on my ChromeOS and Android devices to figure out a ...
5 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 ...
5 months ago (2017-03-23 05:19:16 UTC) #5
bashi
Added tests. Could you take another look?
4 months, 4 weeks 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 ...
4 months, 4 weeks 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 ...
4 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 ...
4 months, 3 weeks ago (2017-03-28 05:48:36 UTC) #20
haraken
LGTM
4 months, 3 weeks ago (2017-03-28 06:56:11 UTC) #21
bashi
kinuko@: Could you review content/ ?
4 months, 3 weeks ago (2017-03-28 07:16:21 UTC) #23
bashi
isherman@: Could you review histograms.xml?
4 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 ...
4 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: ...
4 months, 3 weeks ago (2017-03-28 09:37:17 UTC) #31
Ilya Sherman
Metrics LGTM, thanks.
4 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
4 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 ...
4 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
4 months, 3 weeks ago (2017-03-30 01:25:22 UTC) #41
commit-bot: I haz the power
4 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b