diff options
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(); - } -} |