Commit 9f520647 authored by Cheng-Yu Lee's avatar Cheng-Yu Lee Committed by Commit Bot

Discard tab immediately without waiting for LogMemory().

LogMemory() calls memory::OomMemoryDetails::Log(), which is an async
call and may take a long time (only 60% returns < 1s, 10% returns > 5s).
When we're running out of memory, we need to discard tab faster.

BUG=850545

Change-Id: Ia652b2a80f968b0e437904bb6bc2c7c131c1958d
Reviewed-on: https://chromium-review.googlesource.com/1097038Reviewed-by: default avatarCheng-Yu Lee <cylee@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Commit-Queue: Cheng-Yu Lee <cylee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567891}
parent 7f8b47da
...@@ -14,16 +14,14 @@ ...@@ -14,16 +14,14 @@
namespace memory { namespace memory {
// static // static
void OomMemoryDetails::Log(const std::string& title, void OomMemoryDetails::Log(const std::string& title) {
const base::Closure& callback) {
// Deletes itself upon completion. // Deletes itself upon completion.
OomMemoryDetails* details = new OomMemoryDetails(title, callback); OomMemoryDetails* details = new OomMemoryDetails(title);
details->StartFetch(); details->StartFetch();
} }
OomMemoryDetails::OomMemoryDetails(const std::string& title, OomMemoryDetails::OomMemoryDetails(const std::string& title)
const base::Closure& callback) : title_(title) {
: title_(title), callback_(callback) {
AddRef(); // Released in OnDetailsAvailable(). AddRef(); // Released in OnDetailsAvailable().
start_time_ = base::TimeTicks::Now(); start_time_ = base::TimeTicks::Now();
} }
...@@ -45,8 +43,6 @@ void OomMemoryDetails::OnDetailsAvailable() { ...@@ -45,8 +43,6 @@ void OomMemoryDetails::OnDetailsAvailable() {
#endif #endif
LOG(WARNING) << title_ << " (" << delta.InMilliseconds() << " ms):\n" LOG(WARNING) << title_ << " (" << delta.InMilliseconds() << " ms):\n"
<< log_string; << log_string;
if (!callback_.is_null())
callback_.Run();
// Delete ourselves so we don't have to worry about OomPriorityManager // Delete ourselves so we don't have to worry about OomPriorityManager
// deleting us when we're still working. // deleting us when we're still working.
Release(); Release();
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <string> #include <string>
#include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/memory_details.h" #include "chrome/browser/memory_details.h"
...@@ -19,12 +18,12 @@ namespace memory { ...@@ -19,12 +18,12 @@ namespace memory {
// memory event in an attempt to identify the culprit. // memory event in an attempt to identify the culprit.
class OomMemoryDetails : public MemoryDetails { class OomMemoryDetails : public MemoryDetails {
public: public:
// Logs the memory details asynchronously and then run |callback|. // Logs the memory details asynchronously.
// The |title| is printed at the beginning of the message. // The |title| is printed at the beginning of the message.
static void Log(const std::string& title, const base::Closure& callback); static void Log(const std::string& title);
private: private:
OomMemoryDetails(const std::string& title, const base::Closure& callback); OomMemoryDetails(const std::string& title);
~OomMemoryDetails() override; ~OomMemoryDetails() override;
// MemoryDetails overrides: // MemoryDetails overrides:
...@@ -32,7 +31,6 @@ class OomMemoryDetails : public MemoryDetails { ...@@ -32,7 +31,6 @@ class OomMemoryDetails : public MemoryDetails {
std::string title_; std::string title_;
base::TimeTicks start_time_; base::TimeTicks start_time_;
base::Closure callback_;
DISALLOW_COPY_AND_ASSIGN(OomMemoryDetails); DISALLOW_COPY_AND_ASSIGN(OomMemoryDetails);
}; };
......
...@@ -304,14 +304,15 @@ WebContents* TabManager::DiscardTabByExtension(content::WebContents* contents) { ...@@ -304,14 +304,15 @@ WebContents* TabManager::DiscardTabByExtension(content::WebContents* contents) {
} }
void TabManager::LogMemoryAndDiscardTab(DiscardReason reason) { void TabManager::LogMemoryAndDiscardTab(DiscardReason reason) {
LogMemory("Tab Discards Memory details", // Discard immediately without waiting for LogMemory() (https://crbug/850545).
base::Bind(&TabManager::PurgeMemoryAndDiscardTab, reason)); // Consider removing LogMemory() at all if nobody cares about the log.
LogMemory("Tab Discards Memory details");
PurgeMemoryAndDiscardTab(reason);
} }
void TabManager::LogMemory(const std::string& title, void TabManager::LogMemory(const std::string& title) {
const base::Closure& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
memory::OomMemoryDetails::Log(title, callback); memory::OomMemoryDetails::Log(title);
} }
void TabManager::AddObserver(TabLifecycleObserver* observer) { void TabManager::AddObserver(TabLifecycleObserver* observer) {
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/callback.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -119,8 +118,8 @@ class TabManager : public LifecycleUnitObserver, ...@@ -119,8 +118,8 @@ class TabManager : public LifecycleUnitObserver,
// multiple threads and takes time. // multiple threads and takes time.
void LogMemoryAndDiscardTab(DiscardReason reason); void LogMemoryAndDiscardTab(DiscardReason reason);
// Log memory statistics for the running processes, then call the callback. // Log memory statistics for the running processes.
void LogMemory(const std::string& title, const base::Closure& callback); void LogMemory(const std::string& title);
// TODO(fdoray): Remove these methods. TabManager shouldn't know about tabs. // TODO(fdoray): Remove these methods. TabManager shouldn't know about tabs.
// https://crbug.com/775644 // https://crbug.com/775644
......
...@@ -267,7 +267,7 @@ SadTab::SadTab(content::WebContents* web_contents, SadTabKind kind) ...@@ -267,7 +267,7 @@ SadTab::SadTab(content::WebContents* web_contents, SadTabKind kind)
{ {
const std::string spec = web_contents->GetURL().GetOrigin().spec(); const std::string spec = web_contents->GetURL().GetOrigin().spec();
memory::OomMemoryDetails::Log( memory::OomMemoryDetails::Log(
"Tab OOM-Killed Memory details: " + spec + ", ", base::Closure()); "Tab OOM-Killed Memory details: " + spec + ", ");
} }
FALLTHROUGH; FALLTHROUGH;
#endif #endif
......
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