Improve reading proc stats

StatsCompanion can now do the process filtering on its own
(e.g. so that it can do prefix matching instead of exact names in future
CLs).

As part of this, change reading cmdline and status to the native reader which
has better performance.

10064 pull timing before: avg: 41ms max: 55ms
                  after:  avg: 35ms max: 41ms

Change-Id: I3fcdffb727b12a98c10e0712272cc3edf0b24208
Test: Manual, atest android.cts.statsd.atom.UidAtomTests

3 files changed, 64 insertions, 220 deletions

diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
index 518a29c2017a..d879273df3bc 100644
--- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
+++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
@@ -18,7 +18,6 @@ package com.android.server.stats;
 import static android.app.AppOpsManager.OP_FLAGS_ALL_TRUSTED;
 import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED;
 import static android.content.pm.PermissionInfo.PROTECTION_DANGEROUS;
-import static android.os.Process.getPidsForCommands;
 import static android.os.Process.getUidForPid;
 import static android.os.storage.VolumeInfo.TYPE_PRIVATE;
 import static android.os.storage.VolumeInfo.TYPE_PUBLIC;
@@ -27,6 +26,7 @@ import static com.android.internal.util.Preconditions.checkNotNull;
 import static com.android.server.am.MemoryStatUtil.readMemoryStatFromFilesystem;
 import static com.android.server.stats.IonMemoryUtil.readProcessSystemIonHeapSizesFromDebugfs;
 import static com.android.server.stats.IonMemoryUtil.readSystemIonHeapSizeFromDebugfs;
+import static com.android.server.stats.ProcfsMemoryUtil.forEachPid;
 import static com.android.server.stats.ProcfsMemoryUtil.readCmdlineFromProcfs;
 import static com.android.server.stats.ProcfsMemoryUtil.readMemorySnapshotFromProcfs;
 
@@ -144,6 +144,8 @@ import com.android.server.stats.ProcfsMemoryUtil.MemorySnapshot;
 import com.android.server.storage.DiskStatsFileLogger;
 import com.android.server.storage.DiskStatsLoggingService;
 
+import com.google.android.collect.Sets;
+
 import libcore.io.IoUtils;
 
 import org.json.JSONArray;
@@ -163,6 +165,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
@@ -216,7 +219,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
      * <p>Processes are matched by their cmdline in procfs. Example: cat /proc/pid/cmdline returns
      * /system/bin/statsd for the stats daemon.
      */
-    private static final String[] MEMORY_INTERESTING_NATIVE_PROCESSES = new String[]{
+    private static final Set<String> MEMORY_INTERESTING_NATIVE_PROCESSES = Sets.newHashSet(
             "/system/bin/statsd",  // Stats daemon.
             "/system/bin/surfaceflinger",
             "/system/bin/apexd",  // APEX daemon.
@@ -239,8 +242,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
             "/system/bin/traced_probes",  // Perfetto.
             "webview_zygote",
             "zygote",
-            "zygote64",
-    };
+            "zygote64");
     /**
      * Lowest available uid for apps.
      *
@@ -1220,27 +1222,28 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
             e.writeInt(snapshot.rssHighWaterMarkInKilobytes);
             pulledData.add(e);
         }
-        int[] pids = getPidsForCommands(MEMORY_INTERESTING_NATIVE_PROCESSES);
-        for (int pid : pids) {
-            final String processName = readCmdlineFromProcfs(pid);
+        forEachPid((pid, cmdLine) -> {
+            if (!MEMORY_INTERESTING_NATIVE_PROCESSES.contains(cmdLine)) {
+                return;
+            }
             final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid);
             if (snapshot == null) {
-                continue;
+                return;
             }
             // 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;
+                return;
             }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
             e.writeInt(snapshot.uid);
-            e.writeString(processName);
+            e.writeString(cmdLine);
             // RSS high-water mark in bytes.
             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.
         SystemProperties.set("sys.rss_hwm_reset.on", "1");
     }
@@ -1267,22 +1270,23 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
             e.writeInt(snapshot.anonRssInKilobytes + snapshot.swapInKilobytes);
             pulledData.add(e);
         }
-        int[] pids = getPidsForCommands(MEMORY_INTERESTING_NATIVE_PROCESSES);
-        for (int pid : pids) {
-            final String processName = readCmdlineFromProcfs(pid);
+        forEachPid((pid, cmdLine) -> {
+            if (!MEMORY_INTERESTING_NATIVE_PROCESSES.contains(cmdLine)) {
+                return;
+            }
             final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid);
             if (snapshot == null) {
-                continue;
+                return;
             }
             // 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;
+                return;
             }
             StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos);
             e.writeInt(snapshot.uid);
-            e.writeString(processName);
+            e.writeString(cmdLine);
             e.writeInt(pid);
             e.writeInt(-1001);  // Placeholder for native processes, OOM_SCORE_ADJ_MIN - 1.
             e.writeInt(snapshot.rssInKilobytes);
@@ -1290,7 +1294,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
             e.writeInt(snapshot.swapInKilobytes);
             e.writeInt(snapshot.anonRssInKilobytes + snapshot.swapInKilobytes);
             pulledData.add(e);
-        }
+        });
     }
 
     private static boolean isAppUid(int uid) {
diff --git a/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java b/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
index 8431ae4aeb98..c1eacce1f304 100644
--- a/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
+++ b/services/core/java/com/android/server/stats/ProcfsMemoryUtil.java
@@ -15,28 +15,22 @@
  */
 package com.android.server.stats;
 
-import android.annotation.Nullable;
-import android.os.FileUtils;
-import android.util.Slog;
+import static android.os.Process.PROC_OUT_STRING;
 
-import com.android.internal.annotations.VisibleForTesting;
+import android.annotation.Nullable;
+import android.os.Process;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import java.util.function.BiConsumer;
 
 final class ProcfsMemoryUtil {
-    private static final String TAG = "ProcfsMemoryUtil";
-
-    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 static final int[] CMDLINE_OUT = new int[] { PROC_OUT_STRING };
+    private static final String[] STATUS_KEYS = new String[] {
+            "Uid:",
+            "VmHWM:",
+            "VmRSS:",
+            "RssAnon:",
+            "VmSwap:"
+    };
 
     private ProcfsMemoryUtil() {}
 
@@ -46,30 +40,21 @@ final class ProcfsMemoryUtil {
      */
     @Nullable
     static MemorySnapshot readMemorySnapshotFromProcfs(int pid) {
-        return parseMemorySnapshotFromStatus(readFile("/proc/" + pid + "/status"));
-    }
-
-    @VisibleForTesting
-    @Nullable
-    static MemorySnapshot parseMemorySnapshotFromStatus(String contents) {
-        if (contents.isEmpty()) {
+        long[] output = new long[STATUS_KEYS.length];
+        output[0] = -1;
+        Process.readProcLines("/proc/" + pid + "/status", STATUS_KEYS, output);
+        if (output[0] == -1 || (output[3] == 0 && output[4] == 0)) {
+            // Could not open file or anon rss / swap are 0 indicating the process is in a zombie
+            // state.
             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;
+        final MemorySnapshot snapshot = new MemorySnapshot();
+        snapshot.uid = (int) output[0];
+        snapshot.rssHighWaterMarkInKilobytes = (int) output[1];
+        snapshot.rssInKilobytes = (int) output[2];
+        snapshot.anonRssInKilobytes = (int) output[3];
+        snapshot.swapInKilobytes = (int) output[4];
+        return snapshot;
     }
 
     /**
@@ -78,30 +63,26 @@ final class ProcfsMemoryUtil {
      * Returns content of /proc/pid/cmdline (e.g. /system/bin/statsd) or an empty string
      * if the file is not available.
      */
-    public static String readCmdlineFromProcfs(int pid) {
-        return parseCmdline(readFile("/proc/" + pid + "/cmdline"));
-    }
-
-    /**
-     * Parses cmdline out of the contents of the /proc/pid/cmdline file in procfs.
-     *
-     * Parsing is required to strip anything after the first null byte.
-     */
-    @VisibleForTesting
-    static String parseCmdline(String contents) {
-        int firstNullByte = contents.indexOf("\0");
-        if (firstNullByte == -1) {
-            return contents;
+    static String readCmdlineFromProcfs(int pid) {
+        String[] cmdline = new String[1];
+        if (!Process.readProcFile("/proc/" + pid + "/cmdline", CMDLINE_OUT, cmdline, null, null)) {
+            return "";
         }
-        return contents.substring(0, firstNullByte);
+        return cmdline[0];
     }
 
-    private static String readFile(String path) {
-        try {
-            final File file = new File(path);
-            return FileUtils.readTextFile(file, 0 /* max */, null /* ellipsis */);
-        } catch (IOException e) {
-            return "";
+    static void forEachPid(BiConsumer<Integer, String> func) {
+        int[] pids = new int[1024];
+        pids = Process.getPids("/proc", pids);
+        for (int pid : pids) {
+            if (pid < 0) {
+                return;
+            }
+            String cmdline = readCmdlineFromProcfs(pid);
+            if (cmdline.isEmpty()) {
+                continue;
+            }
+            func.accept(pid, cmdline);
         }
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java b/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java
deleted file mode 100644
index d1ac19c540a4..000000000000
--- a/services/tests/servicestests/src/com/android/server/stats/ProcfsMemoryUtilTest.java
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- * Copyright (C) 2019 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.android.server.stats;
-
-import static com.android.server.stats.ProcfsMemoryUtil.parseCmdline;
-import static com.android.server.stats.ProcfsMemoryUtil.parseMemorySnapshotFromStatus;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import androidx.test.filters.SmallTest;
-
-import com.android.server.stats.ProcfsMemoryUtil.MemorySnapshot;
-
-import org.junit.Test;
-
-import java.io.ByteArrayOutputStream;
-
-/**
- * Build/Install/Run:
- *  atest FrameworksServicesTests:ProcfsMemoryUtilTest
- */
-@SmallTest
-public class ProcfsMemoryUtilTest {
-    private static final String STATUS_CONTENTS = "Name:\tandroid.youtube\n"
-            + "State:\tS (sleeping)\n"
-            + "Tgid:\t12088\n"
-            + "Pid:\t12088\n"
-            + "PPid:\t723\n"
-            + "TracerPid:\t0\n"
-            + "Uid:\t10083\t10083\t10083\t10083\n"
-            + "Gid:\t10083\t10083\t10083\t10083\n"
-            + "Ngid:\t0\n"
-            + "FDSize:\t128\n"
-            + "Groups:\t3003 9997 20083 50083 \n"
-            + "VmPeak:\t 4546844 kB\n"
-            + "VmSize:\t 4542636 kB\n"
-            + "VmLck:\t       0 kB\n"
-            + "VmPin:\t       0 kB\n"
-            + "VmHWM:\t  137668 kB\n" // RSS high-water mark
-            + "VmRSS:\t  126776 kB\n" // RSS
-            + "RssAnon:\t   37860 kB\n"
-            + "RssFile:\t   88764 kB\n"
-            + "RssShmem:\t     152 kB\n"
-            + "VmData:\t 4125112 kB\n"
-            + "VmStk:\t    8192 kB\n"
-            + "VmExe:\t      24 kB\n"
-            + "VmLib:\t  102432 kB\n"
-            + "VmPTE:\t    1300 kB\n"
-            + "VmPMD:\t      36 kB\n"
-            + "VmSwap:\t      22 kB\n" // Swap
-            + "Threads:\t95\n"
-            + "SigQ:\t0/13641\n"
-            + "SigPnd:\t0000000000000000\n"
-            + "ShdPnd:\t0000000000000000\n"
-            + "SigBlk:\t0000000000001204\n"
-            + "SigIgn:\t0000000000000001\n"
-            + "SigCgt:\t00000006400084f8\n"
-            + "CapInh:\t0000000000000000\n"
-            + "CapPrm:\t0000000000000000\n"
-            + "CapEff:\t0000000000000000\n"
-            + "CapBnd:\t0000000000000000\n"
-            + "CapAmb:\t0000000000000000\n"
-            + "Seccomp:\t2\n"
-            + "Cpus_allowed:\tff\n"
-            + "Cpus_allowed_list:\t0-7\n"
-            + "Mems_allowed:\t1\n"
-            + "Mems_allowed_list:\t0\n"
-            + "voluntary_ctxt_switches:\t903\n"
-            + "nonvoluntary_ctxt_switches:\t104\n";
-
-    @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);
-    }
-
-    @Test
-    public void testParseMemorySnapshotFromStatus_invalidValue() {
-        MemorySnapshot snapshot =
-                parseMemorySnapshotFromStatus("test\nVmRSS:\tx0x0x\nVmSwap:\t1 kB\ntest");
-        assertThat(snapshot).isNull();
-    }
-
-    @Test
-    public void testParseMemorySnapshotFromStatus_emptyContents() {
-        MemorySnapshot snapshot = parseMemorySnapshotFromStatus("");
-        assertThat(snapshot).isNull();
-    }
-
-    @Test
-    public void testParseCmdline_invalidValue() {
-        byte[] nothing = new byte[] {0x00, 0x74, 0x65, 0x73, 0x74}; // \0test
-
-        assertThat(parseCmdline(bytesToString(nothing))).isEmpty();
-    }
-
-    @Test
-    public void testParseCmdline_correctValue_noNullBytes() {
-        assertThat(parseCmdline("com.google.app")).isEqualTo("com.google.app");
-    }
-
-    @Test
-    public void testParseCmdline_correctValue_withNullBytes() {
-        byte[] trailing = new byte[] {0x74, 0x65, 0x73, 0x74, 0x00, 0x00, 0x00}; // test\0\0\0
-
-        assertThat(parseCmdline(bytesToString(trailing))).isEqualTo("test");
-
-        // test\0\0test
-        byte[] inside = new byte[] {0x74, 0x65, 0x73, 0x74, 0x00, 0x00, 0x74, 0x65, 0x73, 0x74};
-
-        assertThat(parseCmdline(bytesToString(trailing))).isEqualTo("test");
-    }
-
-    @Test
-    public void testParseCmdline_emptyContents() {
-        assertThat(parseCmdline("")).isEmpty();
-    }
-
-    private static String bytesToString(byte[] bytes) {
-        ByteArrayOutputStream output = new ByteArrayOutputStream();
-        output.write(bytes, 0, bytes.length);
-        return output.toString();
-    }
-}