Changing OOM range to 0, 1000 and tweaking OOM algorithm.

With this change, we now use the newer oom_score_adj file (with
fallback to oom_adj when on a system that doesn't support it) so that
we can take advantage of a finer range ([0, 1000] instead of [0, 15]).

Also tweaked the OOM priority manager to prioritize things in a
slightly different order, preferring (even more) not to kill tabs that
the user has currently selected.

BUG=chromium-os:18421, chromium:65009
TEST=Ran on device, observed OOM adj values, forced OOM conditions to
     watch kills.

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

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

6 files changed, 78 insertions, 41 deletions

diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc
index 1979602..a1d635e 100644
--- a/chrome/app/chrome_main.cc
+++ b/chrome/app/chrome_main.cc
@@ -174,22 +174,31 @@ bool HasDeprecatedArguments(const std::wstring& command_line) {
 
 #if defined(OS_LINUX)
 static void AdjustLinuxOOMScore(const std::string& process_type) {
-  const int kMiscScore = 7;
-#if defined(OS_CHROMEOS)
-  // On ChromeOS, we want plugins to die after the renderers.  If this
-  // works well for ChromeOS, we may do it for Linux as well.
-  const int kPluginScore = 4;
-#else
-  const int kPluginScore = 10;
-#endif
+  // Browsers and zygotes should still be killable, but killed last.
+  const int kZygoteScore = 0;
+  // The minimum amount to bump a score by.  This is large enough that
+  // even if it's translated into the old values, it will still go up
+  // by at least one.
+  const int kScoreBump = 100;
+  // This is the lowest score that renderers and extensions start with
+  // in the OomPriorityManager.
+  const int kRendererScore = chrome::kLowestRendererOomScore;
+  // For "miscellaneous" things, we want them after renderers,
+  // but before plugins.
+  const int kMiscScore = kRendererScore - kScoreBump;
+  // We want plugins to die after the renderers.
+  const int kPluginScore = kMiscScore - kScoreBump;
   int score = -1;
 
+  DCHECK(kMiscScore > 0);
+  DCHECK(kPluginScore > 0);
+
   if (process_type == switches::kPluginProcess ||
       process_type == switches::kPpapiPluginProcess) {
     score = kPluginScore;
   } else if (process_type == switches::kPpapiBrokerProcess) {
-    // Kill the broker before the plugin.
-    score = kPluginScore + 1;
+    // The broker should be killed before the PPAPI plugin.
+    score = kPluginScore + kScoreBump;
   } else if (process_type == switches::kUtilityProcess ||
              process_type == switches::kWorkerProcess ||
              process_type == switches::kGpuProcess ||
@@ -197,19 +206,25 @@ static void AdjustLinuxOOMScore(const std::string& process_type) {
     score = kMiscScore;
   } else if (process_type == switches::kProfileImportProcess) {
     NOTIMPLEMENTED();
+    score = kZygoteScore;
 #ifndef DISABLE_NACL
   } else if (process_type == switches::kNaClLoaderProcess) {
     score = kPluginScore;
 #endif
   } else if (process_type == switches::kZygoteProcess ||
              process_type.empty()) {
-    // Pass - browser / zygote process stays at 0.
+    // For zygotes and unlabeled process types, we want to still make
+    // them killable by the OOM killer.
+    score = kZygoteScore;
   } else if (process_type == switches::kExtensionProcess ||
              process_type == switches::kRendererProcess) {
     LOG(WARNING) << "process type '" << process_type << "' "
-                 << "should go through the zygote.";
-    // When debugging, these process types can end up being run directly.
-    return;
+                 << "should be created through the zygote.";
+    // When debugging, these process types can end up being run
+    // directly, but this isn't the typical path for assigning the OOM
+    // score for them.  Still, we want to assign a score that is
+    // somewhat representative for debugging.
+    score = kRendererScore;
   } else {
     NOTREACHED() << "Unknown process type";
   }
@@ -220,7 +235,7 @@ static void AdjustLinuxOOMScore(const std::string& process_type) {
 
 void SetupCRT(const CommandLine& command_line) {
 #if defined(OS_WIN)
-#ifdef _CRTDBG_MAP_ALLOC
+#if defined(_CRTDBG_MAP_ALLOC)
   _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
   _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE);
 #else
@@ -535,7 +550,7 @@ int RunNamedProcessTypeMain(const std::string& process_type,
 #endif
 #if !defined(DISABLE_NACL)
     { switches::kNaClLoaderProcess, NaClMain },
-#ifdef _WIN64  // The broker process is used only on Win64.
+#if defined(_WIN64)  // The broker process is used only on Win64.
     { switches::kNaClBrokerProcess, NaClBrokerMain },
 #endif
 #endif  // DISABLE_NACL
diff --git a/chrome/browser/oom_priority_manager.cc b/chrome/browser/oom_priority_manager.cc
index 44abf0f..fcd7a91 100644
--- a/chrome/browser/oom_priority_manager.cc
+++ b/chrome/browser/oom_priority_manager.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/common/chrome_constants.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/tab_contents/tab_contents.h"
@@ -29,7 +30,7 @@ using base::ProcessMetrics;
 
 namespace browser {
 
-// The default interval in seconds after which to adjust the oom_adj
+// The default interval in seconds after which to adjust the oom_score_adj
 // value.
 #define ADJUSTMENT_INTERVAL_SECONDS 10
 
@@ -66,7 +67,11 @@ bool OomPriorityManager::CompareRendererStats(RendererStats first,
   static const int64 kTimeBucketInterval =
       TimeDelta::FromMinutes(BUCKET_INTERVAL_MINUTES).ToInternalValue();
 
-  // Being pinned is most important.
+  // Being currently selected is most important.
+  if (first.is_selected != second.is_selected)
+    return first.is_selected == true;
+
+  // Being pinned is second most important.
   if (first.is_pinned != second.is_pinned)
     return first.is_pinned == true;
 
@@ -82,16 +87,17 @@ bool OomPriorityManager::CompareRendererStats(RendererStats first,
 }
 
 // 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.
+// existing renderers in priority order, and hand out oom_score_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
+// 3) is the tab currently selected
 //
 // We also need to collect:
-// 3) size in memory of a tab
+// 4) 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() {
@@ -109,7 +115,8 @@ void OomPriorityManager::AdjustOomPriorities() {
       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.
+      stats.memory_used = 0;  // This gets calculated in DoAdjustOomPriorities.
+      stats.is_selected = model->IsTabSelected(i);
       renderer_stats.push_back(stats);
     }
   }
@@ -145,30 +152,32 @@ void OomPriorityManager::DoAdjustOomPriorities(StatsList renderer_stats) {
   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;
+  // assigning priorities in the range of kLowestRendererOomScore to
+  // kHighestRendererOomScore (defined in chrome_constants.h).
+  // oom_score_adj takes values from -1000 to 1000.  Negative values
+  // are reserved for system processes, and we want to give some room
+  // below the range we're using to allow for things that want to be
+  // above the renderers in priority, so the defined range 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
+  // (least likely to be killed) of the duplicates, so that a
+  // particular renderer process takes on the oom_score_adj of the
+  // least likely tab to be killed.
+  const int kPriorityRange = chrome::kHighestRendererOomScore -
+                             chrome::kLowestRendererOomScore;
   float priority_increment =
       static_cast<float>(kPriorityRange) / renderer_stats.size();
-  float priority = kMinPriority;
+  float priority = chrome::kLowestRendererOomScore;
   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);
       ZygoteHost::GetInstance()->AdjustRendererOOMScore(
-          iterator->renderer_handle,
-          static_cast<int>(priority + 0.5f));
+          iterator->renderer_handle, static_cast<int>(priority + 0.5f));
       priority += priority_increment;
     }
   }
diff --git a/chrome/browser/oom_priority_manager.h b/chrome/browser/oom_priority_manager.h
index 55ce733..3090947d 100644
--- a/chrome/browser/oom_priority_manager.h
+++ b/chrome/browser/oom_priority_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -15,7 +15,7 @@ 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
+// /proc/<pid>/oom_score_adj) of the renderers so that they match the
 // algorithm embedded here for priority in being killed upon OOM
 // conditions.
 //
@@ -32,6 +32,7 @@ class OomPriorityManager {
  private:
   struct RendererStats {
     bool is_pinned;
+    bool is_selected;
     base::TimeTicks last_selected;
     size_t memory_used;
     base::ProcessHandle renderer_handle;
diff --git a/chrome/browser/ui/browser_list.h b/chrome/browser/ui/browser_list.h
index 3b3f061..b926083 100644
--- a/chrome/browser/ui/browser_list.h
+++ b/chrome/browser/ui/browser_list.h
@@ -244,7 +244,7 @@ class TabContentsIterator {
   }
 
   // Returns the Browser instance associated with the current TabContents.
-  // Valid as long as !Done()
+  // Valid as long as !done()
   Browser* browser() const {
     if (browser_iterator_ != BrowserList::end())
       return *browser_iterator_;
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 42bc248..844ef3b 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -177,6 +177,11 @@ const bool kEnableTouchIcon = true;
 const bool kEnableTouchIcon = false;
 #endif
 
+#if defined(OS_LINUX)
+extern const int kLowestRendererOomScore = 300;
+extern const int kHighestRendererOomScore = 1000;
+#endif
+
 }  // namespace chrome
 
 #undef FPL
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 4e89208..fb05878 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -99,6 +99,13 @@ extern const int kJavascriptMessageExpectedDelay;
 // Are touch icons enabled? False by default.
 extern const bool kEnableTouchIcon;
 
+#if defined(OS_LINUX)
+// The highest and lowest assigned OOM score adjustment
+// (oom_score_adj) used by the OomPriority Manager.
+extern const int kLowestRendererOomScore;
+extern const int kHighestRendererOomScore;
+#endif
+
 }  // namespace chrome
 
 #endif  // CHROME_COMMON_CHROME_CONSTANTS_H_