Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Unified Diff: chrome/browser/memory/tab_manager_delegate_chromeos.h

Issue 2095413002: TabManagerDelegate: Better prioritize ARC processes. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/memory/tab_manager_delegate_chromeos.h
diff --git a/chrome/browser/memory/tab_manager_delegate_chromeos.h b/chrome/browser/memory/tab_manager_delegate_chromeos.h
index 3722980f7c7d0ee85a1c2ac20cf506401fdd008f..8dc68bacb0010002f2dcbdbb7befd1aef10a708b 100644
--- a/chrome/browser/memory/tab_manager_delegate_chromeos.h
+++ b/chrome/browser/memory/tab_manager_delegate_chromeos.h
@@ -11,11 +11,13 @@
#include "base/containers/hash_tables.h"
#include "base/gtest_prod_util.h"
+#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/process/process.h"
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/arc/arc_process.h"
+#include "chrome/browser/memory/arc_process_adj_level.h"
#include "chrome/browser/memory/tab_manager.h"
#include "chrome/browser/memory/tab_stats.h"
#include "chrome/browser/ui/browser_list_observer.h"
@@ -26,47 +28,19 @@
namespace memory {
-// The importance of tabs/apps. The lower the value, the more likely a process
-// is to be selected on memory pressure. The values is additive, for example,
-// one tab could be of value (CHROME_NORMAL | CHROME_PINNED).
-// TODO(cylee): Refactor this CL so the prioritize logic is unified in
-// TabManager.
-enum ProcessPriority {
- // Processes on Android side which generally don't have an app window, and
- // possibly be auto relaunched if killed.
- ANDROID_BACKGROUND = 1,
- ANDROID_SERVICE = 1 << 1,
- ANDROID_CANT_SAVE_STATE = 1 << 2,
- ANDROID_PERCEPTIBLE = 1 << 3,
- ANDROID_VISIBLE = 1 << 4,
- ANDROID_TOP_SLEEPING = 1 << 5,
- ANDROID_FOREGROUND_SERVICE = 1 << 6,
- ANDROID_FOREGROUND = 1 << 7,
- // A chrome window can be one of the 3 exclusive types below:
- // internal page, normal page, or chrome app.
- CHROME_INTERNAL = 1 << 8,
- CHROME_NORMAL = 1 << 9,
- CHROME_APP = 1 << 10,
- // An android app which is on top of screen from Android's point of view,
- // but the app window is inactive from Chrome OS's point of view.
- // Give it a higher priority then normal chrome tab since it could not be
- // reloaded if killed.
- ANDROID_TOP_INACTIVE = CHROME_APP,
- // A chrome tab could have following 3 additional attributes
- // (not exclusive).
- CHROME_PINNED = 1 << 11,
- CHROME_MEDIA = 1 << 12,
- CHROME_CANT_SAVE_STATE = 1 << 13,
- // The highest level of priority. Either a selected tab or an Android app in
- // an active window. In theory there should be at most one process with this
- // priority at a time, but at the time of writing Chrome couldn't get Android
- // window stack info yet so there may be multiple Android apps be token as on
- // top of screen for now.
- CHROME_SELECTED = 1 << 14,
- ANDROID_TOP = CHROME_SELECTED,
-
- ANDROID_PERSISTENT = 1 << 30,
- ANDROID_NON_EXISTS = 0x7FFFFFFF
+// Possible types of Apps/Tabs processes. From most important to least
+// important.
+enum ProcessType {
+ // There can be only one process having process type "FOCUSED_APP"
+ // or "FOCUSED_TAB" in the system at any give time (i.e., The focused window
+ // could be either a chrome window or an Android app. But not both.
+ FOCUSED_APP = 1,
+ FOCUSED_TAB = FOCUSED_APP,
+
+ VISIBLE_APP = 2,
+ BACKGROUND_TAB = 3,
+ BACKGROUND_APP = 4,
+ UNKNOWN_TYPE = 5,
};
// The Chrome OS TabManagerDelegate is responsible for keeping the
@@ -132,7 +106,7 @@ class TabManagerDelegate : public arc::ArcBridgeService::Observer,
FRIEND_TEST_ALL_PREFIXES(TabManagerDelegateTest, KillMultipleProcesses);
FRIEND_TEST_ALL_PREFIXES(TabManagerDelegateTest, SetOomScoreAdj);
- struct Candidate;
+ class Candidate;
class FocusedProcess;
class UmaReporter;
@@ -150,7 +124,7 @@ class TabManagerDelegate : public arc::ArcBridgeService::Observer,
// Cache OOM scores in memory.
typedef base::hash_map<base::ProcessHandle, int> ProcessScoreMap;
- // Get the list of candidates to kill, sorted by reversed importance.
+ // Get the list of candidates to kill, sorted by importance descendingly.
static std::vector<Candidate> GetSortedCandidates(
const TabStatsList& tab_list,
const std::vector<arc::ArcProcess>& arc_processes);
@@ -188,8 +162,8 @@ class TabManagerDelegate : public arc::ArcBridgeService::Observer,
// distributed evenly in [|range_begin|, |range_end|).
// The new score is set in |new_map|.
void DistributeOomScoreInRange(
- std::vector<TabManagerDelegate::Candidate>::reverse_iterator rbegin,
- std::vector<TabManagerDelegate::Candidate>::reverse_iterator rend,
+ std::vector<TabManagerDelegate::Candidate>::const_iterator begin,
+ std::vector<TabManagerDelegate::Candidate>::const_iterator end,
int range_begin,
int range_end,
ProcessScoreMap* new_map);
@@ -234,25 +208,61 @@ class TabManagerDelegate : public arc::ArcBridgeService::Observer,
};
// On ARC enabled machines, either a tab or an app could be a possible
-// victim of low memory kill process. This is a helper struct which holds a
+// victim of low memory kill process. This is a helper class which holds a
// pointer to an app or a tab (but not both) to facilitate prioritizing the
// victims.
-struct TabManagerDelegate::Candidate {
- Candidate(const TabStats* _tab, int _priority)
- : tab(_tab), priority(_priority), is_arc_app(false) {}
- Candidate(const arc::ArcProcess* _app, int _priority)
- : app(_app), priority(_priority), is_arc_app(true) {}
+class TabManagerDelegate::Candidate {
+ public:
+ explicit Candidate(const TabStats* tab)
+ : tab_(tab), app_(NULL), process_type_(GetProcessType()) {
+ }
+ explicit Candidate(const arc::ArcProcess* app)
+ : tab_(NULL), app_(app), process_type_(GetProcessType()) {
+ }
+ // Higher priority first.
bool operator<(const Candidate& rhs) const {
- return priority < rhs.priority;
+ if (app() && rhs.app()) {
+ // Sort by (adj, last_activity_time) pair.
+ // Larger adj value means lower priority as defined in Android.
+ if (app()->adj() != rhs.app()->adj())
+ return app()->adj() < rhs.app()->adj();
+ // LRU manner.
+ return app()->last_activity_time() > rhs.app()->last_activity_time();
+ } else if (tab() && rhs.tab()) {
+ return TabManager::CompareTabStats(*tab(), *rhs.tab());
+ }
+ // If one is tab and the other is Android app, simply sort by their process
+ // type for now.
+ return process_type() < rhs.process_type();
+ }
+
+ const TabStats* tab() const { return tab_; }
+ const arc::ArcProcess* app() const { return app_; }
+ ProcessType process_type() const { return process_type_; }
+
+ private:
+ ProcessType GetProcessType() {
+ if (app()) {
+ if (app()->adj() == ArcProcessAdjLevel::FOREGROUND_APP_ADJ) {
+ return ProcessType::FOCUSED_APP;
+ } else if (app()->adj() == ArcProcessAdjLevel::VISIBLE_APP_ADJ) {
+ return ProcessType::VISIBLE_APP;
+ }
+ return ProcessType::BACKGROUND_APP;
+ } else if (tab()) {
+ if (tab()->is_selected) {
+ return ProcessType::FOCUSED_TAB;
+ }
+ return ProcessType::BACKGROUND_TAB;
+ }
+ NOTREACHED() << "Unexpected process type";
+ return ProcessType::UNKNOWN_TYPE;
}
- union {
- const TabStats* tab;
- const arc::ArcProcess* app;
- };
- int priority;
- bool is_arc_app;
+ const TabStats* tab_;
+ const arc::ArcProcess* app_;
+ ProcessType process_type_;
};
// A thin wrapper over library process_metric.h to get memory status so unit

Powered by Google App Engine
This is Rietveld 408576698