From 652973fca83c48d8b4622493f10e656b8d86dd17 Mon Sep 17 00:00:00 2001
From: Dianne Hackborn <hackbod@google.com>
Date: Wed, 10 Sep 2014 17:08:48 -0700
Subject: [PATCH] Fix issue #17414533: update batterystats occasionally takes a
 couple...

...hundred milliseconds.

Rework the locking so that no critical paths block on the cpu collection.

Change-Id: Ie615a033f7f8b523b67abee62c581d1a8fce324c
---
 .../internal/os/ProcessCpuTracker.java        | 24 ++++++++++--------
 .../server/am/ActivityManagerService.java     | 25 +++++++++++--------
 .../com/android/server/am/ActivityStack.java  | 11 +++-----
 3 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/core/java/com/android/internal/os/ProcessCpuTracker.java b/core/java/com/android/internal/os/ProcessCpuTracker.java
index e58d68fd19809..86f580d3461d4 100644
--- a/core/java/com/android/internal/os/ProcessCpuTracker.java
+++ b/core/java/com/android/internal/os/ProcessCpuTracker.java
@@ -64,7 +64,9 @@ public class ProcessCpuTracker {
 
     /** Stores user time and system time in 100ths of a second. */
     private final long[] mProcessStatsData = new long[4];
-    /** Stores user time and system time in 100ths of a second. */
+
+    /** Stores user time and system time in 100ths of a second.  Used for
+     * public API to retrieve CPU use for a process.  Must lock while in use. */
     private final long[] mSinglePidStatsData = new long[4];
 
     private static final int[] PROCESS_FULL_STATS_FORMAT = new int[] {
@@ -533,18 +535,20 @@ public class ProcessCpuTracker {
 
     /**
      * Returns the total time (in clock ticks, or 1/100 sec) spent executing in
-     * both user and system code.
+     * both user and system code.  Safe to call without lock held.
      */
     public long getCpuTimeForPid(int pid) {
-        final String statFile = "/proc/" + pid + "/stat";
-        final long[] statsData = mSinglePidStatsData;
-        if (Process.readProcFile(statFile, PROCESS_STATS_FORMAT,
-                null, statsData, null)) {
-            long time = statsData[PROCESS_STAT_UTIME]
-                    + statsData[PROCESS_STAT_STIME];
-            return time;
+        synchronized (mSinglePidStatsData) {
+            final String statFile = "/proc/" + pid + "/stat";
+            final long[] statsData = mSinglePidStatsData;
+            if (Process.readProcFile(statFile, PROCESS_STATS_FORMAT,
+                    null, statsData, null)) {
+                long time = statsData[PROCESS_STAT_UTIME]
+                        + statsData[PROCESS_STAT_STIME];
+                return time;
+            }
+            return 0;
         }
-        return 0;
     }
 
     /**
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index a9a47327ead66..3d2e7d280961c 100755
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -1075,13 +1075,16 @@ public final class ActivityManagerService extends ActivityManagerNative
 
     /**
      * Runtime CPU use collection thread.  This object's lock is used to
-     * protect all related state.
+     * perform synchronization with the thread (notifying it to run).
      */
     final Thread mProcessCpuThread;
 
     /**
-     * Used to collect process stats when showing not responding dialog.
-     * Protected by mProcessCpuThread.
+     * Used to collect per-process CPU use for ANRs, battery stats, etc.
+     * Must acquire this object's lock when accessing it.
+     * NOTE: this lock will be held while doing long operations (trawling
+     * through all processes in /proc), so it should never be acquired by
+     * any critical paths such as when holding the main activity manager lock.
      */
     final ProcessCpuTracker mProcessCpuTracker = new ProcessCpuTracker(
             MONITOR_THREAD_CPU_USAGE);
@@ -1595,7 +1598,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                             infoMap.put(mi.pid, mi);
                         }
                         updateCpuStatsNow();
-                        synchronized (mProcessCpuThread) {
+                        synchronized (mProcessCpuTracker) {
                             final int N = mProcessCpuTracker.countStats();
                             for (int i=0; i<N; i++) {
                                 ProcessCpuTracker.Stats st = mProcessCpuTracker.getStats(i);
@@ -1894,7 +1897,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                 if (memInfo != null) {
                     updateCpuStatsNow();
                     long nativeTotalPss = 0;
-                    synchronized (mProcessCpuThread) {
+                    synchronized (mProcessCpuTracker) {
                         final int N = mProcessCpuTracker.countStats();
                         for (int j=0; j<N; j++) {
                             ProcessCpuTracker.Stats st = mProcessCpuTracker.getStats(j);
@@ -1912,7 +1915,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                         }
                     }
                     memInfo.readMemInfo();
-                    synchronized (this) {
+                    synchronized (ActivityManagerService.this) {
                         if (DEBUG_PSS) Slog.d(TAG, "Collected native and kernel memory in "
                                 + (SystemClock.uptimeMillis()-start) + "ms");
                         mProcessStats.addSysMemUsageLocked(memInfo.getCachedSizeKb(),
@@ -2183,7 +2186,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                 return;
             }
 
-            synchronized (mActivityManagerService.mProcessCpuThread) {
+            synchronized (mActivityManagerService.mProcessCpuTracker) {
                 pw.print(mActivityManagerService.mProcessCpuTracker.printCurrentLoad());
                 pw.print(mActivityManagerService.mProcessCpuTracker.printCurrentState(
                         SystemClock.uptimeMillis()));
@@ -2379,7 +2382,7 @@ public final class ActivityManagerService extends ActivityManagerNative
     }
 
     void updateCpuStatsNow() {
-        synchronized (mProcessCpuThread) {
+        synchronized (mProcessCpuTracker) {
             mProcessCpuMutexFree.set(false);
             final long now = SystemClock.uptimeMillis();
             boolean haveNewCpuStats = false;
@@ -5074,7 +5077,7 @@ public final class ActivityManagerService extends ActivityManagerNative
         String cpuInfo = null;
         if (MONITOR_CPU_USAGE) {
             updateCpuStatsNow();
-            synchronized (mProcessCpuThread) {
+            synchronized (mProcessCpuTracker) {
                 cpuInfo = mProcessCpuTracker.printCurrentState(anrTime);
             }
             info.append(processCpuTracker.printCurrentLoad());
@@ -13700,7 +13703,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                     findPid = Integer.parseInt(args[opti]);
                 } catch (NumberFormatException e) {
                 }
-                synchronized (mProcessCpuThread) {
+                synchronized (mProcessCpuTracker) {
                     final int N = mProcessCpuTracker.countStats();
                     for (int i=0; i<N; i++) {
                         ProcessCpuTracker.Stats st = mProcessCpuTracker.getStats(i);
@@ -13862,7 +13865,7 @@ public final class ActivityManagerService extends ActivityManagerNative
             // If we are showing aggregations, also look for native processes to
             // include so that our aggregations are more accurate.
             updateCpuStatsNow();
-            synchronized (mProcessCpuThread) {
+            synchronized (mProcessCpuTracker) {
                 final int N = mProcessCpuTracker.countStats();
                 for (int i=0; i<N; i++) {
                     ProcessCpuTracker.Stats st = mProcessCpuTracker.getStats(i);
diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index bcf3b17d1085e..d71f94e143935 100755
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -1042,11 +1042,8 @@ final class ActivityStack {
 
             if (prev.app != null && prev.cpuTimeAtResume > 0
                     && mService.mBatteryStatsService.isOnBattery()) {
-                long diff;
-                synchronized (mService.mProcessCpuThread) {
-                    diff = mService.mProcessCpuTracker.getCpuTimeForPid(prev.app.pid)
-                            - prev.cpuTimeAtResume;
-                }
+                long diff = mService.mProcessCpuTracker.getCpuTimeForPid(prev.app.pid)
+                        - prev.cpuTimeAtResume;
                 if (diff > 0) {
                     BatteryStatsImpl bsi = mService.mBatteryStatsService.getActiveStatistics();
                     synchronized (bsi) {
@@ -1097,9 +1094,7 @@ final class ActivityStack {
         // TODO: To be more accurate, the mark should be before the onCreate,
         //       not after the onResume. But for subsequent starts, onResume is fine.
         if (next.app != null) {
-            synchronized (mService.mProcessCpuThread) {
-                next.cpuTimeAtResume = mService.mProcessCpuTracker.getCpuTimeForPid(next.app.pid);
-            }
+            next.cpuTimeAtResume = mService.mProcessCpuTracker.getCpuTimeForPid(next.app.pid);
         } else {
             next.cpuTimeAtResume = 0; // Couldn't get the cpu time of process
         }