From 65a9b7122ada9db7a95cdfe31fa97b328bd04a06 Mon Sep 17 00:00:00 2001 From: Rafal Slawik Date: Tue, 16 Apr 2019 13:16:15 +0100 Subject: [PATCH] Monitor swap Pull value of VmSwap from /proc/PID/status when capturing ProcessMemoryState atom. Before change: (average pull time nanos) 43355896 (max pull time nanos) 75649278 After change: (average pull time nanos) 86307073 (max pull time nanos) 151681474 Delta: 2x increase Pulling frequency is controled via westworld and we trade-off more expensive reads for more actionable data (helping detect memory leaks). Bug: 130624561 Test: atest MemoryStatUtilTest Test: benchmark pulling ProcessMemoryState atom Test: manually take a statsd report Change-Id: I1d90563b70b5253b3d31ddab4810db870620c4d4 --- cmds/statsd/src/atoms.proto | 6 ++++- cmds/statsd/src/external/StatsPullerManager.cpp | 2 +- .../java/com/android/server/am/MemoryStatUtil.java | 27 ++++++++++++++++------ .../server/stats/StatsCompanionService.java | 1 + .../com/android/server/am/MemoryStatUtilTest.java | 27 +++++++++++++--------- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/cmds/statsd/src/atoms.proto b/cmds/statsd/src/atoms.proto index 55d3fba8845d..08572f3b97f1 100644 --- a/cmds/statsd/src/atoms.proto +++ b/cmds/statsd/src/atoms.proto @@ -3791,7 +3791,7 @@ message ProcessMemoryState { // SWAP // Value is read from memory.stat, field total_swap if per-app memory - // cgroups are enabled. Otherwise, 0. + // cgroups are enabled. Otherwise, VmSwap from /proc/PID/status. optional int64 swap_in_bytes = 8; // Deprecated: use ProcessMemoryHighWaterMark atom instead. Always 0. @@ -3831,6 +3831,10 @@ message NativeProcessMemoryState { // Elapsed real time when the process started. // Value is read from /proc/PID/stat, field 22. optional int64 start_time_nanos = 7; + + // SWAP + // Value read from /proc/PID/status, field VmSwap. + optional int64 swap_in_bytes = 8; } /* diff --git a/cmds/statsd/src/external/StatsPullerManager.cpp b/cmds/statsd/src/external/StatsPullerManager.cpp index 51839c4077bc..de43b832c62a 100644 --- a/cmds/statsd/src/external/StatsPullerManager.cpp +++ b/cmds/statsd/src/external/StatsPullerManager.cpp @@ -146,7 +146,7 @@ std::map StatsPullerManager::kAllPullAtomInfo = { .puller = new StatsCompanionServicePuller(android::util::PROCESS_MEMORY_STATE)}}, // native_process_memory_state {android::util::NATIVE_PROCESS_MEMORY_STATE, - {.additiveFields = {3, 4, 5, 6}, + {.additiveFields = {3, 4, 5, 6, 8}, .puller = new StatsCompanionServicePuller(android::util::NATIVE_PROCESS_MEMORY_STATE)}}, // process_memory_high_water_mark {android::util::PROCESS_MEMORY_HIGH_WATER_MARK, diff --git a/services/core/java/com/android/server/am/MemoryStatUtil.java b/services/core/java/com/android/server/am/MemoryStatUtil.java index 9cda89a17766..78d2634e9236 100644 --- a/services/core/java/com/android/server/am/MemoryStatUtil.java +++ b/services/core/java/com/android/server/am/MemoryStatUtil.java @@ -40,7 +40,6 @@ import java.util.regex.Pattern; */ public final class MemoryStatUtil { static final int BYTES_IN_KILOBYTE = 1024; - static final int PAGE_SIZE = 4096; static final long JIFFY_NANOS = 1_000_000_000 / Os.sysconf(OsConstants._SC_CLK_TCK); private static final String TAG = TAG_WITH_CLASS_NAME ? "MemoryStatUtil" : TAG_AM; @@ -65,15 +64,20 @@ public final class MemoryStatUtil { private static final Pattern RSS_IN_BYTES = Pattern.compile("total_rss (\\d+)"); private static final Pattern CACHE_IN_BYTES = Pattern.compile("total_cache (\\d+)"); private static final Pattern SWAP_IN_BYTES = Pattern.compile("total_swap (\\d+)"); - private static final Pattern RSS_HIGH_WATERMARK_IN_BYTES = + + private static final Pattern RSS_HIGH_WATERMARK_IN_KILOBYTES = Pattern.compile("VmHWM:\\s*(\\d+)\\s*kB"); + private static final Pattern PROCFS_RSS_IN_KILOBYTES = + Pattern.compile("VmRSS:\\s*(\\d+)\\s*kB"); + private static final Pattern PROCFS_SWAP_IN_KILOBYTES = + Pattern.compile("VmSwap:\\s*(\\d+)\\s*kB"); + private static final Pattern ION_HEAP_SIZE_IN_BYTES = Pattern.compile("\n\\s*total\\s*(\\d+)\\s*\n"); private static final int PGFAULT_INDEX = 9; private static final int PGMAJFAULT_INDEX = 11; private static final int START_TIME_INDEX = 21; - private static final int RSS_IN_PAGES_INDEX = 23; private MemoryStatUtil() {} @@ -107,7 +111,8 @@ public final class MemoryStatUtil { @Nullable public static MemoryStat readMemoryStatFromProcfs(int pid) { final String statPath = String.format(Locale.US, PROC_STAT_FILE_FMT, pid); - return parseMemoryStatFromProcfs(readFileContents(statPath)); + final String statusPath = String.format(Locale.US, PROC_STATUS_FILE_FMT, pid); + return parseMemoryStatFromProcfs(readFileContents(statPath), readFileContents(statusPath)); } /** @@ -179,10 +184,14 @@ public final class MemoryStatUtil { */ @VisibleForTesting @Nullable - static MemoryStat parseMemoryStatFromProcfs(String procStatContents) { + static MemoryStat parseMemoryStatFromProcfs( + String procStatContents, String procStatusContents) { if (procStatContents == null || procStatContents.isEmpty()) { return null; } + if (procStatusContents == null || procStatusContents.isEmpty()) { + return null; + } final String[] splits = procStatContents.split(" "); if (splits.length < 24) { @@ -193,7 +202,10 @@ public final class MemoryStatUtil { final MemoryStat memoryStat = new MemoryStat(); memoryStat.pgfault = Long.parseLong(splits[PGFAULT_INDEX]); memoryStat.pgmajfault = Long.parseLong(splits[PGMAJFAULT_INDEX]); - memoryStat.rssInBytes = Long.parseLong(splits[RSS_IN_PAGES_INDEX]) * PAGE_SIZE; + memoryStat.rssInBytes = + tryParseLong(PROCFS_RSS_IN_KILOBYTES, procStatusContents) * BYTES_IN_KILOBYTE; + memoryStat.swapInBytes = + tryParseLong(PROCFS_SWAP_IN_KILOBYTES, procStatusContents) * BYTES_IN_KILOBYTE; memoryStat.startTimeNanos = Long.parseLong(splits[START_TIME_INDEX]) * JIFFY_NANOS; return memoryStat; } catch (NumberFormatException e) { @@ -212,7 +224,8 @@ public final class MemoryStatUtil { return 0; } // Convert value read from /proc/pid/status from kilobytes to bytes. - return tryParseLong(RSS_HIGH_WATERMARK_IN_BYTES, procStatusContents) * BYTES_IN_KILOBYTE; + return tryParseLong(RSS_HIGH_WATERMARK_IN_KILOBYTES, procStatusContents) + * BYTES_IN_KILOBYTE; } diff --git a/services/core/java/com/android/server/stats/StatsCompanionService.java b/services/core/java/com/android/server/stats/StatsCompanionService.java index 44ed070fda23..5efe72cbce14 100644 --- a/services/core/java/com/android/server/stats/StatsCompanionService.java +++ b/services/core/java/com/android/server/stats/StatsCompanionService.java @@ -1158,6 +1158,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { e.writeLong(memoryStat.rssInBytes); e.writeLong(0); // unused e.writeLong(memoryStat.startTimeNanos); + e.writeLong(memoryStat.swapInBytes); pulledData.add(e); } } diff --git a/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java b/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java index 5fb762eadb49..174571de1e4c 100644 --- a/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java +++ b/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java @@ -19,7 +19,6 @@ package com.android.server.am; import static com.android.server.am.MemoryStatUtil.BYTES_IN_KILOBYTE; import static com.android.server.am.MemoryStatUtil.JIFFY_NANOS; import static com.android.server.am.MemoryStatUtil.MemoryStat; -import static com.android.server.am.MemoryStatUtil.PAGE_SIZE; import static com.android.server.am.MemoryStatUtil.parseCmdlineFromProcfs; import static com.android.server.am.MemoryStatUtil.parseIonHeapSizeFromDebugfs; import static com.android.server.am.MemoryStatUtil.parseMemoryStatFromMemcg; @@ -102,7 +101,7 @@ public class MemoryStatUtilTest { "0", "2222", // this in start time (in ticks per second) "1257177088", - "3", // this is RSS (number of pages) + "3", "4294967295", "2936971264", "2936991289", @@ -147,8 +146,8 @@ public class MemoryStatUtilTest { + "VmSize:\t 4542636 kB\n" + "VmLck:\t 0 kB\n" + "VmPin:\t 0 kB\n" - + "VmHWM:\t 137668 kB\n" // RSS high watermark - + "VmRSS:\t 126776 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" @@ -158,7 +157,7 @@ public class MemoryStatUtilTest { + "VmLib:\t 102432 kB\n" + "VmPTE:\t 1300 kB\n" + "VmPMD:\t 36 kB\n" - + "VmSwap:\t 0 kB\n" + + "VmSwap:\t 22 kB\n" // Swap + "Threads:\t95\n" + "SigQ:\t0/13641\n" + "SigPnd:\t0000000000000000\n" @@ -227,28 +226,34 @@ public class MemoryStatUtilTest { @Test public void testParseMemoryStatFromProcfs_parsesCorrectValues() { - MemoryStat stat = parseMemoryStatFromProcfs(PROC_STAT_CONTENTS); + MemoryStat stat = parseMemoryStatFromProcfs(PROC_STAT_CONTENTS, PROC_STATUS_CONTENTS); assertEquals(1, stat.pgfault); assertEquals(2, stat.pgmajfault); - assertEquals(3 * PAGE_SIZE, stat.rssInBytes); + assertEquals(126776 * BYTES_IN_KILOBYTE, stat.rssInBytes); assertEquals(0, stat.cacheInBytes); - assertEquals(0, stat.swapInBytes); + assertEquals(22 * BYTES_IN_KILOBYTE, stat.swapInBytes); assertEquals(2222 * JIFFY_NANOS, stat.startTimeNanos); } @Test public void testParseMemoryStatFromProcfs_emptyContents() { - MemoryStat stat = parseMemoryStatFromProcfs(""); + MemoryStat stat = parseMemoryStatFromProcfs("", PROC_STATUS_CONTENTS); assertNull(stat); - stat = parseMemoryStatFromProcfs(null); + stat = parseMemoryStatFromProcfs(null, PROC_STATUS_CONTENTS); + assertNull(stat); + + stat = parseMemoryStatFromProcfs(PROC_STAT_CONTENTS, ""); + assertNull(stat); + + stat = parseMemoryStatFromProcfs(PROC_STAT_CONTENTS, null); assertNull(stat); } @Test public void testParseMemoryStatFromProcfs_invalidValue() { String contents = String.join(" ", Collections.nCopies(24, "memory")); - assertNull(parseMemoryStatFromProcfs(contents)); + assertNull(parseMemoryStatFromProcfs(contents, PROC_STATUS_CONTENTS)); } @Test -- 2.11.0