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

chrome/browser/oom_priority_manager.cc

 // 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.
 
 #include "chrome/browser/oom_priority_manager.h"
 
 #include <list>
 
 #include "base/process.h"
 #include "base/process_util.h"
@@ skipping 11 matching lines @@
 #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
+// The default interval in seconds after which to adjust the oom_score_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();
 }
@@ skipping 16 matching lines @@
 
 // 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.
+  // 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;
 
   // 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.
+// 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() {
   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.
+      stats.memory_used = 0;  // This gets calculated in DoAdjustOomPriorities.
+      stats.is_selected = model->IsTabSelected(i);
       renderer_stats.push_back(stats);
     }
   }
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(this, &OomPriorityManager::DoAdjustOomPriorities,
                         renderer_stats));
 }
 
@@ skipping 15 matching lines @@
       // 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;
+  // 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();
   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);
       ZygoteHost::GetInstance()->AdjustRendererOOMScore(
           iterator->renderer_handle,
           static_cast<int>(priority + 0.5f));
       priority += priority_increment;
     }
   }
 }
 
 }  // namespace browser