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

chrome/browser/oom_priority_manager.cc

Index: chrome/browser/oom_priority_manager.cc
diff --git a/chrome/browser/oom_priority_manager.cc b/chrome/browser/oom_priority_manager.cc
index 44abf0fedb03db61d1de408ce7265f035b5d4b21..8032a6d58c05f90a2b8852c6ec5ed0ff05f1b6c2 100644
--- a/chrome/browser/oom_priority_manager.cc
+++ b/chrome/browser/oom_priority_manager.cc
@@ -29,7 +29,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 +66,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 +86,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 +114,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,18 +151,21 @@ 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;
+  // assigning priorities in the range of 300 to 1000.  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 300 to 1000 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 kMinPriority = 300;
+  const int kMaxPriority = 1000;

[stevenjb, 2011/08/18 00:20:07]

It would be good to avoid having these constants defined in two places. Can we
have one class that sets these values and provides accessors?

[Greg Spencer (Chromium), 2011/08/18 23:10:50]

OK, I added two constants to chrome/common/chrome_constants.h so that it can be
accessed from both places without having to include across their boundaries.

   const int kPriorityRange = kMaxPriority - kMinPriority;
   float priority_increment =
       static_cast<float>(kPriorityRange) / renderer_stats.size();