Commit 14344f5d authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

[discarding] Only log memory details on memory pressure on ChromeOS.

We should minimize the amount of work done when the system is under
memory pressure, to allow it to exit that state.

Since the "Tab Discards Memory details" log is only used on ChromeOS,
this CL removes it on other platforms. Collecting memory details is
the culprit for 3% of IO thread hangs, so we shouldn't do it for no
reason.

Bug: 1040522
Change-Id: Ie4fff140a7024d9372a3888a515466885cce68da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980282Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730202}
parent 0d1c005f
......@@ -268,12 +268,20 @@ WebContents* TabManager::DiscardTabByExtension(content::WebContents* contents) {
return DiscardTabImpl(LifecycleUnitDiscardReason::EXTERNAL);
}
void TabManager::LogMemoryAndDiscardTab(LifecycleUnitDiscardReason reason) {
void TabManager::DiscardTabFromMemoryPressure() {
DCHECK(!base::FeatureList::IsEnabled(
performance_manager::features::kUrgentDiscardingFromPerformanceManager));
// Discard immediately without waiting for LogMemory() (https://crbug/850545).
// Consider removing LogMemory() at all if nobody cares about the log.
LogMemory("Tab Discards Memory details");
#if defined(OS_CHROMEOS)
// Output a log with per-process memory usage and number of file descriptors,
// as well as GPU memory details. Discard happens without waiting for the log
// (https://crbug.com/850545) Per comment at
// https://crrev.com/c/chromium/src/+/1980282/3#message-d45cc354e7776d7e3d208e22dd2f6bbca3e9eae8,
// this log is used to diagnose issues on ChromeOS. Do not output it on other
// platforms since it is not used and data shows it can create IO thread hangs
// (https://crbug.com/1040522).
memory::OomMemoryDetails::Log("Tab Discards Memory details");
#endif // defined(OS_CHROMEOS)
// Start handling memory pressure. Suppress further notifications before
// completion in case a slow handler queues up multiple dispatches of this
......@@ -282,12 +290,7 @@ void TabManager::LogMemoryAndDiscardTab(LifecycleUnitDiscardReason reason) {
TabDiscardDoneCB tab_discard_done(base::BindOnce(
&TabManager::OnTabDiscardDone, weak_ptr_factory_.GetWeakPtr()));
DiscardTab(reason, std::move(tab_discard_done));
}
void TabManager::LogMemory(const std::string& title) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
memory::OomMemoryDetails::Log(title);
DiscardTab(LifecycleUnitDiscardReason::URGENT, std::move(tab_discard_done));
}
void TabManager::AddObserver(TabLifecycleObserver* observer) {
......@@ -385,7 +388,7 @@ void TabManager::OnMemoryPressure(
case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
return;
case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
LogMemoryAndDiscardTab(LifecycleUnitDiscardReason::URGENT);
DiscardTabFromMemoryPressure();
return;
}
NOTREACHED();
......
......@@ -8,7 +8,6 @@
#include <stdint.h>
#include <memory>
#include <string>
#include <vector>
#include "base/compiler_specific.h"
......@@ -104,13 +103,8 @@ class TabManager : public LifecycleUnitObserver,
// was discarded.
content::WebContents* DiscardTabByExtension(content::WebContents* contents);
// Log memory statistics for the running processes, then discards a tab.
// Tab discard happens sometime later, as collecting the statistics touches
// multiple threads and takes time.
void LogMemoryAndDiscardTab(LifecycleUnitDiscardReason reason);
// Log memory statistics for the running processes.
void LogMemory(const std::string& title);
// Discards a tab in response to memory pressure.
void DiscardTabFromMemoryPressure();
// TODO(fdoray): Remove these methods. TabManager shouldn't know about tabs.
// https://crbug.com/775644
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment