Parse /proc/pid/status for memory only once

Read and parse /proc/pid/status for native processes only once.
Previously, it was read twice (getUidForPid and read*FromProcfs) and
regular expressions were applied to its contents few times.

A single read also allowed to simplify the code: use null to represent
failed read.

Bug: 140986627
Test: atest ProcfsMemoryUtilTest
Test: atest UidAtomTests#testProcessMemorySnapshot
Test: atest UidAtomTests#testProcessMemoryHighWaterMark
Test: atest UidAtomTests#testNativeProcessMemoryState
Change-Id: I09d05b663f99602fcfacae253af7349989d48749

3 files changed, 86 insertions, 111 deletions

diff --git a/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java b/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
index d49b9589cb15..2b25b8921540 100644
--- a/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
+++ b/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
@@ -15,6 +15,7 @@
  */
 package com.android.server.stats;
 
+import android.annotation.Nullable;
 import android.os.FileUtils;
 import android.util.Slog;
 
@@ -22,61 +23,53 @@ import com.android.internal.annotations.VisibleForTesting;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Locale;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 final class ProcfsMemoryUtil {
     private static final String TAG = "ProcfsMemoryUtil";
 
-    /** Path to procfs status file: /proc/pid/status. */
-    private static final String STATUS_FILE_FMT = "/proc/%d/status";
-
-    private static final Pattern RSS_HIGH_WATER_MARK_IN_KILOBYTES =
-            Pattern.compile("VmHWM:\\s*(\\d+)\\s*kB");
-    private static final Pattern RSS_IN_KILOBYTES =
-            Pattern.compile("VmRSS:\\s*(\\d+)\\s*kB");
-    private static final Pattern ANON_RSS_IN_KILOBYTES =
-            Pattern.compile("RssAnon:\\s*(\\d+)\\s*kB");
-    private static final Pattern SWAP_IN_KILOBYTES =
-            Pattern.compile("VmSwap:\\s*(\\d+)\\s*kB");
+    private static final Pattern STATUS_MEMORY_STATS =
+            Pattern.compile(String.join(
+                    ".*",
+                    "Uid:\\s*(\\d+)\\s*",
+                    "VmHWM:\\s*(\\d+)\\s*kB",
+                    "VmRSS:\\s*(\\d+)\\s*kB",
+                    "RssAnon:\\s*(\\d+)\\s*kB",
+                    "VmSwap:\\s*(\\d+)\\s*kB"), Pattern.DOTALL);
 
     private ProcfsMemoryUtil() {}
 
     /**
-     * Reads RSS high-water mark of a process from procfs. Returns value of the VmHWM field in
-     * /proc/PID/status in kilobytes or 0 if not available.
-     */
-    static int readRssHighWaterMarkFromProcfs(int pid) {
-        final String statusPath = String.format(Locale.US, STATUS_FILE_FMT, pid);
-        return parseVmHWMFromStatus(readFile(statusPath));
-    }
-
-    /**
-     * Parses RSS high-water mark out from the contents of the /proc/pid/status file in procfs. The
-     * returned value is in kilobytes.
-     */
-    @VisibleForTesting
-    static int parseVmHWMFromStatus(String contents) {
-        return tryParseInt(contents, RSS_HIGH_WATER_MARK_IN_KILOBYTES);
-    }
-
-    /**
-     * Reads memory stat of a process from procfs. Returns values of the VmRss, AnonRSS, VmSwap
-     * fields in /proc/pid/status in kilobytes or 0 if not available.
+     * Reads memory stats of a process from procfs. Returns values of the VmHWM, VmRss, AnonRSS,
+     * VmSwap fields in /proc/pid/status in kilobytes or null if not available.
      */
+    @Nullable
     static MemorySnapshot readMemorySnapshotFromProcfs(int pid) {
-        final String statusPath = String.format(Locale.US, STATUS_FILE_FMT, pid);
-        return parseMemorySnapshotFromStatus(readFile(statusPath));
+        return parseMemorySnapshotFromStatus(readFile("/proc/" + pid + "/status"));
     }
 
     @VisibleForTesting
+    @Nullable
     static MemorySnapshot parseMemorySnapshotFromStatus(String contents) {
-        final MemorySnapshot snapshot = new MemorySnapshot();
-        snapshot.rssInKilobytes = tryParseInt(contents, RSS_IN_KILOBYTES);
-        snapshot.anonRssInKilobytes = tryParseInt(contents, ANON_RSS_IN_KILOBYTES);
-        snapshot.swapInKilobytes = tryParseInt(contents, SWAP_IN_KILOBYTES);
-        return snapshot;
+        if (contents.isEmpty()) {
+            return null;
+        }
+        try {
+            final Matcher matcher = STATUS_MEMORY_STATS.matcher(contents);
+            if (matcher.find()) {
+                final MemorySnapshot snapshot = new MemorySnapshot();
+                snapshot.uid = Integer.parseInt(matcher.group(1));
+                snapshot.rssHighWaterMarkInKilobytes = Integer.parseInt(matcher.group(2));
+                snapshot.rssInKilobytes = Integer.parseInt(matcher.group(3));
+                snapshot.anonRssInKilobytes = Integer.parseInt(matcher.group(4));
+                snapshot.swapInKilobytes = Integer.parseInt(matcher.group(5));
+                return snapshot;
+            }
+        } catch (NumberFormatException e) {
+            Slog.e(TAG, "Failed to parse value", e);
+        }
+        return null;
     }
 
     private static String readFile(String path) {
@@ -88,26 +81,11 @@ final class ProcfsMemoryUtil {
         }
     }
 
-    private static int tryParseInt(String contents, Pattern pattern) {
-        if (contents.isEmpty()) {
-            return 0;
-        }
-        final Matcher matcher = pattern.matcher(contents);
-        try {
-            return matcher.find() ? Integer.parseInt(matcher.group(1)) : 0;
-        } catch (NumberFormatException e) {
-            Slog.e(TAG, "Failed to parse value", e);
-            return 0;
-        }
-    }
-
     static final class MemorySnapshot {
+        public int uid;
+        public int rssHighWaterMarkInKilobytes;
         public int rssInKilobytes;
         public int anonRssInKilobytes;
         public int swapInKilobytes;
-
-        boolean isEmpty() {
-            return (anonRssInKilobytes + swapInKilobytes) == 0;
-        }
     }
 }
diff --git a/services/core/java/com/android/server/stats/StatsCompanionService.java b/services/core/java/com/android/server/stats/StatsCompanionService.java
index e1a48ed3b550..67830a930caf 100644
--- a/services/core/java/com/android/server/stats/StatsCompanionService.java
+++ b/services/core/java/com/android/server/stats/StatsCompanionService.java
@@ -30,7 +30,6 @@ import static com.android.server.am.MemoryStatUtil.readMemoryStatFromProcfs;
 import static com.android.server.stats.IonMemoryUtil.readProcessSystemIonHeapSizesFromDebugfs;
 import static com.android.server.stats.IonMemoryUtil.readSystemIonHeapSizeFromDebugfs;
 import static com.android.server.stats.ProcfsMemoryUtil.readMemorySnapshotFromProcfs;
-import static com.android.server.stats.ProcfsMemoryUtil.readRssHighWaterMarkFromProcfs;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -245,6 +244,13 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
             "zygote",
             "zygote64",
     };
+    /**
+     * Lowest available uid for apps.
+     *
+     * <p>Used to quickly discard memory snapshots of the zygote forks from native process
+     * measurements.
+     */
+    private static final int MIN_APP_UID = 10_000;
 
     private static final int CPU_TIME_PER_THREAD_FREQ_MAX_NUM_FREQUENCIES = 8;
 
@@ -1197,20 +1203,18 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
     private void pullNativeProcessMemoryState(
             int tagId, long elapsedNanos, long wallClockNanos,
             List<StatsLogEventWrapper> pulledData) {
-        final List<String> processNames = Arrays.asList(MEMORY_INTERESTING_NATIVE_PROCESSES);
         int[] pids = getPidsForCommands(MEMORY_INTERESTING_NATIVE_PROCESSES);
-        for (int i = 0; i < pids.length; i++) {
-            int pid = pids[i];
+        for (int pid : pids) {
+            String processName = readCmdlineFromProcfs(pid);
             MemoryStat memoryStat = readMemoryStatFromProcfs(pid);
             if (memoryStat == null) {
                 continue;
             }
             int uid = getUidForPid(pid);
-            String processName = readCmdlineFromProcfs(pid);
-            // Sometimes we get here processName that is not included in the whitelist. It comes
+            // Sometimes we get here a process that is not included in the whitelist. It comes
             // from forking the zygote for an app. We can ignore that sample because this process
             // is collected by ProcessMemoryState.
-            if (!processNames.contains(processName)) {
+            if (isAppUid(uid)) {
                 continue;
             }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
@@ -1238,34 +1242,37 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
                 LocalServices.getService(
                         ActivityManagerInternal.class).getMemoryStateForProcesses();
         for (ProcessMemoryState managedProcess : managedProcessList) {
-            final int rssHighWaterMarkInKilobytes =
-                    readRssHighWaterMarkFromProcfs(managedProcess.pid);
-            if (rssHighWaterMarkInKilobytes == 0) {
+            final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(managedProcess.pid);
+            if (snapshot == null) {
                 continue;
             }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
             e.writeInt(managedProcess.uid);
             e.writeString(managedProcess.processName);
             // RSS high-water mark in bytes.
-            e.writeLong((long) rssHighWaterMarkInKilobytes * 1024L);
-            e.writeInt(rssHighWaterMarkInKilobytes);
+            e.writeLong((long) snapshot.rssHighWaterMarkInKilobytes * 1024L);
+            e.writeInt(snapshot.rssHighWaterMarkInKilobytes);
             pulledData.add(e);
         }
         int[] pids = getPidsForCommands(MEMORY_INTERESTING_NATIVE_PROCESSES);
-        for (int i = 0; i < pids.length; i++) {
-            final int pid = pids[i];
-            final int uid = getUidForPid(pid);
+        for (int pid : pids) {
             final String processName = readCmdlineFromProcfs(pid);
-            final int rssHighWaterMarkInKilobytes = readRssHighWaterMarkFromProcfs(pid);
-            if (rssHighWaterMarkInKilobytes == 0) {
+            final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid);
+            if (snapshot == null) {
+                continue;
+            }
+            // Sometimes we get here a process that is not included in the whitelist. It comes
+            // from forking the zygote for an app. We can ignore that sample because this process
+            // is collected by ProcessMemoryState.
+            if (isAppUid(snapshot.uid)) {
                 continue;
             }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
-            e.writeInt(uid);
+            e.writeInt(snapshot.uid);
             e.writeString(processName);
             // RSS high-water mark in bytes.
-            e.writeLong((long) rssHighWaterMarkInKilobytes * 1024L);
-            e.writeInt(rssHighWaterMarkInKilobytes);
+            e.writeLong((long) snapshot.rssHighWaterMarkInKilobytes * 1024L);
+            e.writeInt(snapshot.rssHighWaterMarkInKilobytes);
             pulledData.add(e);
         }
         // Invoke rss_hwm_reset binary to reset RSS HWM counters for all processes.
@@ -1279,15 +1286,15 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
                 LocalServices.getService(
                         ActivityManagerInternal.class).getMemoryStateForProcesses();
         for (ProcessMemoryState managedProcess : managedProcessList) {
+            final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(managedProcess.pid);
+            if (snapshot == null) {
+                continue;
+            }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
             e.writeInt(managedProcess.uid);
             e.writeString(managedProcess.processName);
             e.writeInt(managedProcess.pid);
             e.writeInt(managedProcess.oomScore);
-            final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(managedProcess.pid);
-            if (snapshot.isEmpty()) {
-                continue;
-            }
             e.writeInt(snapshot.rssInKilobytes);
             e.writeInt(snapshot.anonRssInKilobytes);
             e.writeInt(snapshot.swapInKilobytes);
@@ -1296,15 +1303,22 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
         }
         int[] pids = getPidsForCommands(MEMORY_INTERESTING_NATIVE_PROCESSES);
         for (int pid : pids) {
-            StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
-            e.writeInt(getUidForPid(pid));
-            e.writeString(readCmdlineFromProcfs(pid));
-            e.writeInt(pid);
-            e.writeInt(-1001);  // Placeholder for native processes, OOM_SCORE_ADJ_MIN - 1.
+            final String processName = readCmdlineFromProcfs(pid);
             final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid);
-            if (snapshot.isEmpty()) {
+            if (snapshot == null) {
                 continue;
             }
+            // Sometimes we get here a process that is not included in the whitelist. It comes
+            // from forking the zygote for an app. We can ignore that sample because this process
+            // is collected by ProcessMemoryState.
+            if (isAppUid(snapshot.uid)) {
+                continue;
+            }
+            StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
+            e.writeInt(snapshot.uid);
+            e.writeString(processName);
+            e.writeInt(pid);
+            e.writeInt(-1001);  // Placeholder for native processes, OOM_SCORE_ADJ_MIN - 1.
             e.writeInt(snapshot.rssInKilobytes);
             e.writeInt(snapshot.anonRssInKilobytes);
             e.writeInt(snapshot.swapInKilobytes);
@@ -1313,6 +1327,10 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
         }
     }
 
+    private static boolean isAppUid(int uid) {
+        return uid >= MIN_APP_UID;
+    }
+
     private void pullSystemIonHeapSize(
             int tagId, long elapsedNanos, long wallClockNanos,
             List<StatsLogEventWrapper> pulledData) {
diff --git a/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java b/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java
index ae5777403528..dd2ee5cce13b 100644
--- a/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java
+++ b/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java
@@ -16,7 +16,6 @@
 package com.android.server.stats;
 
 import static com.android.server.stats.ProcfsMemoryUtil.parseMemorySnapshotFromStatus;
-import static com.android.server.stats.ProcfsMemoryUtil.parseVmHWMFromStatus;
 
 import static com.google.common.truth.Truth.assertThat;
 
@@ -80,45 +79,25 @@ public class ProcfsMemoryUtilTest {
             + "nonvoluntary_ctxt_switches:\t104\n";
 
     @Test
-    public void testParseVmHWMFromStatus_parsesCorrectValue() {
-        assertThat(parseVmHWMFromStatus(STATUS_CONTENTS)).isEqualTo(137668);
-    }
-
-    @Test
-    public void testParseVmHWMFromStatus_invalidValue() {
-        assertThat(parseVmHWMFromStatus("test\nVmHWM: x0x0x\ntest")).isEqualTo(0);
-    }
-
-    @Test
-    public void testParseVmHWMFromStatus_emptyContents() {
-        assertThat(parseVmHWMFromStatus("")).isEqualTo(0);
-    }
-
-    @Test
     public void testParseMemorySnapshotFromStatus_parsesCorrectValue() {
         MemorySnapshot snapshot = parseMemorySnapshotFromStatus(STATUS_CONTENTS);
+        assertThat(snapshot.uid).isEqualTo(10083);
+        assertThat(snapshot.rssHighWaterMarkInKilobytes).isEqualTo(137668);
         assertThat(snapshot.rssInKilobytes).isEqualTo(126776);
         assertThat(snapshot.anonRssInKilobytes).isEqualTo(37860);
         assertThat(snapshot.swapInKilobytes).isEqualTo(22);
-        assertThat(snapshot.isEmpty()).isFalse();
     }
 
     @Test
     public void testParseMemorySnapshotFromStatus_invalidValue() {
         MemorySnapshot snapshot =
                 parseMemorySnapshotFromStatus("test\nVmRSS:\tx0x0x\nVmSwap:\t1 kB\ntest");
-        assertThat(snapshot.rssInKilobytes).isEqualTo(0);
-        assertThat(snapshot.anonRssInKilobytes).isEqualTo(0);
-        assertThat(snapshot.swapInKilobytes).isEqualTo(1);
-        assertThat(snapshot.isEmpty()).isFalse();
+        assertThat(snapshot).isNull();
     }
 
     @Test
     public void testParseMemorySnapshotFromStatus_emptyContents() {
         MemorySnapshot snapshot = parseMemorySnapshotFromStatus("");
-        assertThat(snapshot.rssInKilobytes).isEqualTo(0);
-        assertThat(snapshot.anonRssInKilobytes).isEqualTo(0);
-        assertThat(snapshot.swapInKilobytes).isEqualTo(0);
-        assertThat(snapshot.isEmpty()).isTrue();
+        assertThat(snapshot).isNull();
     }
 }