From 754eb07a78c77353bc92837febb49bfe53ce6abf Mon Sep 17 00:00:00 2001 From: Ioannis Ilkos Date: Mon, 27 Jan 2020 16:42:24 +0000 Subject: [PATCH] Remove process whitelist from memory measurements The process whitelist can still be applied on the statsd configuration layer. By removing the whitelist, we eliminate the memory measurement blindspot. 10064 latency on 100 iterations: avg: 57ms max: 357ms Bug: 148365709 Test: Manual Change-Id: I6e826896b4c874bbb10ea2be65034d39dfff50a3 --- .../server/stats/pull/ProcfsMemoryUtil.java | 15 ++-- .../stats/pull/StatsPullAtomService.java | 87 +++++-------------- 2 files changed, 34 insertions(+), 68 deletions(-) diff --git a/services/core/java/com/android/server/stats/pull/ProcfsMemoryUtil.java b/services/core/java/com/android/server/stats/pull/ProcfsMemoryUtil.java index 638dfd23c27fc..67677c6cf17e2 100644 --- a/services/core/java/com/android/server/stats/pull/ProcfsMemoryUtil.java +++ b/services/core/java/com/android/server/stats/pull/ProcfsMemoryUtil.java @@ -19,8 +19,7 @@ import static android.os.Process.PROC_OUT_STRING; import android.annotation.Nullable; import android.os.Process; - -import java.util.function.BiConsumer; +import android.util.SparseArray; public final class ProcfsMemoryUtil { private static final int[] CMDLINE_OUT = new int[] { PROC_OUT_STRING }; @@ -71,19 +70,25 @@ public final class ProcfsMemoryUtil { return cmdline[0]; } - public static void forEachPid(BiConsumer func) { + /** + * Scans all /proc/pid/cmdline entries and returns a mapping between pid and cmdline. + */ + public static SparseArray getProcessCmdlines() { int[] pids = new int[1024]; pids = Process.getPids("/proc", pids); + + SparseArray cmdlines = new SparseArray<>(pids.length); for (int pid : pids) { if (pid < 0) { - return; + break; } String cmdline = readCmdlineFromProcfs(pid); if (cmdline.isEmpty()) { continue; } - func.accept(pid, cmdline); + cmdlines.append(pid, cmdline); } + return cmdlines; } public static final class MemorySnapshot { diff --git a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java index 92e5a01fc7962..b12f83195961c 100644 --- a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java +++ b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java @@ -27,7 +27,7 @@ import static android.os.storage.VolumeInfo.TYPE_PUBLIC; import static com.android.server.am.MemoryStatUtil.readMemoryStatFromFilesystem; import static com.android.server.stats.pull.IonMemoryUtil.readProcessSystemIonHeapSizesFromDebugfs; import static com.android.server.stats.pull.IonMemoryUtil.readSystemIonHeapSizeFromDebugfs; -import static com.android.server.stats.pull.ProcfsMemoryUtil.forEachPid; +import static com.android.server.stats.pull.ProcfsMemoryUtil.getProcessCmdlines; import static com.android.server.stats.pull.ProcfsMemoryUtil.readCmdlineFromProcfs; import static com.android.server.stats.pull.ProcfsMemoryUtil.readMemorySnapshotFromProcfs; @@ -93,6 +93,7 @@ import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; import android.util.Slog; +import android.util.SparseArray; import android.util.StatsEvent; import android.util.StatsLog; import android.util.proto.ProtoOutputStream; @@ -130,8 +131,6 @@ import com.android.server.stats.pull.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; @@ -1147,37 +1146,6 @@ public class StatsPullAtomService extends SystemService { return StatsManager.PULL_SUCCESS; } - /** - * Which native processes to snapshot memory for. - * - *

Processes are matched by their cmdline in procfs. Example: cat /proc/pid/cmdline returns - * /system/bin/statsd for the stats daemon. - */ - private static final Set MEMORY_INTERESTING_NATIVE_PROCESSES = Sets.newHashSet( - "/system/bin/statsd", // Stats daemon. - "/system/bin/surfaceflinger", - "/system/bin/apexd", // APEX daemon. - "/system/bin/audioserver", - "/system/bin/cameraserver", - "/system/bin/drmserver", - "/system/bin/healthd", - "/system/bin/incidentd", - "/system/bin/installd", - "/system/bin/lmkd", // Low memory killer daemon. - "/system/bin/logd", - "media.codec", - "media.extractor", - "media.metrics", - "/system/bin/mediadrmserver", - "/system/bin/mediaserver", - "/system/bin/performanced", - "/system/bin/tombstoned", - "/system/bin/traced", // Perfetto. - "/system/bin/traced_probes", // Perfetto. - "webview_zygote", - "zygote", - "zygote64"); - /** * Lowest available uid for apps. * @@ -1219,30 +1187,25 @@ public class StatsPullAtomService extends SystemService { .build(); pulledData.add(e); } - forEachPid((pid, cmdLine) -> { - if (!MEMORY_INTERESTING_NATIVE_PROCESSES.contains(cmdLine)) { - return; - } - final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid); - if (snapshot == null) { - 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)) { - return; + // Complement the data with native system processes + SparseArray processCmdlines = getProcessCmdlines(); + managedProcessList.forEach(managedProcess -> processCmdlines.delete(managedProcess.pid)); + int size = processCmdlines.size(); + for (int i = 0; i < size; ++i) { + final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(processCmdlines.keyAt(i)); + if (snapshot == null || isAppUid(snapshot.uid)) { + continue; } StatsEvent e = StatsEvent.newBuilder() .setAtomId(atomTag) .writeInt(snapshot.uid) - .writeString(cmdLine) + .writeString(processCmdlines.valueAt(i)) // RSS high-water mark in bytes. .writeLong(snapshot.rssHighWaterMarkInKilobytes * 1024L) .writeInt(snapshot.rssHighWaterMarkInKilobytes) .build(); pulledData.add(e); - }); + } // Invoke rss_hwm_reset binary to reset RSS HWM counters for all processes. SystemProperties.set("sys.rss_hwm_reset.on", "1"); return StatsManager.PULL_SUCCESS; @@ -1280,24 +1243,22 @@ public class StatsPullAtomService extends SystemService { .build(); pulledData.add(e); } - forEachPid((pid, cmdLine) -> { - if (!MEMORY_INTERESTING_NATIVE_PROCESSES.contains(cmdLine)) { - return; - } + // Complement the data with native system processes. Given these measurements can be taken + // in response to LMKs happening, we want to first collect the managed app stats (to + // maximize the probability that a heavyweight process will be sampled before it dies). + SparseArray processCmdlines = getProcessCmdlines(); + managedProcessList.forEach(managedProcess -> processCmdlines.delete(managedProcess.pid)); + int size = processCmdlines.size(); + for (int i = 0; i < size; ++i) { + int pid = processCmdlines.keyAt(i); final MemorySnapshot snapshot = readMemorySnapshotFromProcfs(pid); - if (snapshot == null) { - 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)) { - return; + if (snapshot == null || isAppUid(snapshot.uid)) { + continue; } StatsEvent e = StatsEvent.newBuilder() .setAtomId(atomTag) .writeInt(snapshot.uid) - .writeString(cmdLine) + .writeString(processCmdlines.valueAt(i)) .writeInt(pid) .writeInt(-1001) // Placeholder for native processes, OOM_SCORE_ADJ_MIN - 1. .writeInt(snapshot.rssInKilobytes) @@ -1306,7 +1267,7 @@ public class StatsPullAtomService extends SystemService { .writeInt(snapshot.anonRssInKilobytes + snapshot.swapInKilobytes) .build(); pulledData.add(e); - }); + } return StatsManager.PULL_SUCCESS; }