From 7d611c750c3d518794b2e580539bf9a22222a41f Mon Sep 17 00:00:00 2001
From: Misha Wagner <mishaw@google.com>
Date: Mon, 20 Sep 2021 13:42:27 +0100
Subject: [PATCH] Change CacheOomRanker's "uses" feature.

The previous implementation was incorrect, as an app goes in and out of
the cache several times during oom_adj calculation. If all apps started
at the same time, then this would be OK - as they all get incremented
the same amount, plus get incremented when they actually go in and out
of the cache. This would result in the relative counts being correct.

However, as all apps aren't started at the same time, the old
implementation is instead a measure of how long the app has been
started.

So instead we use mSetProcState, which isn't changed on each oom_adj
calculation. This measure should accurately reflect the number of times
the process is used - whether that's by a user app open, or a content
receiver call, or an intent being broadcast.

See go/sim-v-impl for more details.

Test: atest CacheOomRankerTest
Bug: 196031723
Change-Id: I49d02362b8277f8472355e78412f79256c2289a1
Merged-In: I49d02362b8277f8472355e78412f79256c2289a1
(cherry picked from commit 480cc3123c5ee6ae4fff56599124794a0ff25aa4)
---
 core/java/android/app/ActivityManager.java            |  5 +++++
 .../com/android/server/am/ProcessStateRecord.java     | 11 +++++------
 .../src/com/android/server/am/CacheOomRankerTest.java |  9 +++++----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/core/java/android/app/ActivityManager.java b/core/java/android/app/ActivityManager.java
index 4376d225e676d..f53c5b6c97482 100644
--- a/core/java/android/app/ActivityManager.java
+++ b/core/java/android/app/ActivityManager.java
@@ -771,6 +771,11 @@ public class ActivityManager {
         return procState >= PROCESS_STATE_TRANSIENT_BACKGROUND;
     }
 
+    /** @hide Should this process state be considered in the cache? */
+    public static final boolean isProcStateCached(int procState) {
+        return procState >= PROCESS_STATE_CACHED_ACTIVITY;
+    }
+
     /** @hide Is this a foreground service type? */
     public static boolean isForegroundService(int procState) {
         return procState == PROCESS_STATE_FOREGROUND_SERVICE;
diff --git a/services/core/java/com/android/server/am/ProcessStateRecord.java b/services/core/java/com/android/server/am/ProcessStateRecord.java
index d4474d6637695..7c3928570ae2b 100644
--- a/services/core/java/com/android/server/am/ProcessStateRecord.java
+++ b/services/core/java/com/android/server/am/ProcessStateRecord.java
@@ -577,6 +577,10 @@ final class ProcessStateRecord {
 
     @GuardedBy({"mService", "mProcLock"})
     void setSetProcState(int setProcState) {
+        if (ActivityManager.isProcStateCached(mSetProcState)
+                && !ActivityManager.isProcStateCached(setProcState)) {
+            mCacheOomRankerUseCount++;
+        }
         mSetProcState = setProcState;
     }
 
@@ -840,12 +844,7 @@ final class ProcessStateRecord {
 
     @GuardedBy("mService")
     void setCached(boolean cached) {
-        if (mCached != cached) {
-            mCached = cached;
-            if (cached) {
-                ++mCacheOomRankerUseCount;
-            }
-        }
+        mCached = cached;
     }
 
     @GuardedBy("mService")
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/CacheOomRankerTest.java b/services/tests/mockingservicestests/src/com/android/server/am/CacheOomRankerTest.java
index 15dfd266853ee..d0c9242969454 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/CacheOomRankerTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/CacheOomRankerTest.java
@@ -24,6 +24,7 @@ import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 
+import android.app.ActivityManager;
 import android.app.IApplicationThread;
 import android.content.ComponentName;
 import android.content.Context;
@@ -581,7 +582,7 @@ public class CacheOomRankerTest {
     }
 
     private ProcessRecord nextProcessRecord(int setAdj, long lastActivityTime, long lastRss,
-            int returnedToCacheCount) {
+            int wentToForegroundCount) {
         ApplicationInfo ai = new ApplicationInfo();
         ai.packageName = "a.package.name" + mNextPackageName++;
         ProcessRecord app = new ProcessRecord(mAms, ai, ai.packageName + ":process", mNextUid++);
@@ -593,9 +594,9 @@ public class CacheOomRankerTest {
         app.setLastActivityTime(lastActivityTime);
         app.mProfile.setLastRss(lastRss);
         app.mState.setCached(false);
-        for (int i = 0; i < returnedToCacheCount; ++i) {
-            app.mState.setCached(false);
-            app.mState.setCached(true);
+        for (int i = 0; i < wentToForegroundCount; ++i) {
+            app.mState.setSetProcState(ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE);
+            app.mState.setSetProcState(ActivityManager.PROCESS_STATE_CACHED_RECENT);
         }
         // Sets the thread returned by ProcessRecord#getThread, which we use to check whether the
         // app is currently launching.