Merge "Fix app compaction bug and add tests." into rvc-dev am: 197709093a

Change-Id: Ib1bf66dc49b2a47e9bacdffd5bbeab2576c7c810

services/core/java/com/android/server/am/CachedAppOptimizer.java

@@ -45,6 +45,7 @@ import com.android.server.ServiceThread;
 
 import java.io.FileOutputStream;
 import java.io.FileReader;
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -114,6 +115,14 @@ public final class CachedAppOptimizer {
     }
     private PropertyChangedCallbackForTest mTestCallback;
 
+    // This interface is for functions related to the Process object that need a different
+    // implementation in the tests as we are not creating real processes when testing compaction.
+    @VisibleForTesting
+    interface ProcessDependencies {
+        long[] getRss(int pid);
+        void performCompaction(String action, int pid) throws IOException;
+    }
+
     // Handler constants.
     static final int COMPACT_PROCESS_SOME = 1;
     static final int COMPACT_PROCESS_FULL = 2;
@@ -215,13 +224,16 @@ public final class CachedAppOptimizer {
     @VisibleForTesting final Set<Integer> mProcStateThrottle;
 
     // Handler on which compaction runs.
-    private Handler mCompactionHandler;
+    @VisibleForTesting
+    Handler mCompactionHandler;
     private Handler mFreezeHandler;
 
     // Maps process ID to last compaction statistics for processes that we've fully compacted. Used
     // when evaluating throttles that we only consider for "full" compaction, so we don't store
-    // data for "some" compactions.
+    // data for "some" compactions. Uses LinkedHashMap to ensure insertion order is kept and
-    private Map<Integer, LastCompactionStats> mLastCompactionStats =
+    // facilitate removal of the oldest entry.
+    @VisibleForTesting
+    LinkedHashMap<Integer, LastCompactionStats> mLastCompactionStats =
             new LinkedHashMap<Integer, LastCompactionStats>() {
                 @Override
                 protected boolean removeEldestEntry(Map.Entry eldest) {
@@ -233,17 +245,20 @@ public final class CachedAppOptimizer {
     private int mFullCompactionCount;
     private int mPersistentCompactionCount;
     private int mBfgsCompactionCount;
+    private final ProcessDependencies mProcessDependencies;
 
     public CachedAppOptimizer(ActivityManagerService am) {
-        mAm = am;
+        this(am, null, new DefaultProcessDependencies());
-        mCachedAppOptimizerThread = new ServiceThread("CachedAppOptimizerThread",
-                THREAD_PRIORITY_FOREGROUND, true);
-        mProcStateThrottle = new HashSet<>();
     }
 
     @VisibleForTesting
-    CachedAppOptimizer(ActivityManagerService am, PropertyChangedCallbackForTest callback) {
+    CachedAppOptimizer(ActivityManagerService am, PropertyChangedCallbackForTest callback,
-        this(am);
+            ProcessDependencies processDependencies) {
+        mAm = am;
+        mCachedAppOptimizerThread = new ServiceThread("CachedAppOptimizerThread",
+            THREAD_PRIORITY_FOREGROUND, true);
+        mProcStateThrottle = new HashSet<>();
+        mProcessDependencies = processDependencies;
         mTestCallback = callback;
     }
 
@@ -659,7 +674,8 @@ public final class CachedAppOptimizer {
         }
     }
 
-    private static final class LastCompactionStats {
+    @VisibleForTesting
+    static final class LastCompactionStats {
         private final long[] mRssAfterCompaction;
 
         LastCompactionStats(long[] rss) {
@@ -712,9 +728,7 @@ public final class CachedAppOptimizer {
 
                         lastCompactAction = proc.lastCompactAction;
                         lastCompactTime = proc.lastCompactTime;
-                        // remove rather than get so that insertion order will be updated when we
+                        lastCompactionStats = mLastCompactionStats.get(pid);
-                        // put the post-compaction stats back into the map.
-                        lastCompactionStats = mLastCompactionStats.remove(pid);
                     }
 
                     if (pid == 0) {
@@ -806,7 +820,7 @@ public final class CachedAppOptimizer {
                         return;
                     }
 
-                    long[] rssBefore = Process.getRss(pid);
+                    long[] rssBefore = mProcessDependencies.getRss(pid);
                     long anonRssBefore = rssBefore[2];
 
                     if (rssBefore[0] == 0 && rssBefore[1] == 0 && rssBefore[2] == 0
@@ -863,16 +877,13 @@ public final class CachedAppOptimizer {
                         default:
                             break;
                     }
-
                     try {
                         Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "Compact "
                                 + ((pendingAction == COMPACT_PROCESS_SOME) ? "some" : "full")
                                 + ": " + name);
                         long zramFreeKbBefore = Debug.getZramFreeKb();
-                        FileOutputStream fos = new FileOutputStream("/proc/" + pid + "/reclaim");
+                        mProcessDependencies.performCompaction(action, pid);
-                        fos.write(action.getBytes());
+                        long[] rssAfter = mProcessDependencies.getRss(pid);
-                        fos.close();
-                        long[] rssAfter = Process.getRss(pid);
                         long end = SystemClock.uptimeMillis();
                         long time = end - start;
                         long zramFreeKbAfter = Debug.getZramFreeKb();
@@ -882,7 +893,6 @@ public final class CachedAppOptimizer {
                                 rssAfter[2] - rssBefore[2], rssAfter[3] - rssBefore[3], time,
                                 lastCompactAction, lastCompactTime, lastOomAdj, procState,
                                 zramFreeKbBefore, zramFreeKbAfter - zramFreeKbBefore);
-
                         // Note that as above not taking mPhenoTypeFlagLock here to avoid locking
                         // on every single compaction for a flag that will seldom change and the
                         // impact of reading the wrong value here is low.
@@ -894,14 +904,14 @@ public final class CachedAppOptimizer {
                                     lastOomAdj, ActivityManager.processStateAmToProto(procState),
                                     zramFreeKbBefore, zramFreeKbAfter);
                         }
-
                         synchronized (mAm) {
                             proc.lastCompactTime = end;
                             proc.lastCompactAction = pendingAction;
                         }
-
                         if (action.equals(COMPACT_ACTION_FULL)
                                 || action.equals(COMPACT_ACTION_ANON)) {
+                            // Remove entry and insert again to update insertion order.
+                            mLastCompactionStats.remove(pid);
                             mLastCompactionStats.put(pid, new LastCompactionStats(rssAfter));
                         }
                     } catch (Exception e) {
@@ -1018,4 +1028,23 @@ public final class CachedAppOptimizer {
             }
         }
     }
+
+    /**
+     * Default implementation for ProcessDependencies, public vor visibility to OomAdjuster class.
+     */
+    private static final class DefaultProcessDependencies implements ProcessDependencies {
+        // Get memory RSS from process.
+        @Override
+        public long[] getRss(int pid) {
+            return Process.getRss(pid);
+        }
+
+        // Compact process.
+        @Override
+        public void performCompaction(String action, int pid) throws IOException {
+            try (FileOutputStream fos = new FileOutputStream("/proc/" + pid + "/reclaim")) {
+                fos.write(action.getBytes());
+            }
+        }
+    }
 }

services/tests/mockingservicestests/src/com/android/server/am/CachedAppOptimizerTest.java

@@ -16,14 +16,23 @@
 
 package com.android.server.am;
 
+import static android.app.ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE;
+
 import static com.android.server.am.ActivityManagerService.Injector;
 import static com.android.server.am.CachedAppOptimizer.compactActionIntToString;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+import android.content.ComponentName;
 import android.content.Context;
+import android.content.pm.ApplicationInfo;
+import android.content.pm.PackageManagerInternal;
 import android.os.Handler;
 import android.os.HandlerThread;
+import android.os.MessageQueue;
 import android.os.Process;
 import android.platform.test.annotations.Presubmit;
 import android.provider.DeviceConfig;
@@ -31,9 +40,11 @@ import android.text.TextUtils;
 
 import androidx.test.platform.app.InstrumentationRegistry;
 
+import com.android.server.LocalServices;
 import com.android.server.ServiceThread;
 import com.android.server.appop.AppOpsService;
 import com.android.server.testables.TestableDeviceConfig;
+import com.android.server.wm.ActivityTaskManagerService;
 
 import org.junit.After;
 import org.junit.Assume;
@@ -45,6 +56,7 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
@@ -68,25 +80,36 @@ public final class CachedAppOptimizerTest {
     private HandlerThread mHandlerThread;
     private Handler mHandler;
     private CountDownLatch mCountDown;
+    private ActivityManagerService mAms;
+    private Context mContext;
+    private TestInjector mInjector;
+    private TestProcessDependencies mProcessDependencies;
+
+    @Mock
+    private PackageManagerInternal mPackageManagerInt;
 
     @Rule
-    public TestableDeviceConfig.TestableDeviceConfigRule
+    public final TestableDeviceConfig.TestableDeviceConfigRule
             mDeviceConfigRule = new TestableDeviceConfig.TestableDeviceConfigRule();
+    @Rule
+    public final ApplicationExitInfoTest.ServiceThreadRule
+            mServiceThreadRule = new ApplicationExitInfoTest.ServiceThreadRule();
 
     @Before
     public void setUp() {
         mHandlerThread = new HandlerThread("");
         mHandlerThread.start();
         mHandler = new Handler(mHandlerThread.getLooper());
-
         mThread = new ServiceThread("TestServiceThread", Process.THREAD_PRIORITY_DEFAULT,
                 true /* allowIo */);
         mThread.start();
-
+        mContext = InstrumentationRegistry.getInstrumentation().getContext();
-        ActivityManagerService ams = new ActivityManagerService(
+        mInjector = new TestInjector(mContext);
-                new TestInjector(InstrumentationRegistry.getInstrumentation().getContext()),
+        mAms = new ActivityManagerService(
-                mThread);
+                new TestInjector(mContext), mServiceThreadRule.getThread());
-        mCachedAppOptimizerUnderTest = new CachedAppOptimizer(ams,
+        doReturn(new ComponentName("", "")).when(mPackageManagerInt).getSystemUiServiceComponent();
+        mProcessDependencies = new TestProcessDependencies();
+        mCachedAppOptimizerUnderTest = new CachedAppOptimizer(mAms,
                 new CachedAppOptimizer.PropertyChangedCallbackForTest() {
                     @Override
                     public void onPropertyChanged() {
@@ -94,7 +117,9 @@ public final class CachedAppOptimizerTest {
                             mCountDown.countDown();
                         }
                     }
-                });
+                }, mProcessDependencies);
+        LocalServices.removeServiceForTest(PackageManagerInternal.class);
+        LocalServices.addService(PackageManagerInternal.class, mPackageManagerInt);
     }
 
     @After
@@ -104,6 +129,19 @@ public final class CachedAppOptimizerTest {
         mCountDown = null;
     }
 
+    private ProcessRecord makeProcessRecord(int pid, int uid, int packageUid, String processName,
+            String packageName) {
+        ApplicationInfo ai = new ApplicationInfo();
+        ai.packageName = packageName;
+        ProcessRecord app = new ProcessRecord(mAms, ai, processName, uid);
+        app.pid = pid;
+        app.info.uid = packageUid;
+        // Exact value does not mater, it can be any state for which compaction is allowed.
+        app.setProcState = PROCESS_STATE_BOUND_FOREGROUND_SERVICE;
+        app.setAdj = 905;
+        return app;
+    }
+
     @Test
     public void init_setsDefaults() {
         mCachedAppOptimizerUnderTest.init();
@@ -197,7 +235,7 @@ public final class CachedAppOptimizerTest {
                 CachedAppOptimizer.DEFAULT_USE_FREEZER);
         DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ACTIVITY_MANAGER,
                 CachedAppOptimizer.KEY_USE_FREEZER, CachedAppOptimizer.DEFAULT_USE_FREEZER
-                ?  "false" : "true" , false);
+                        ? "false" : "true", false);
 
         // Then calling init will read and set that flag.
         mCachedAppOptimizerUnderTest.init();
@@ -790,6 +828,174 @@ public final class CachedAppOptimizerTest {
                 .containsExactlyElementsIn(expected);
     }
 
+    @Test
+    public void processWithDeltaRSSTooSmall_notFullCompacted() throws Exception {
+        // Initialize CachedAppOptimizer and set flags to (1) enable compaction, (2) set RSS
+        // throttle to 12000.
+        mCachedAppOptimizerUnderTest.init();
+        setFlag(CachedAppOptimizer.KEY_USE_COMPACTION, "true", true);
+        setFlag(CachedAppOptimizer.KEY_COMPACT_FULL_DELTA_RSS_THROTTLE_KB, "12000", false);
+        initActivityManagerService();
+
+        // Simulate RSS anon memory larger than throttle.
+        long[] rssBefore1 =
+                new long[]{/*totalRSS*/ 10000, /*fileRSS*/ 10000, /*anonRSS*/ 12000, /*swap*/
+                        10000};
+        long[] rssAfter1 =
+                new long[]{/*totalRSS*/ 9000, /*fileRSS*/ 9000, /*anonRSS*/ 11000, /*swap*/9000};
+        // Delta between rssAfter1 and rssBefore2 is below threshold (500).
+        long[] rssBefore2 =
+                new long[]{/*totalRSS*/ 9500, /*fileRSS*/ 9500, /*anonRSS*/ 11500, /*swap*/9500};
+        long[] rssAfter2 =
+                new long[]{/*totalRSS*/ 8000, /*fileRSS*/ 8000, /*anonRSS*/ 9000, /*swap*/8000};
+        // Delta between rssAfter1 and rssBefore3 is above threshold (13000).
+        long[] rssBefore3 =
+                new long[]{/*totalRSS*/ 10000, /*fileRSS*/ 18000, /*anonRSS*/ 13000, /*swap*/ 7000};
+        long[] rssAfter3 =
+                new long[]{/*totalRSS*/ 10000, /*fileRSS*/ 11000, /*anonRSS*/ 10000, /*swap*/ 6000};
+        long[] valuesAfter = {};
+        // Process that passes properties.
+        int pid = 1;
+        ProcessRecord processRecord = makeProcessRecord(pid, 2, 3, "p1", "app1");
+
+        // GIVEN we simulate RSS memory before above thresholds and it is the first time 'p1' is
+        // compacted.
+        mProcessDependencies.setRss(rssBefore1);
+        mProcessDependencies.setRssAfterCompaction(rssAfter1); //
+        // WHEN we try to run compaction
+        mCachedAppOptimizerUnderTest.compactAppFull(processRecord);
+        waitForHandler();
+        // THEN process IS compacted.
+        assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull();
+        valuesAfter = mCachedAppOptimizerUnderTest.mLastCompactionStats.get(
+                pid).getRssAfterCompaction();
+        assertThat(valuesAfter).isEqualTo(rssAfter1);
+
+        // WHEN delta is below threshold (500).
+        mProcessDependencies.setRss(rssBefore2);
+        mProcessDependencies.setRssAfterCompaction(rssAfter2);
+        // This is to avoid throttle of compacting too soon.
+        processRecord.lastCompactTime = processRecord.lastCompactTime - 10_000;
+        // WHEN we try to run compaction.
+        mCachedAppOptimizerUnderTest.compactAppFull(processRecord);
+        waitForHandler();
+        // THEN process IS NOT compacted - values after compaction for process 1 should remain the
+        // same as from the last compaction.
+        assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull();
+        valuesAfter = mCachedAppOptimizerUnderTest.mLastCompactionStats.get(
+                pid).getRssAfterCompaction();
+        assertThat(valuesAfter).isEqualTo(rssAfter1);
+
+        // WHEN delta is above threshold (13000).
+        mProcessDependencies.setRss(rssBefore3);
+        mProcessDependencies.setRssAfterCompaction(rssAfter3);
+        // This is to avoid throttle of compacting too soon.
+        processRecord.lastCompactTime = processRecord.lastCompactTime - 10_000;
+        // WHEN we try to run compaction
+        mCachedAppOptimizerUnderTest.compactAppFull(processRecord);
+        waitForHandler();
+        // THEN process IS compacted - values after compaction for process 1 should be updated.
+        assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull();
+        valuesAfter = mCachedAppOptimizerUnderTest.mLastCompactionStats.get(
+                pid).getRssAfterCompaction();
+        assertThat(valuesAfter).isEqualTo(rssAfter3);
+
+    }
+
+    @Test
+    public void processWithAnonRSSTooSmall_notFullCompacted() throws Exception {
+        // Initialize CachedAppOptimizer and set flags to (1) enable compaction, (2) set RSS
+        // throttle to 8000.
+        mCachedAppOptimizerUnderTest.init();
+        setFlag(CachedAppOptimizer.KEY_USE_COMPACTION, "true", true);
+        setFlag(CachedAppOptimizer.KEY_COMPACT_FULL_RSS_THROTTLE_KB, "8000", false);
+        initActivityManagerService();
+
+        // Simulate RSS anon memory larger than throttle.
+        long[] rssBelowThreshold =
+                new long[]{/*Total RSS*/ 10000, /*File RSS*/ 10000, /*Anon RSS*/ 7000, /*Swap*/
+                        10000};
+        long[] rssBelowThresholdAfter =
+                new long[]{/*Total RSS*/ 9000, /*File RSS*/ 7000, /*Anon RSS*/ 4000, /*Swap*/
+                        8000};
+        long[] rssAboveThreshold =
+                new long[]{/*Total RSS*/ 10000, /*File RSS*/ 10000, /*Anon RSS*/ 9000, /*Swap*/
+                        10000};
+        long[] rssAboveThresholdAfter =
+                new long[]{/*Total RSS*/ 8000, /*File RSS*/ 9000, /*Anon RSS*/ 6000, /*Swap*/5000};
+        // Process that passes properties.
+        int pid = 1;
+        ProcessRecord processRecord =
+                makeProcessRecord(pid, 2, 3, "p1",
+                        "app1");
+
+        // GIVEN we simulate RSS memory before below threshold.
+        mProcessDependencies.setRss(rssBelowThreshold);
+        mProcessDependencies.setRssAfterCompaction(rssBelowThresholdAfter);
+        // WHEN we try to run compaction
+        mCachedAppOptimizerUnderTest.compactAppFull(processRecord);
+        waitForHandler();
+        // THEN process IS NOT compacted.
+        assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNull();
+
+        // GIVEN we simulate RSS memory before above threshold.
+        mProcessDependencies.setRss(rssAboveThreshold);
+        mProcessDependencies.setRssAfterCompaction(rssAboveThresholdAfter);
+        // WHEN we try to run compaction
+        mCachedAppOptimizerUnderTest.compactAppFull(processRecord);
+        waitForHandler();
+        // THEN process IS compacted.
+        assertThat(mCachedAppOptimizerUnderTest.mLastCompactionStats.get(pid)).isNotNull();
+        long[] valuesAfter = mCachedAppOptimizerUnderTest.mLastCompactionStats.get(
+                pid).getRssAfterCompaction();
+        assertThat(valuesAfter).isEqualTo(rssAboveThresholdAfter);
+    }
+
+
+    private void setFlag(String key, String value, boolean defaultValue) throws Exception {
+        mCountDown = new CountDownLatch(1);
+        DeviceConfig.setProperty(DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, key, value, defaultValue);
+        assertThat(mCountDown.await(5, TimeUnit.SECONDS)).isTrue();
+    }
+
+    private void waitForHandler() {
+        Idle idle = new Idle();
+        mCachedAppOptimizerUnderTest.mCompactionHandler.getLooper().getQueue().addIdleHandler(idle);
+        mCachedAppOptimizerUnderTest.mCompactionHandler.post(() -> { });
+        idle.waitForIdle();
+    }
+
+    private void initActivityManagerService() {
+        mAms = new ActivityManagerService(mInjector, mServiceThreadRule.getThread());
+        mAms.mActivityTaskManager = new ActivityTaskManagerService(mContext);
+        mAms.mActivityTaskManager.initialize(null, null, mContext.getMainLooper());
+        mAms.mAtmInternal = spy(mAms.mActivityTaskManager.getAtmInternal());
+        mAms.mPackageManagerInt = mPackageManagerInt;
+    }
+
+    private static final class Idle implements MessageQueue.IdleHandler {
+        private boolean mIdle;
+
+        @Override
+        public boolean queueIdle() {
+            synchronized (this) {
+                mIdle = true;
+                notifyAll();
+            }
+            return false;
+        }
+
+        public synchronized void waitForIdle() {
+            while (!mIdle) {
+                try {
+                    // Wait with a timeout of 10s.
+                    wait(10000);
+                } catch (InterruptedException e) {
+                }
+            }
+        }
+    }
+
     private class TestInjector extends Injector {
 
         TestInjector(Context context) {
@@ -806,4 +1012,29 @@ public final class CachedAppOptimizerTest {
             return mHandler;
         }
     }
+
+    // Test implementation for ProcessDependencies.
+    private static final class TestProcessDependencies
+            implements CachedAppOptimizer.ProcessDependencies {
+        private long[] mRss;
+        private long[] mRssAfterCompaction;
+
+        @Override
+        public long[] getRss(int pid) {
+            return mRss;
+        }
+
+        @Override
+        public void performCompaction(String action, int pid) throws IOException {
+            mRss = mRssAfterCompaction;
+        }
+
+        public void setRss(long[] newValues) {
+            mRss = newValues;
+        }
+
+        public void setRssAfterCompaction(long[] newValues) {
+            mRssAfterCompaction = newValues;
+        }
+    }
 }