This is another try to commit OOM priority management.

This changes nothing, except that I now initialize memory_used in oom_priority_manager.cc to placate the incorrect gcc warning about it being used when uninitialized.

The original review is here: http://codereview.chromium.org/4498001/

BUG=none
TEST=Passed chromeos try-bot, built both debug and release locally.

Review URL: http://codereview.chromium.org/5328003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67342 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 282 insertions, 13 deletions

diff --git a/base/task.h b/base/task.h
index 28d15fc..b21ccd8 100644
--- a/base/task.h
+++ b/base/task.h
@@ -274,7 +274,7 @@ struct RunnableMethodTraits {
 //   };
 //   }  // namespace foo
 //
-//   DISABLE_RUNNABLE_METHOD_REFCOUNT(foo::Bar)
+//   DISABLE_RUNNABLE_METHOD_REFCOUNT(foo::Bar);
 //
 // This is different from DISALLOW_COPY_AND_ASSIGN which is declared inside the
 // class.
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index da04cdf..1e57561 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -115,6 +115,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/boot_times_loader.h"
+#include "chrome/browser/oom_priority_manager.h"
 #endif
 
 // TODO(port): several win-only methods have been pulled out of this, but
@@ -1562,9 +1563,9 @@ int BrowserMain(const MainFunctionParams& parameters) {
   bool record_search_engine = is_first_run && !profile->IsOffTheRecord();
 #endif
 
-    // ChildProcess:: is a misnomer unless you consider context.  Use
-    // of --wait-for-debugger only makes sense when Chrome itself is a
-    // child process (e.g. when launched by PyAuto).
+  // ChildProcess:: is a misnomer unless you consider context.  Use
+  // of --wait-for-debugger only makes sense when Chrome itself is a
+  // child process (e.g. when launched by PyAuto).
   if (parsed_command_line.HasSwitch(switches::kWaitForDebugger)) {
     ChildProcess::WaitForDebugger(L"Browser");
   }
@@ -1582,6 +1583,17 @@ int BrowserMain(const MainFunctionParams& parameters) {
     }
   }
 
+#if defined(OS_CHROMEOS)
+  // Run the Out of Memory priority manager while in this scope.  Wait
+  // until here to start so that we give the most amount of time for
+  // the other services to start up before we start adjusting the oom
+  // priority.  In reality, it doesn't matter much where in this scope
+  // this is started, but it must be started in this scope so it will
+  // also be terminated when this scope exits.
+  scoped_ptr<browser::OomPriorityManager> oom_priority_manager(
+      new browser::OomPriorityManager);
+#endif
+
   // Create the instance of the cloud print proxy service so that it can launch
   // the service process if needed. This is needed because the service process
   // might have shutdown because an update was available.
diff --git a/chrome/browser/oom_priority_manager.cc b/chrome/browser/oom_priority_manager.cc
new file mode 100644
index 0000000..446f108
--- /dev/null
+++ b/chrome/browser/oom_priority_manager.cc
@@ -0,0 +1,177 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/oom_priority_manager.h"
+
+#include <list>
+
+#include "base/process.h"
+#include "base/process_util.h"
+#include "base/thread.h"
+#include "build/build_config.h"
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/tab_contents_wrapper.h"
+#include "chrome/browser/tabs/tab_strip_model.h"
+#include "chrome/browser/zygote_host_linux.h"
+
+#if !defined(OS_CHROMEOS)
+#error This file only meant to be compiled on ChromeOS
+#endif
+
+using base::TimeDelta;
+using base::TimeTicks;
+using base::ProcessHandle;
+using base::ProcessMetrics;
+
+namespace browser {
+
+// The default interval in seconds after which to adjust the oom_adj
+// value.
+#define ADJUSTMENT_INTERVAL_SECONDS 10
+
+// The default interval in minutes for considering activation times
+// "equal".
+#define BUCKET_INTERVAL_MINUTES 10
+
+OomPriorityManager::OomPriorityManager() {
+  StartTimer();
+}
+
+OomPriorityManager::~OomPriorityManager() {
+  StopTimer();
+}
+
+void OomPriorityManager::StartTimer() {
+  if (!timer_.IsRunning()) {
+    timer_.Start(TimeDelta::FromSeconds(ADJUSTMENT_INTERVAL_SECONDS),
+                 this,
+                 &OomPriorityManager::AdjustOomPriorities);
+  }
+}
+
+void OomPriorityManager::StopTimer() {
+  timer_.Stop();
+}
+
+// Returns true if |first| is considered less desirable to be killed
+// than |second|.
+bool OomPriorityManager::CompareRendererStats(RendererStats first,
+                                              RendererStats second) {
+  // The size of the slop in comparing activation times.  [This is
+  // allocated here to avoid static initialization at startup time.]
+  static const int64 kTimeBucketInterval =
+      TimeDelta::FromMinutes(BUCKET_INTERVAL_MINUTES).ToInternalValue();
+
+  // Being pinned is most important.
+  if (first.is_pinned != second.is_pinned)
+    return first.is_pinned == true;
+
+  // We want to be a little "fuzzy" when we compare these, because
+  // it's not really possible for the times to be identical, but if
+  // the user selected two tabs at about the same time, we still want
+  // to take the one that uses more memory.
+  if (abs((first.last_selected - second.last_selected).ToInternalValue()) <
+      kTimeBucketInterval)
+    return first.last_selected < second.last_selected;
+
+  return first.memory_used < second.memory_used;
+}
+
+// Here we collect most of the information we need to sort the
+// existing renderers in priority order, and hand out oom_adj scores
+// based on that sort order.
+//
+// Things we need to collect on the browser thread (because
+// TabStripModel isn't thread safe):
+// 1) whether or not a tab is pinned
+// 2) last time a tab was selected
+//
+// We also need to collect:
+// 3) size in memory of a tab
+// But we do that in DoAdjustOomPriorities on the FILE thread so that
+// we avoid jank, because it accesses /proc.
+void OomPriorityManager::AdjustOomPriorities() {
+  if (BrowserList::size() == 0)
+    return;
+
+  StatsList renderer_stats;
+  for (BrowserList::const_iterator browser_iterator = BrowserList::begin();
+       browser_iterator != BrowserList::end(); ++browser_iterator) {
+    Browser* browser = *browser_iterator;
+    const TabStripModel* model = browser->tabstrip_model();
+    for (int i = 0; i < model->count(); i++) {
+      TabContents* contents = model->GetTabContentsAt(i)->tab_contents();
+      RendererStats stats;
+      stats.last_selected = contents->last_selected_time();
+      stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
+      stats.is_pinned = model->IsTabPinned(i);
+      stats.memory_used = 0; // This gets calculated in DoAdjustOomPriorities.
+      renderer_stats.push_back(stats);
+    }
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &OomPriorityManager::DoAdjustOomPriorities,
+                        renderer_stats));
+}
+
+void OomPriorityManager::DoAdjustOomPriorities(StatsList renderer_stats) {
+  for (StatsList::iterator stats_iter = renderer_stats.begin();
+       stats_iter != renderer_stats.end(); ++stats_iter) {
+    scoped_ptr<ProcessMetrics> metrics(ProcessMetrics::CreateProcessMetrics(
+        stats_iter->renderer_handle));
+
+    base::WorkingSetKBytes working_set_kbytes;
+    if (metrics->GetWorkingSetKBytes(&working_set_kbytes)) {
+      // We use the proportional set size (PSS) to calculate memory
+      // usage "badness" on Linux.
+      stats_iter->memory_used = working_set_kbytes.shared * 1024;
+    } else {
+      // and if for some reason we can't get PSS, we revert to using
+      // resident set size (RSS).  This will be zero if the process
+      // has already gone away, but we can live with that, since the
+      // process is gone anyhow.
+      stats_iter->memory_used = metrics->GetWorkingSetSize();
+    }
+  }
+
+  // Now we sort the data we collected so that least desirable to be
+  // killed is first, most desirable is last.
+  renderer_stats.sort(OomPriorityManager::CompareRendererStats);
+
+  // Now we assign priorities based on the sorted list.  We're
+  // assigning priorities in the range of 5 to 10.  oom_adj takes
+  // values from -17 to 15.  Negative values are reserved for system
+  // processes, and we want to give some room on either side of the
+  // range we're using to allow for things that want to be above or
+  // below the renderers in priority, so 5 to 10 gives us some
+  // variation in priority without taking up the whole range.  In the
+  // end, however, it's a pretty arbitrary range to use.  Higher
+  // values are more likely to be killed by the OOM killer.  We also
+  // remove any duplicate PIDs, leaving the most important of the
+  // duplicates.
+  const int kMinPriority = 5;
+  const int kMaxPriority = 10;
+  const int kPriorityRange = kMaxPriority - kMinPriority;
+  float priority_increment =
+      static_cast<float>(kPriorityRange) / renderer_stats.size();
+  float priority = kMinPriority;
+  std::set<base::ProcessHandle> already_seen;
+  for (StatsList::iterator iterator = renderer_stats.begin();
+       iterator != renderer_stats.end(); ++iterator) {
+    if (already_seen.find(iterator->renderer_handle) == already_seen.end()) {
+      already_seen.insert(iterator->renderer_handle);
+      Singleton<ZygoteHost>::get()->AdjustRendererOOMScore(
+          iterator->renderer_handle,
+          static_cast<int>(priority + 0.5f));
+      priority += priority_increment;
+    }
+  }
+}
+
+}  // namespace browser
diff --git a/chrome/browser/oom_priority_manager.h b/chrome/browser/oom_priority_manager.h
new file mode 100644
index 0000000..55ce733
--- /dev/null
+++ b/chrome/browser/oom_priority_manager.h
@@ -0,0 +1,61 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
+#define CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
+
+#include <list>
+
+#include "base/timer.h"
+#include "base/process.h"
+
+namespace browser {
+
+// The OomPriorityManager periodically checks (see
+// ADJUSTMENT_INTERVAL_SECONDS in the source) the status of renderers
+// and adjusts the out of memory (OOM) adjustment value (in
+// /proc/<pid>/oom_adj) of the renderers so that they match the
+// algorithm embedded here for priority in being killed upon OOM
+// conditions.
+//
+// The algorithm used favors killing tabs that are not pinned, have
+// been idle for longest, and take up the most memory, in that order
+// of priority.  We round the idle times to the nearest few minutes
+// (see BUCKET_INTERVAL_MINUTES in the source) so that we can bucket
+// them, as no two tabs will have exactly the same idle time.
+class OomPriorityManager {
+ public:
+  OomPriorityManager();
+  ~OomPriorityManager();
+
+ private:
+  struct RendererStats {
+    bool is_pinned;
+    base::TimeTicks last_selected;
+    size_t memory_used;
+    base::ProcessHandle renderer_handle;
+  };
+  typedef std::list<RendererStats> StatsList;
+
+  void StartTimer();
+  void StopTimer();
+
+  // Posts DoAdjustOomPriorities task to the file thread.  Called when
+  // the timer fires.
+  void AdjustOomPriorities();
+
+  // Called by AdjustOomPriorities.  Runs on the file thread.
+  void DoAdjustOomPriorities(StatsList list);
+
+  static bool CompareRendererStats(RendererStats first, RendererStats second);
+
+  base::RepeatingTimer<OomPriorityManager> timer_;
+
+  DISALLOW_COPY_AND_ASSIGN(OomPriorityManager);
+};
+}  // namespace browser
+
+DISABLE_RUNNABLE_METHOD_REFCOUNT(browser::OomPriorityManager);
+
+#endif  // CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 9029bd0..3631575 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -797,6 +797,7 @@ void TabContents::DidBecomeSelected() {
   }
 
   WebCacheManager::GetInstance()->ObserveActivity(GetRenderProcessHost()->id());
+  last_selected_time_ = base::TimeTicks::Now();
 }
 
 void TabContents::WasHidden() {
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index c3d6556..4d54db5 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -328,6 +328,9 @@ class TabContents : public PageNavigator,
   // Invoked when the tab contents becomes selected. If you override, be sure
   // and invoke super's implementation.
   virtual void DidBecomeSelected();
+  base::TimeTicks last_selected_time() const {
+    return last_selected_time_;
+  }
 
   // Invoked when the tab contents becomes hidden.
   // NOTE: If you override this, call the superclass version too!
@@ -1282,6 +1285,9 @@ class TabContents : public PageNavigator,
   // The time that we started to close the tab.
   base::TimeTicks tab_close_start_time_;
 
+  // The time that this tab was last selected.
+  base::TimeTicks last_selected_time_;
+
   // Information about the language the page is in and has been translated to.
   LanguageState language_state_;
 
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 51582ad..1a9ac8f 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -477,6 +477,7 @@ void TabStripModel::SetTabPinned(int index, bool pinned) {
 }
 
 bool TabStripModel::IsTabPinned(int index) const {
+  DCHECK(ContainsIndex(index));
   return contents_data_[index]->pinned;
 }
 
@@ -961,7 +962,8 @@ void TabStripModel::ChangeSelectedContentsFrom(
   ObserverListBase<TabStripModelObserver>::Iterator it(observers_);
   TabStripModelObserver* obs;
   while ((obs = it.GetNext()) != NULL)
-    obs->TabSelectedAt(last_selected_contents, new_contents, selected_index_, user_gesture);
+    obs->TabSelectedAt(last_selected_contents, new_contents,
+                       selected_index_, user_gesture);
   /*
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
       TabSelectedAt(last_selected_contents, new_contents, selected_index_,
diff --git a/chrome/browser/zygote_host_linux.cc b/chrome/browser/zygote_host_linux.cc
index 5536eaf..b6115f1 100644
--- a/chrome/browser/zygote_host_linux.cc
+++ b/chrome/browser/zygote_host_linux.cc
@@ -242,6 +242,13 @@ pid_t ZygoteHost::ForkRenderer(
       return base::kNullProcessHandle;
   }
 
+  const int kRendererScore = 5;
+  AdjustRendererOOMScore(pid, kRendererScore);
+
+  return pid;
+}
+
+void ZygoteHost::AdjustRendererOOMScore(base::ProcessHandle pid, int score) {
   // 1) You can't change the oom_adj of a non-dumpable process (EPERM) unless
   //    you're root. Because of this, we can't set the oom_adj from the browser
   //    process.
@@ -274,27 +281,23 @@ pid_t ZygoteHost::ForkRenderer(
     selinux_valid = true;
   }
 
-  const int kRendererScore = 5;
   if (using_suid_sandbox_ && !selinux) {
     base::ProcessHandle sandbox_helper_process;
-    base::file_handle_mapping_vector dummy_map;
     std::vector<std::string> adj_oom_score_cmdline;
 
     adj_oom_score_cmdline.push_back(sandbox_binary_);
     adj_oom_score_cmdline.push_back(base::kAdjustOOMScoreSwitch);
     adj_oom_score_cmdline.push_back(base::Int64ToString(pid));
-    adj_oom_score_cmdline.push_back(base::IntToString(kRendererScore));
+    adj_oom_score_cmdline.push_back(base::IntToString(score));
     CommandLine adj_oom_score_cmd(adj_oom_score_cmdline);
-    if (base::LaunchApp(adj_oom_score_cmdline, dummy_map, false,
+    if (base::LaunchApp(adj_oom_score_cmd, false, true,
                         &sandbox_helper_process)) {
       ProcessWatcher::EnsureProcessGetsReaped(sandbox_helper_process);
     }
   } else if (!using_suid_sandbox_) {
-    if (!base::AdjustOOMScore(pid, kRendererScore))
-      LOG(ERROR) << "Failed to adjust OOM score of renderer";
+    if (!base::AdjustOOMScore(pid, score))
+      PLOG(ERROR) << "Failed to adjust OOM score of renderer with pid " << pid;
   }
-
-  return pid;
 }
 
 void ZygoteHost::EnsureProcessTerminated(pid_t process) {
diff --git a/chrome/browser/zygote_host_linux.h b/chrome/browser/zygote_host_linux.h
index dd00336..ffb7964 100644
--- a/chrome/browser/zygote_host_linux.h
+++ b/chrome/browser/zygote_host_linux.h
@@ -67,6 +67,9 @@ class ZygoteHost {
     return 0;
   }
 
+  // Adjust the OOM score of the given renderer's PID.
+  void AdjustRendererOOMScore(base::ProcessHandle process_handle, int score);
+
  private:
   friend struct DefaultSingletonTraits<ZygoteHost>;
   ZygoteHost();
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index a812d15..b66417a 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2340,6 +2340,8 @@
         'browser/ntp_background_util.h',
         'browser/omnibox_search_hint.cc',
         'browser/omnibox_search_hint.h',
+        'browser/oom_priority_manager.cc',
+        'browser/oom_priority_manager.h',
         'browser/options_page_base.cc',
         'browser/options_page_base.h',
         'browser/options_util.cc',
@@ -3438,6 +3440,8 @@
             ['exclude', 'browser/dom_ui/mediaplayer_ui.cc'],
             ['exclude', 'browser/dom_ui/slideshow_ui.cc'],
             ['exclude', 'browser/extensions/extension_tts_api_chromeos.cc'],
+            ['exclude', 'browser/oom_priority_manager.cc'],
+            ['exclude', 'browser/oom_priority_manager.h'],
             ['exclude', 'browser/renderer_host/offline_resource_handler.cc'],
             ['exclude', 'browser/renderer_host/offline_resource_handler.h'],
           ],