Trying again to land OOM priority manager changes.

First landing failed because of an obscure problem with building
linux_shared. This change passes the linux_shared trybot (and
linux and linux_chromeos trybots).

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.

Original review: http://codereview.chromium.org/7671033/

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/7708020

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

base/process_util.cc

@@ -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.
 

base/process_util.h

@@ -173,13 +173,18 @@ BASE_EXPORT FilePath GetProcessExecutablePath(ProcessHandle process);
 // Exposed for testing.
 BASE_EXPORT int ParseProcStatCPU(const std::string& input);
 
-static const char kAdjustOOMScoreSwitch[] = "--adjust-oom-score";
-
-// This adjusts /proc/process/oom_adj so the Linux OOM killer will prefer
-// certain process types over others. The range for the adjustment is
-// [-17,15], with [0,15] being user accessible.
+// The maximum allowed value for the OOM score.
+const int kMaxOomScore = 1000;
+
+// This adjusts /proc/<pid>/oom_score_adj so the Linux OOM killer will
+// prefer to kill certain process types over others. The range for the
+// adjustment is [-1000, 1000], with [0, 1000] being user accessible.
+// If the Linux system doesn't support the newer oom_score_adj range
+// of [0, 1000], then we revert to using the older oom_adj, and
+// translate the given value into [0, 15].  Some aliasing of values
+// may occur in that case, of course.
 BASE_EXPORT bool AdjustOOMScore(ProcessId process, int score);
-#endif
+#endif  // defined(OS_LINUX)
 
 #if defined(OS_POSIX)
 // Returns the ID for the parent of the given process.
@@ -189,7 +194,7 @@ BASE_EXPORT ProcessId GetParentProcessId(ProcessHandle process);
 // given multimap. Only call this function in a child process where you know
 // that there aren't any other threads.
 BASE_EXPORT void CloseSuperfluousFds(const InjectiveMultimap& saved_map);
-#endif
+#endif  // defined(OS_POSIX)
 
 // TODO(evan): rename these to use StudlyCaps.
 typedef std::vector<std::pair<std::string, std::string> > environment_vector;
@@ -205,7 +210,7 @@ struct LaunchOptions {
 #else
                     environ(NULL), fds_to_remap(NULL), new_process_group(false),
                     clone_flags(0)
-#endif
+#endif  // !defined(OS_WIN)
       {}
 
   // If true, wait for the process to complete.
@@ -248,7 +253,7 @@ struct LaunchOptions {
 
   // If non-zero, start the process using clone(), using flags as provided.
   int clone_flags;
-#endif
+#endif   // !defined(OS_WIN)
 };
 
 // Launch a process via the command line |cmdline|.
@@ -335,7 +340,7 @@ BASE_EXPORT bool GetAppOutputRestricted(const CommandLine& cl,
 // |*exit_code|.
 BASE_EXPORT bool GetAppOutputWithExitCode(const CommandLine& cl,
                                           std::string* output, int* exit_code);
-#endif
+#endif  // defined(OS_POSIX)
 
 // Used to filter processes by process ID.
 class ProcessFilter {
@@ -372,12 +377,12 @@ BASE_EXPORT bool KillProcess(ProcessHandle process, int exit_code, bool wait);
 // Attempts to kill the process group identified by |process_group_id|. Returns
 // true on success.
 BASE_EXPORT bool KillProcessGroup(ProcessHandle process_group_id);
-#endif
+#endif  // defined(OS_POSIX)
 
 #if defined(OS_WIN)
 BASE_EXPORT bool KillProcessById(ProcessId process_id, int exit_code,
                                  bool wait);
-#endif
+#endif  // defined(OS_WIN)
 
 // Get the termination status of the process by interpreting the
 // circumstances of the child process' death. |exit_code| is set to
@@ -632,7 +637,7 @@ class BASE_EXPORT ProcessMetrics {
   explicit ProcessMetrics(ProcessHandle process);
 #else
   ProcessMetrics(ProcessHandle process, PortProvider* port_provider);
-#endif  // !defined(OS_MACOSX)
+#endif  // defined(OS_MACOSX)
 
   ProcessHandle process_;
 
@@ -651,7 +656,7 @@ class BASE_EXPORT ProcessMetrics {
 #elif defined(OS_POSIX)
   // Jiffie count at the last_time_ we updated.
   int last_cpu_;
-#endif  // defined(OS_MACOSX)
+#endif  // defined(OS_POSIX)
 
   DISALLOW_COPY_AND_ASSIGN(ProcessMetrics);
 };
@@ -674,7 +679,7 @@ struct SystemMemoryInfoKB {
 // Fills in the provided |meminfo| structure. Returns true on success.
 // Exposed for memory debugging widget.
 BASE_EXPORT bool GetSystemMemoryInfo(SystemMemoryInfoKB* meminfo);
-#endif
+#endif  // defined(OS_LINUX)
 
 // Returns the memory committed by the system in KBytes.
 // Returns 0 if it can't compute the commit charge.
@@ -699,8 +704,8 @@ BASE_EXPORT void EnableTerminationOnOutOfMemory();
 #if defined(OS_MACOSX)
 // Exposed for testing.
 BASE_EXPORT malloc_zone_t* GetPurgeableZone();
-#endif
-#endif
+#endif  // defined(OS_MACOSX)
+#endif  // !defined(OS_WIN)
 
 // Enables stack dump to console output on exception and signals.
 // When enabled, the process will quit immediately. This is meant to be used in

base/process_util_linux.cc

@@ -27,6 +27,10 @@
 
 namespace {
 
+// Max score for the old oom_adj range.  Used for conversion of new
+// values to old values.
+const int kMaxOldOomScore = 15;
+
 enum ParsingState {
   KEY_NAME,
   KEY_VALUE
@@ -734,20 +738,41 @@ void EnableTerminationOnOutOfMemory() {
 #endif
 }
 
+// NOTE: This is not the only version of this function in the source:
+// the setuid sandbox (in process_util_linux.c, in the sandbox source)
+// also has it's own C version.
 bool AdjustOOMScore(ProcessId process, int score) {
-  if (score < 0 || score > 15)
+  if (score < 0 || score > kMaxOomScore)
     return false;
 
-  FilePath oom_adj("/proc");
-  oom_adj = oom_adj.Append(base::Int64ToString(process));
-  oom_adj = oom_adj.AppendASCII("oom_adj");
+  FilePath oom_path("/proc");
+  oom_path = oom_path.Append(base::Int64ToString(process));
+
+  // Attempt to write the newer oom_score_adj file first.
+  FilePath oom_file = oom_path.AppendASCII("oom_score_adj");
+  if (file_util::PathExists(oom_file)) {
+    std::string score_str = base::IntToString(score);
+    VLOG(1) << "Adjusting oom_score_adj of " << process << " to " << score_str;
+    int score_len = static_cast<int>(score_str.length());
+    return (score_len == file_util::WriteFile(oom_file,
+                                              score_str.c_str(),
+                                              score_len));
+  }
 
-  if (!file_util::PathExists(oom_adj))
-    return false;
+  // If the oom_score_adj file doesn't exist, then we write the old
+  // style file and translate the oom_adj score to the range 0-15.
+  oom_file = oom_path.AppendASCII("oom_adj");
+  if (file_util::PathExists(oom_file)) {
+    std::string score_str = base::IntToString(
+        score * kMaxOldOomScore / kMaxOomScore);
+    VLOG(1) << "Adjusting oom_adj of " << process << " to " << score_str;
+    int score_len = static_cast<int>(score_str.length());
+    return (score_len == file_util::WriteFile(oom_file,
+                                              score_str.c_str(),
+                                              score_len));
+  }
 
-  std::string score_str = base::IntToString(score);
-  return (static_cast<int>(score_str.length()) ==
-          file_util::WriteFile(oom_adj, score_str.c_str(), score_str.length()));
+  return false;
 }
 
 }  // namespace base

chrome/app/chrome_main.cc

@@ -174,42 +174,57 @@ 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 ||
              process_type == switches::kServiceProcess) {
     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

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;
     }
   }