From bfcd920858c132da7348494e5fe5e9155e168e64 Mon Sep 17 00:00:00 2001 From: Rajeev Kumar Date: Fri, 2 Mar 2018 22:42:13 +0000 Subject: [PATCH] With build failure fix: Add stats logging in appDiedLocked. This reverts commit bcf577a436faffa414d30b3a9c0b0209006b71f7. Change-Id: I6d4a2aec4faf8ad4722a1c94c48858aebf408674 Test: runtest frameworks-services Bug: 73660232 --- .../server/am/ActivityManagerService.java | 11 ++- .../server/am/ActivityMetricsLogger.java | 4 +- .../com/android/server/am/MemoryStatUtil.java | 95 ++++++++++++++++--- .../android/server/am/MemoryStatUtilTest.java | 87 ++++++++--------- 4 files changed, 134 insertions(+), 63 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 0de4f860d6424..29d4e922e28dd 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -192,7 +192,8 @@ import static com.android.server.am.ActivityStackSupervisor.MATCH_TASK_IN_STACKS import static com.android.server.am.ActivityStackSupervisor.ON_TOP; import static com.android.server.am.ActivityStackSupervisor.PRESERVE_WINDOWS; import static com.android.server.am.ActivityStackSupervisor.REMOVE_FROM_RECENTS; -import static com.android.server.am.MemoryStatUtil.readMemoryStatFromMemcg; +import static com.android.server.am.MemoryStatUtil.readMemoryStatFromFilesystem; +import static com.android.server.am.MemoryStatUtil.hasMemcg; import static com.android.server.am.TaskRecord.INVALID_TASK_ID; import static com.android.server.am.TaskRecord.LOCK_TASK_AUTH_DONT_LOCK; import static com.android.server.am.TaskRecord.REPARENT_KEEP_STACK_AT_FRONT; @@ -5925,6 +5926,12 @@ public class ActivityManagerService extends IActivityManager.Stub Slog.d(TAG_PROCESSES, "Received spurious death notification for thread " + thread.asBinder()); } + + // On the device which doesn't have Cgroup, log LmkStateChanged which is used as a signal + // for pulling memory stats of other running processes when this process died. + if (!hasMemcg()) { + StatsLog.write(StatsLog.APP_DIED, SystemClock.elapsedRealtime()); + } } /** @@ -26155,7 +26162,7 @@ public class ActivityManagerService extends IActivityManager.Stub final ProcessRecord r = mPidsSelfLocked.valueAt(i); final int pid = r.pid; final int uid = r.uid; - final MemoryStat memoryStat = readMemoryStatFromMemcg(uid, pid); + final MemoryStat memoryStat = readMemoryStatFromFilesystem(uid, pid); if (memoryStat == null) { continue; } diff --git a/services/core/java/com/android/server/am/ActivityMetricsLogger.java b/services/core/java/com/android/server/am/ActivityMetricsLogger.java index 5d5ed550a00c5..352b75704d6e3 100644 --- a/services/core/java/com/android/server/am/ActivityMetricsLogger.java +++ b/services/core/java/com/android/server/am/ActivityMetricsLogger.java @@ -34,7 +34,7 @@ import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_METRICS; import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM; import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME; import static com.android.server.am.MemoryStatUtil.MemoryStat; -import static com.android.server.am.MemoryStatUtil.readMemoryStatFromMemcg; +import static com.android.server.am.MemoryStatUtil.readMemoryStatFromFilesystem; import android.content.Context; import android.content.pm.ApplicationInfo; @@ -635,7 +635,7 @@ class ActivityMetricsLogger { final int pid = info.processRecord.pid; final int uid = info.applicationInfo.uid; - final MemoryStat memoryStat = readMemoryStatFromMemcg(uid, pid); + final MemoryStat memoryStat = readMemoryStatFromFilesystem(uid, pid); if (memoryStat == null) { if (DEBUG_METRICS) Slog.i(TAG, "logAppStartMemoryStateCapture memoryStat null"); return; diff --git a/services/core/java/com/android/server/am/MemoryStatUtil.java b/services/core/java/com/android/server/am/MemoryStatUtil.java index a2a84ec43ed2e..da36bd12781b5 100644 --- a/services/core/java/com/android/server/am/MemoryStatUtil.java +++ b/services/core/java/com/android/server/am/MemoryStatUtil.java @@ -38,8 +38,12 @@ import java.util.regex.Pattern; final class MemoryStatUtil { private static final String TAG = TAG_WITH_CLASS_NAME ? "MemoryStatUtil" : TAG_AM; + /** Path to check if device has memcg */ + private static final String MEMCG_TEST_PATH = "/dev/memcg/apps/memory.stat"; /** Path to memory stat file for logging app start memory state */ private static final String MEMORY_STAT_FILE_FMT = "/dev/memcg/apps/uid_%d/pid_%d/memory.stat"; + /** Path to procfs stat file for logging app start memory state */ + private static final String PROC_STAT_FILE_FMT = "/proc/%d/stat"; private static final Pattern PGFAULT = Pattern.compile("total_pgfault (\\d+)"); private static final Pattern PGMAJFAULT = Pattern.compile("total_pgmajfault (\\d+)"); @@ -47,24 +51,57 @@ final class MemoryStatUtil { 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 int PGFAULT_INDEX = 9; + private static final int PGMAJFAULT_INDEX = 11; + private static final int RSS_IN_BYTES_INDEX = 23; + + /** True if device has memcg */ + private static volatile Boolean sDeviceHasMemCg; + private MemoryStatUtil() {} + /** + * Reads memory stat for a process. + * + * Reads from memcg if available on device, else fallback to procfs. + * Returns null if no stats can be read. + */ + @Nullable + static MemoryStat readMemoryStatFromFilesystem(int uid, int pid) { + return hasMemcg() ? readMemoryStatFromMemcg(uid, pid) : readMemoryStatFromProcfs(pid); + } + /** * Reads memory.stat of a process from memcg. + * + * Returns null if file is not found in memcg or if file has unrecognized contents. */ @Nullable static MemoryStat readMemoryStatFromMemcg(int uid, int pid) { - final String memoryStatPath = String.format(Locale.US, MEMORY_STAT_FILE_FMT, uid, pid); - final File memoryStatFile = new File(memoryStatPath); - if (!memoryStatFile.exists()) { - if (DEBUG_METRICS) Slog.i(TAG, memoryStatPath + " not found"); + final String path = String.format(Locale.US, MEMORY_STAT_FILE_FMT, uid, pid); + return parseMemoryStatFromMemcg(readFileContents(path)); + } + + /** + * Reads memory stat of a process from procfs. + * + * Returns null if file is not found in procfs or if file has unrecognized contents. + */ + @Nullable + static MemoryStat readMemoryStatFromProcfs(int pid) { + final String path = String.format(Locale.US, PROC_STAT_FILE_FMT, pid); + return parseMemoryStatFromProcfs(readFileContents(path)); + } + + private static String readFileContents(String path) { + final File file = new File(path); + if (!file.exists()) { + if (DEBUG_METRICS) Slog.i(TAG, path + " not found"); return null; } try { - final String memoryStatContents = FileUtils.readTextFile( - memoryStatFile, 0 /* max */, null /* ellipsis */); - return parseMemoryStat(memoryStatContents); + return FileUtils.readTextFile(file, 0 /* max */, null /* ellipsis */); } catch (IOException e) { Slog.e(TAG, "Failed to read file:", e); return null; @@ -76,12 +113,12 @@ final class MemoryStatUtil { */ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) @Nullable - static MemoryStat parseMemoryStat(String memoryStatContents) { - MemoryStat memoryStat = new MemoryStat(); - if (memoryStatContents == null) { - return memoryStat; + static MemoryStat parseMemoryStatFromMemcg(String memoryStatContents) { + if (memoryStatContents == null || memoryStatContents.isEmpty()) { + return null; } + final MemoryStat memoryStat = new MemoryStat(); Matcher m; m = PGFAULT.matcher(memoryStatContents); memoryStat.pgfault = m.find() ? Long.valueOf(m.group(1)) : 0; @@ -96,6 +133,40 @@ final class MemoryStatUtil { return memoryStat; } + /** + * Parses relevant statistics out from the contents of a /proc/pid/stat file in procfs. + */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE) + @Nullable + static MemoryStat parseMemoryStatFromProcfs(String procStatContents) { + if (procStatContents == null || procStatContents.isEmpty()) { + return null; + } + + final String[] splits = procStatContents.split(" "); + if (splits.length < 24) { + return null; + } + + final MemoryStat memoryStat = new MemoryStat(); + memoryStat.pgfault = Long.valueOf(splits[PGFAULT_INDEX]); + memoryStat.pgmajfault = Long.valueOf(splits[PGMAJFAULT_INDEX]); + memoryStat.rssInBytes = Long.valueOf(splits[RSS_IN_BYTES_INDEX]); + return memoryStat; + } + + /** + * Checks if memcg is available on device. + * + * Touches the filesystem to do the check. + */ + static boolean hasMemcg() { + if (sDeviceHasMemCg == null) { + sDeviceHasMemCg = (new File(MEMCG_TEST_PATH)).exists(); + } + return sDeviceHasMemCg; + } + static final class MemoryStat { /** Number of page faults */ long pgfault; @@ -108,4 +179,4 @@ final class MemoryStatUtil { /** Number of bytes of swap usage */ long swapInBytes; } -} +} \ No newline at end of file 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 a7e38104f5be7..8005bc7aed8a8 100644 --- a/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java +++ b/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java @@ -16,10 +16,11 @@ package com.android.server.am; -import static com.android.server.am.MemoryStatUtil.parseMemoryStat; +import static com.android.server.am.MemoryStatUtil.parseMemoryStatFromMemcg; import static com.android.server.am.MemoryStatUtil.MemoryStat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; @@ -31,44 +32,44 @@ import org.junit.runner.RunWith; @SmallTest public class MemoryStatUtilTest { private String MEMORY_STAT_CONTENTS = String.join( - "\n", - "cache 96", // keep different from total_cache to catch reading wrong value - "rss 97", // keep different from total_rss to catch reading wrong value - "rss_huge 0", - "mapped_file 524288", - "writeback 0", - "swap 95", // keep different from total_rss to catch reading wrong value - "pgpgin 16717", - "pgpgout 5037", - "pgfault 99", // keep different from total_pgfault to catch reading wrong value - "pgmajfault 98", // keep different from total_pgmajfault to catch reading wrong value - "inactive_anon 503808", - "active_anon 46309376", - "inactive_file 876544", - "active_file 81920", - "unevictable 0", - "hierarchical_memory_limit 18446744073709551615", - "hierarchical_memsw_limit 18446744073709551615", - "total_cache 4", - "total_rss 3", - "total_rss_huge 0", - "total_mapped_file 524288", - "total_writeback 0", - "total_swap 5", - "total_pgpgin 16717", - "total_pgpgout 5037", - "total_pgfault 1", - "total_pgmajfault 2", - "total_inactive_anon 503808", - "total_active_anon 46309376", - "total_inactive_file 876544", - "total_active_file 81920", - "total_unevictable 0"); + "\n", + "cache 96", // keep different from total_cache to catch reading wrong value + "rss 97", // keep different from total_rss to catch reading wrong value + "rss_huge 0", + "mapped_file 524288", + "writeback 0", + "swap 95", // keep different from total_rss to catch reading wrong value + "pgpgin 16717", + "pgpgout 5037", + "pgfault 99", // keep different from total_pgfault to catch reading wrong value + "pgmajfault 98", // keep different from total_pgmajfault to catch reading wrong value + "inactive_anon 503808", + "active_anon 46309376", + "inactive_file 876544", + "active_file 81920", + "unevictable 0", + "hierarchical_memory_limit 18446744073709551615", + "hierarchical_memsw_limit 18446744073709551615", + "total_cache 4", + "total_rss 3", + "total_rss_huge 0", + "total_mapped_file 524288", + "total_writeback 0", + "total_swap 5", + "total_pgpgin 16717", + "total_pgpgout 5037", + "total_pgfault 1", + "total_pgmajfault 2", + "total_inactive_anon 503808", + "total_active_anon 46309376", + "total_inactive_file 876544", + "total_active_file 81920", + "total_unevictable 0"); @Test public void testParseMemoryStat_parsesCorrectValues() throws Exception { - MemoryStat stat = parseMemoryStat(MEMORY_STAT_CONTENTS); + MemoryStat stat = parseMemoryStatFromMemcg(MEMORY_STAT_CONTENTS); assertEquals(stat.pgfault, 1); assertEquals(stat.pgmajfault, 2); assertEquals(stat.rssInBytes, 3); @@ -78,18 +79,10 @@ public class MemoryStatUtilTest { @Test public void testParseMemoryStat_emptyMemoryStatContents() throws Exception { - MemoryStat stat = parseMemoryStat(""); - assertEquals(stat.pgfault, 0); - assertEquals(stat.pgmajfault, 0); - assertEquals(stat.rssInBytes, 0); - assertEquals(stat.cacheInBytes, 0); - assertEquals(stat.swapInBytes, 0); + MemoryStat stat = parseMemoryStatFromMemcg(""); + assertNull(stat); - stat = parseMemoryStat(null); - assertEquals(stat.pgfault, 0); - assertEquals(stat.pgmajfault, 0); - assertEquals(stat.rssInBytes, 0); - assertEquals(stat.cacheInBytes, 0); - assertEquals(stat.swapInBytes, 0); + stat = parseMemoryStatFromMemcg(null); + assertNull(stat); } }