From 436d04ed95a803fe1125ed1e10e6c83ffe8c7ec3 Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Mon, 8 May 2017 13:29:28 +0100 Subject: [PATCH] ActivityManagerService: Rework dumping of top CPU processes. Try and measure the CPU load as early as possible to get a more realistic picture. Also, reduce the sampling set for the CPU load to a 200ms timeslice. Pros: - Stack dumping for the first / native processes can take a while, which might mean that our CPU measurement happens several seconds after the ANR occurred. - CPU measurements might be skewed by the stack dumping itself. Cons: - We capture traces for the "important" processes ~500ms later. This will not make any difference to the class of ANRs caused by deadlocks etc. It might however mean that we get unhelpful stack traces for ANRs caused by starvation / livelock etc. since the main thread might have made progress in this additional time. Arguably, this is isn't a big drawback since ANR reports from such apps are likely to be noisy anyway. Bug: 32064548 Test: Manual: Compared two ProcessCpuTrackers that would fire at the same time, one with a 500ms timeslice and another with a 200ms timeslice. There was usually no difference in the top 5 processes in their working sets. When there was, it was just an ordering difference the vast majority of the time. Change-Id: Ic81802cb62828fdc02a01b229dc1d6545a465ecd --- .../server/am/ActivityManagerService.java | 81 ++++++++++--------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index b1bcbaab49404..1194a447d3a99 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -5399,7 +5399,35 @@ public class ActivityManagerService extends IActivityManager.Stub return null; } - dumpStackTraces(tracesPath, firstPids, processCpuTracker, lastPids, nativePids); + ArrayList extraPids = null; + + // Lastly, measure CPU usage. + if (processCpuTracker != null) { + processCpuTracker.init(); + try { + Thread.sleep(200); + } catch (InterruptedException ignored) { + } + + processCpuTracker.update(); + + // We'll take the stack crawls of just the top apps using CPU. + final int N = processCpuTracker.countWorkingStats(); + extraPids = new ArrayList<>(); + for (int i = 0; i < N && extraPids.size() < 5; i++) { + ProcessCpuTracker.Stats stats = processCpuTracker.getWorkingStats(i); + if (lastPids.indexOfKey(stats.pid) >= 0) { + if (DEBUG_ANR) Slog.d(TAG, "Collecting stacks for extra pid " + stats.pid); + + extraPids.add(stats.pid); + } else if (DEBUG_ANR) { + Slog.d(TAG, "Skipping next CPU consuming process, not a java proc: " + + stats.pid); + } + } + } + + dumpStackTraces(tracesPath, firstPids, nativePids, extraPids); return tracesFile; } @@ -5461,8 +5489,7 @@ public class ActivityManagerService extends IActivityManager.Stub } private static void dumpStackTraces(String tracesPath, ArrayList firstPids, - ProcessCpuTracker processCpuTracker, SparseArray lastPids, - ArrayList nativePids) { + ArrayList nativePids, ArrayList extraPids) { // Use a FileObserver to detect when traces finish writing. // The order of traces is considered important to maintain for legibility. DumpStackFileObserver observer = new DumpStackFileObserver(tracesPath); @@ -5518,43 +5545,21 @@ public class ActivityManagerService extends IActivityManager.Stub } } - // Lastly, measure CPU usage. - if (processCpuTracker != null) { - processCpuTracker.init(); - System.gc(); - processCpuTracker.update(); - try { - synchronized (processCpuTracker) { - processCpuTracker.wait(500); // measure over 1/2 second. - } - } catch (InterruptedException e) { - } - processCpuTracker.update(); + // Lastly, dump stacks for all extra PIDs from the CPU tracker. + if (extraPids != null) { + for (int pid : extraPids) { + if (DEBUG_ANR) Slog.d(TAG, "Collecting stacks for extra pid " + pid); - // We'll take the stack crawls of just the top apps using CPU. - final int N = processCpuTracker.countWorkingStats(); - int numProcs = 0; - for (int i=0; i= 0) { - numProcs++; - - if (DEBUG_ANR) Slog.d(TAG, "Collecting stacks for extra pid " + stats.pid); - - final long timeTaken = observer.dumpWithTimeout(stats.pid, remainingTime); - remainingTime -= timeTaken; - if (remainingTime <= 0) { - Slog.e(TAG, "Aborting stack trace dump (current extra pid=" + stats.pid + + final long timeTaken = observer.dumpWithTimeout(pid, remainingTime); + remainingTime -= timeTaken; + if (remainingTime <= 0) { + Slog.e(TAG, "Aborting stack trace dump (current extra pid=" + pid + "); deadline exceeded."); - return; - } + return; + } - if (DEBUG_ANR) { - Slog.d(TAG, "Done with extra pid " + stats.pid + " in " + timeTaken + "ms"); - } - } else if (DEBUG_ANR) { - Slog.d(TAG, "Skipping next CPU consuming process, not a java proc: " - + stats.pid); + if (DEBUG_ANR) { + Slog.d(TAG, "Done with extra pid " + pid + " in " + timeTaken + "ms"); } } } @@ -5606,7 +5611,7 @@ public class ActivityManagerService extends IActivityManager.Stub if (app != null) { ArrayList firstPids = new ArrayList(); firstPids.add(app.pid); - dumpStackTraces(tracesPath, firstPids, null, null, null); + dumpStackTraces(tracesPath, firstPids, null, null); } File lastTracesFile = null;