OSDN Git Service

Support WorkChains for WakeLock start / stop / change events.
authorNarayan Kamath <narayan@google.com>
Fri, 8 Dec 2017 11:56:01 +0000 (11:56 +0000)
committerNarayan Kamath <narayan@google.com>
Fri, 22 Dec 2017 16:21:07 +0000 (16:21 +0000)
Log WorkChains associated with a given WorkSource to statsd whenever
a wakelock is acquired / released or changes.

Test: WorkSourceTest, manual.
Bug: 62390666

Change-Id: I1720ba8b1778d38067398caac7cf92c4d375f816

core/java/android/os/PowerManager.java
core/java/android/os/WorkSource.java
core/java/com/android/internal/os/BatteryStatsImpl.java
core/tests/coretests/src/android/os/WorkSourceTest.java
services/core/java/com/android/server/am/BatteryStatsService.java
services/core/java/com/android/server/power/PowerManagerService.java

index 56c6391..cd6d41b 100644 (file)
@@ -1540,7 +1540,7 @@ public final class PowerManager {
          */
         public void setWorkSource(WorkSource ws) {
             synchronized (mToken) {
-                if (ws != null && ws.size() == 0) {
+                if (ws != null && ws.isEmpty()) {
                     ws = null;
                 }
 
@@ -1552,7 +1552,7 @@ public final class PowerManager {
                     changed = true;
                     mWorkSource = new WorkSource(ws);
                 } else {
-                    changed = mWorkSource.diff(ws);
+                    changed = !mWorkSource.equals(ws);
                     if (changed) {
                         mWorkSource.set(ws);
                     }
index bf145a0..e9d4fe8 100644 (file)
@@ -456,6 +456,16 @@ public class WorkSource implements Parcelable {
     }
 
     /**
+     * Returns {@code true} iff. this work source contains zero UIDs and zero WorkChains to
+     * attribute usage to.
+     *
+     * @hide for internal use only.
+     */
+    public boolean isEmpty() {
+        return mNum == 0 && (mChains == null || mChains.isEmpty());
+    }
+
+    /**
      * @return the list of {@code WorkChains} associated with this {@code WorkSource}.
      * @hide
      */
@@ -842,6 +852,14 @@ public class WorkSource implements Parcelable {
             return this;
         }
 
+        /**
+         * Return the UID to which this WorkChain should be attributed to, i.e, the UID performing
+         * the actual work.
+         */
+        public int getAttributionUid() {
+            return mUids[mSize - 1];
+        }
+
         // TODO: The following three trivial getters are purely for testing and will be removed
         // once we have higher level logic in place, e.g for serializing this WorkChain to a proto,
         // diffing it etc.
@@ -932,6 +950,55 @@ public class WorkSource implements Parcelable {
                 };
     }
 
+    /**
+     * Computes the differences in WorkChains contained between {@code oldWs} and {@code newWs}.
+     *
+     * Returns {@code null} if no differences exist, otherwise returns a two element array. The
+     * first element is a list of "new" chains, i.e WorkChains present in {@code newWs} but not in
+     * {@code oldWs}. The second element is a list of "gone" chains, i.e WorkChains present in
+     * {@code oldWs} but not in {@code newWs}.
+     *
+     * @hide
+     */
+    public static ArrayList<WorkChain>[] diffChains(WorkSource oldWs, WorkSource newWs) {
+        ArrayList<WorkChain> newChains = null;
+        ArrayList<WorkChain> goneChains = null;
+
+        // TODO(narayan): This is a dumb O(M*N) algorithm that determines what has changed across
+        // WorkSource objects. We can replace this with something smarter, for e.g by defining
+        // a Comparator between WorkChains. It's unclear whether that will be more efficient if
+        // the number of chains associated with a WorkSource is expected to be small
+        if (oldWs.mChains != null) {
+            for (int i = 0; i < oldWs.mChains.size(); ++i) {
+                final WorkChain wc = oldWs.mChains.get(i);
+                if (newWs.mChains == null || !newWs.mChains.contains(wc)) {
+                    if (goneChains == null) {
+                        goneChains = new ArrayList<>(oldWs.mChains.size());
+                    }
+                    goneChains.add(wc);
+                }
+            }
+        }
+
+        if (newWs.mChains != null) {
+            for (int i = 0; i < newWs.mChains.size(); ++i) {
+                final WorkChain wc = newWs.mChains.get(i);
+                if (oldWs.mChains == null || !oldWs.mChains.contains(wc)) {
+                    if (newChains == null) {
+                        newChains = new ArrayList<>(newWs.mChains.size());
+                    }
+                    newChains.add(wc);
+                }
+            }
+        }
+
+        if (newChains != null || goneChains != null) {
+            return new ArrayList[] { newChains, goneChains };
+        }
+
+        return null;
+    }
+
     @Override
     public int describeContents() {
         return 0;
index 9ab16d8..8966585 100644 (file)
@@ -45,6 +45,7 @@ import android.os.ServiceManager;
 import android.os.SystemClock;
 import android.os.UserHandle;
 import android.os.WorkSource;
+import android.os.WorkSource.WorkChain;
 import android.telephony.DataConnectionRealTimeInfo;
 import android.telephony.ModemActivityInfo;
 import android.telephony.ServiceState;
@@ -79,6 +80,7 @@ import com.android.internal.util.FastXmlSerializer;
 import com.android.internal.util.JournaledFile;
 import com.android.internal.util.XmlUtils;
 
+import java.util.List;
 import libcore.util.EmptyArray;
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
@@ -4064,8 +4066,8 @@ public class BatteryStatsImpl extends BatteryStats {
     private String mInitialAcquireWakeName;
     private int mInitialAcquireWakeUid = -1;
 
-    public void noteStartWakeLocked(int uid, int pid, String name, String historyName, int type,
-            boolean unimportantForLogging, long elapsedRealtime, long uptime) {
+    public void noteStartWakeLocked(int uid, int pid, WorkChain wc, String name, String historyName,
+        int type, boolean unimportantForLogging, long elapsedRealtime, long uptime) {
         uid = mapUid(uid);
         if (type == WAKE_TYPE_PARTIAL) {
             // Only care about partial wake locks, since full wake locks
@@ -4113,12 +4115,21 @@ public class BatteryStatsImpl extends BatteryStats {
                 }
                 requestWakelockCpuUpdate();
             }
+
             getUidStatsLocked(uid).noteStartWakeLocked(pid, name, type, elapsedRealtime);
+
+            // TODO(statsd): Use the attribution chain specified in WorkChain instead of uid.
+            // The debug logging here can be deleted once statsd is wired up.
+            if (DEBUG) {
+                Slog.w(TAG, "StatsLog [start]: uid=" + uid + ", type=" + type + ", name=" + name
+                + ", wc=" + wc);
+            }
+            StatsLog.write(StatsLog.WAKELOCK_STATE_CHANGED, uid, type, name, 1);
         }
     }
 
-    public void noteStopWakeLocked(int uid, int pid, String name, String historyName, int type,
-            long elapsedRealtime, long uptime) {
+    public void noteStopWakeLocked(int uid, int pid, WorkChain wc, String name, String historyName,
+            int type, long elapsedRealtime, long uptime) {
         uid = mapUid(uid);
         if (type == WAKE_TYPE_PARTIAL) {
             mWakeLockNesting--;
@@ -4148,7 +4159,16 @@ public class BatteryStatsImpl extends BatteryStats {
                 }
                 requestWakelockCpuUpdate();
             }
+
             getUidStatsLocked(uid).noteStopWakeLocked(pid, name, type, elapsedRealtime);
+
+            // TODO(statsd): Use the attribution chain specified in WorkChain instead of uid.
+            // The debug logging here can be deleted once statsd is wired up.
+            if (DEBUG) {
+                Slog.w(TAG, "StatsLog [stop]: uid=" + uid + ", type=" + type + ", name=" + name
+                    + ", wc=" + wc);
+            }
+            StatsLog.write(StatsLog.WAKELOCK_STATE_CHANGED, uid, type, name, 0);
         }
     }
 
@@ -4158,8 +4178,17 @@ public class BatteryStatsImpl extends BatteryStats {
         final long uptime = mClocks.uptimeMillis();
         final int N = ws.size();
         for (int i=0; i<N; i++) {
-            noteStartWakeLocked(ws.get(i), pid, name, historyName, type, unimportantForLogging,
-                    elapsedRealtime, uptime);
+            noteStartWakeLocked(ws.get(i), pid, null, name, historyName, type,
+                    unimportantForLogging, elapsedRealtime, uptime);
+        }
+
+        List<WorkChain> wcs = ws.getWorkChains();
+        if (wcs != null) {
+            for (int i = 0; i < wcs.size(); ++i) {
+                final WorkChain wc = wcs.get(i);
+                noteStartWakeLocked(wc.getAttributionUid(), pid, wc, name, historyName, type,
+                        unimportantForLogging, elapsedRealtime, uptime);
+            }
         }
     }
 
@@ -4168,17 +4197,46 @@ public class BatteryStatsImpl extends BatteryStats {
             String newHistoryName, int newType, boolean newUnimportantForLogging) {
         final long elapsedRealtime = mClocks.elapsedRealtime();
         final long uptime = mClocks.uptimeMillis();
+
+        List<WorkChain>[] wcs = WorkSource.diffChains(ws, newWs);
+
         // For correct semantics, we start the need worksources first, so that we won't
         // make inappropriate history items as if all wake locks went away and new ones
         // appeared.  This is okay because tracking of wake locks allows nesting.
+        //
+        // First the starts :
         final int NN = newWs.size();
         for (int i=0; i<NN; i++) {
-            noteStartWakeLocked(newWs.get(i), newPid, newName, newHistoryName, newType,
+            noteStartWakeLocked(newWs.get(i), newPid, null, newName, newHistoryName, newType,
                     newUnimportantForLogging, elapsedRealtime, uptime);
         }
+        if (wcs != null) {
+            List<WorkChain> newChains = wcs[0];
+            if (newChains != null) {
+                for (int i = 0; i < newChains.size(); ++i) {
+                    final WorkChain newChain = newChains.get(i);
+                    noteStartWakeLocked(newChain.getAttributionUid(), newPid, newChain, newName,
+                        newHistoryName, newType, newUnimportantForLogging, elapsedRealtime,
+                        uptime);
+                }
+            }
+        }
+
+        // Then the stops :
         final int NO = ws.size();
         for (int i=0; i<NO; i++) {
-            noteStopWakeLocked(ws.get(i), pid, name, historyName, type, elapsedRealtime, uptime);
+            noteStopWakeLocked(ws.get(i), pid, null, name, historyName, type, elapsedRealtime,
+                    uptime);
+        }
+        if (wcs != null) {
+            List<WorkChain> goneChains = wcs[1];
+            if (goneChains != null) {
+                for (int i = 0; i < goneChains.size(); ++i) {
+                    final WorkChain goneChain = goneChains.get(i);
+                    noteStopWakeLocked(goneChain.getAttributionUid(), pid, goneChain, name,
+                            historyName, type, elapsedRealtime, uptime);
+                }
+            }
         }
     }
 
@@ -4188,7 +4246,17 @@ public class BatteryStatsImpl extends BatteryStats {
         final long uptime = mClocks.uptimeMillis();
         final int N = ws.size();
         for (int i=0; i<N; i++) {
-            noteStopWakeLocked(ws.get(i), pid, name, historyName, type, elapsedRealtime, uptime);
+            noteStopWakeLocked(ws.get(i), pid, null, name, historyName, type, elapsedRealtime,
+                    uptime);
+        }
+
+        List<WorkChain> wcs = ws.getWorkChains();
+        if (wcs != null) {
+            for (int i = 0; i < wcs.size(); ++i) {
+                final WorkChain wc = wcs.get(i);
+                noteStopWakeLocked(wc.getAttributionUid(), pid, wc, name, historyName, type,
+                        elapsedRealtime, uptime);
+            }
         }
     }
 
@@ -4432,10 +4500,10 @@ public class BatteryStatsImpl extends BatteryStats {
                 updateTimeBasesLocked(mOnBatteryTimeBase.isRunning(), state,
                         mClocks.uptimeMillis() * 1000, elapsedRealtime * 1000);
                 // Fake a wake lock, so we consider the device waked as long as the screen is on.
-                noteStartWakeLocked(-1, -1, "screen", null, WAKE_TYPE_PARTIAL, false,
+                noteStartWakeLocked(-1, -1, null, "screen", null, WAKE_TYPE_PARTIAL, false,
                         elapsedRealtime, uptime);
             } else if (isScreenOn(oldState)) {
-                noteStopWakeLocked(-1, -1, "screen", "screen", WAKE_TYPE_PARTIAL,
+                noteStopWakeLocked(-1, -1, null, "screen", "screen", WAKE_TYPE_PARTIAL,
                         elapsedRealtime, uptime);
                 updateTimeBasesLocked(mOnBatteryTimeBase.isRunning(), state,
                         mClocks.uptimeMillis() * 1000, elapsedRealtime * 1000);
@@ -9227,8 +9295,6 @@ public class BatteryStatsImpl extends BatteryStats {
             Wakelock wl = mWakelockStats.startObject(name);
             if (wl != null) {
                 getWakelockTimerLocked(wl, type).startRunningLocked(elapsedRealtimeMs);
-                // TODO(statsd): Hopefully use a worksource instead of a uid (so move elsewhere)
-                StatsLog.write(StatsLog.WAKELOCK_STATE_CHANGED, getUid(), type, name, 1);
             }
             if (type == WAKE_TYPE_PARTIAL) {
                 createAggregatedPartialWakelockTimerLocked().startRunningLocked(elapsedRealtimeMs);
@@ -9247,8 +9313,6 @@ public class BatteryStatsImpl extends BatteryStats {
                 StopwatchTimer wlt = getWakelockTimerLocked(wl, type);
                 wlt.stopRunningLocked(elapsedRealtimeMs);
                 if (!wlt.isRunningLocked()) { // only tell statsd if truly stopped
-                    // TODO(statsd): Possibly use a worksource instead of a uid.
-                    StatsLog.write(StatsLog.WAKELOCK_STATE_CHANGED, getUid(), type, name, 0);
                 }
             }
             if (type == WAKE_TYPE_PARTIAL) {
index 704b780..e2f800f 100644 (file)
@@ -20,6 +20,7 @@ import android.os.WorkSource.WorkChain;
 
 import junit.framework.TestCase;
 
+import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -218,4 +219,107 @@ public class WorkSourceTest extends TestCase {
         assertEquals(20, ws2.get(0));
         assertEquals("foo", ws2.getName(0));
     }
+
+    public void testDiffChains_noChanges() {
+        // WorkSources with no chains.
+        assertEquals(null, WorkSource.diffChains(new WorkSource(), new WorkSource()));
+
+        // WorkSources with the same chains.
+        WorkSource ws1 = new WorkSource();
+        ws1.createWorkChain().addNode(50, "tag");
+        ws1.createWorkChain().addNode(60, "tag2");
+
+        WorkSource ws2 = new WorkSource();
+        ws2.createWorkChain().addNode(50, "tag");
+        ws2.createWorkChain().addNode(60, "tag2");
+
+        assertEquals(null, WorkSource.diffChains(ws1, ws1));
+        assertEquals(null, WorkSource.diffChains(ws2, ws1));
+    }
+
+    public void testDiffChains_noChains() {
+        // Diffs against a worksource with no chains.
+        WorkSource ws1 = new WorkSource();
+        WorkSource ws2 = new WorkSource();
+        ws2.createWorkChain().addNode(70, "tag");
+        ws2.createWorkChain().addNode(60, "tag2");
+
+        // The "old" work source has no chains, so "newChains" should be non-null.
+        ArrayList<WorkChain>[] diffs = WorkSource.diffChains(ws1, ws2);
+        assertNotNull(diffs[0]);
+        assertNull(diffs[1]);
+        assertEquals(2, diffs[0].size());
+        assertEquals(ws2.getWorkChains(), diffs[0]);
+
+        // The "new" work source has no chains, so "oldChains" should be non-null.
+        diffs = WorkSource.diffChains(ws2, ws1);
+        assertNull(diffs[0]);
+        assertNotNull(diffs[1]);
+        assertEquals(2, diffs[1].size());
+        assertEquals(ws2.getWorkChains(), diffs[1]);
+    }
+
+    public void testDiffChains_onlyAdditionsOrRemovals() {
+        WorkSource ws1 = new WorkSource();
+        WorkSource ws2 = new WorkSource();
+        ws2.createWorkChain().addNode(70, "tag");
+        ws2.createWorkChain().addNode(60, "tag2");
+
+        // Both work sources have WorkChains : test the case where changes were only added
+        // or were only removed.
+        ws1.createWorkChain().addNode(70, "tag");
+
+        // The "new" work source only contains additions (60, "tag2") in this case.
+        ArrayList<WorkChain>[] diffs = WorkSource.diffChains(ws1, ws2);
+        assertNotNull(diffs[0]);
+        assertNull(diffs[1]);
+        assertEquals(1, diffs[0].size());
+        assertEquals(new WorkChain().addNode(60, "tag2"), diffs[0].get(0));
+
+        // The "new" work source only contains removals (60, "tag2") in this case.
+        diffs = WorkSource.diffChains(ws2, ws1);
+        assertNull(diffs[0]);
+        assertNotNull(diffs[1]);
+        assertEquals(1, diffs[1].size());
+        assertEquals(new WorkChain().addNode(60, "tag2"), diffs[1].get(0));
+    }
+
+
+    public void testDiffChains_generalCase() {
+        WorkSource ws1 = new WorkSource();
+        WorkSource ws2 = new WorkSource();
+
+        // Both work sources have WorkChains, test the case where chains were added AND removed.
+        ws1.createWorkChain().addNode(0, "tag0");
+        ws2.createWorkChain().addNode(0, "tag0_changed");
+        ArrayList<WorkChain>[] diffs = WorkSource.diffChains(ws1, ws2);
+        assertNotNull(diffs[0]);
+        assertNotNull(diffs[1]);
+        assertEquals(ws2.getWorkChains(), diffs[0]);
+        assertEquals(ws1.getWorkChains(), diffs[1]);
+
+        // Give both WorkSources a chain in common; it should not be a part of any diffs.
+        ws1.createWorkChain().addNode(1, "tag1");
+        ws2.createWorkChain().addNode(1, "tag1");
+        diffs = WorkSource.diffChains(ws1, ws2);
+        assertNotNull(diffs[0]);
+        assertNotNull(diffs[1]);
+        assertEquals(1, diffs[0].size());
+        assertEquals(1, diffs[1].size());
+        assertEquals(new WorkChain().addNode(0, "tag0_changed"), diffs[0].get(0));
+        assertEquals(new WorkChain().addNode(0, "tag0"), diffs[1].get(0));
+
+        // Finally, test the case where more than one chain was added / removed.
+        ws1.createWorkChain().addNode(2, "tag2");
+        ws2.createWorkChain().addNode(2, "tag2_changed");
+        diffs = WorkSource.diffChains(ws1, ws2);
+        assertNotNull(diffs[0]);
+        assertNotNull(diffs[1]);
+        assertEquals(2, diffs[0].size());
+        assertEquals(2, diffs[1].size());
+        assertEquals(new WorkChain().addNode(0, "tag0_changed"), diffs[0].get(0));
+        assertEquals(new WorkChain().addNode(2, "tag2_changed"), diffs[0].get(1));
+        assertEquals(new WorkChain().addNode(0, "tag0"), diffs[1].get(0));
+        assertEquals(new WorkChain().addNode(2, "tag2"), diffs[1].get(1));
+    }
 }
index b920b57..35318f6 100644 (file)
@@ -464,15 +464,16 @@ public final class BatteryStatsService extends IBatteryStats.Stub
             boolean unimportantForLogging) {
         enforceCallingPermission();
         synchronized (mStats) {
-            mStats.noteStartWakeLocked(uid, pid, name, historyName, type, unimportantForLogging,
-                    SystemClock.elapsedRealtime(), SystemClock.uptimeMillis());
+            mStats.noteStartWakeLocked(uid, pid, null, name, historyName, type,
+                    unimportantForLogging, SystemClock.elapsedRealtime(),
+                    SystemClock.uptimeMillis());
         }
     }
 
     public void noteStopWakelock(int uid, int pid, String name, String historyName, int type) {
         enforceCallingPermission();
         synchronized (mStats) {
-            mStats.noteStopWakeLocked(uid, pid, name, historyName, type,
+            mStats.noteStopWakeLocked(uid, pid, null, name, historyName, type,
                     SystemClock.elapsedRealtime(), SystemClock.uptimeMillis());
         }
     }
index 0b590bc..02c8f68 100644 (file)
@@ -4324,7 +4324,7 @@ public final class PowerManagerService extends SystemService
                 mContext.enforceCallingOrSelfPermission(
                         android.Manifest.permission.DEVICE_POWER, null);
             }
-            if (ws != null && ws.size() != 0) {
+            if (ws != null && !ws.isEmpty()) {
                 mContext.enforceCallingOrSelfPermission(
                         android.Manifest.permission.UPDATE_DEVICE_STATS, null);
             } else {
@@ -4379,7 +4379,7 @@ public final class PowerManagerService extends SystemService
             }
 
             mContext.enforceCallingOrSelfPermission(android.Manifest.permission.WAKE_LOCK, null);
-            if (ws != null && ws.size() != 0) {
+            if (ws != null && !ws.isEmpty()) {
                 mContext.enforceCallingOrSelfPermission(
                         android.Manifest.permission.UPDATE_DEVICE_STATS, null);
             } else {