Merge "Merge "Fix parsing of kernel wakelocks" into nyc-dev am: 6f0d399214 am: 7dd96663a0" into nyc-mr1-dev-plus-aosp

core/java/com/android/internal/os/BatteryStatsImpl.java

@@ -8873,8 +8873,6 @@ public class BatteryStatsImpl extends BatteryStats {
             return;
         }
 
-        // Record whether we've seen a non-zero time (for debugging b/22716723).
-        boolean seenNonZeroTime = false;
         for (Map.Entry<String, KernelWakelockStats.Entry> ent : wakelockStats.entrySet()) {
             String name = ent.getKey();
             KernelWakelockStats.Entry kws = ent.getValue();
@@ -8884,27 +8882,24 @@ public class BatteryStatsImpl extends BatteryStats {
                 kwlt = new SamplingTimer(mClocks, mOnBatteryScreenOffTimeBase);
                 mKernelWakelockStats.put(name, kwlt);
             }
+
             kwlt.update(kws.mTotalTime, kws.mCount);
             kwlt.setUpdateVersion(kws.mVersion);
-
-            if (kws.mVersion != wakelockStats.kernelWakelockVersion) {
-                seenNonZeroTime |= kws.mTotalTime > 0;
-            }
         }
 
         int numWakelocksSetStale = 0;
-        if (wakelockStats.size() != mKernelWakelockStats.size()) {
+        // Set timers to stale if they didn't appear in /d/wakeup_sources (or /proc/wakelocks)
-            // Set timers to stale if they didn't appear in /proc/wakelocks this time.
+        // this time.
-            for (Map.Entry<String, SamplingTimer> ent : mKernelWakelockStats.entrySet()) {
+        for (Map.Entry<String, SamplingTimer> ent : mKernelWakelockStats.entrySet()) {
-                SamplingTimer st = ent.getValue();
+            SamplingTimer st = ent.getValue();
-                if (st.getUpdateVersion() != wakelockStats.kernelWakelockVersion) {
+            if (st.getUpdateVersion() != wakelockStats.kernelWakelockVersion) {
-                    st.endSample();
+                st.endSample();
-                    numWakelocksSetStale++;
+                numWakelocksSetStale++;
-                }
             }
         }
 
-        if (!seenNonZeroTime) {
+        // Record whether we've seen a non-zero time (for debugging b/22716723).
+        if (wakelockStats.isEmpty()) {
             Slog.wtf(TAG, "All kernel wakelocks had time of zero");
         }
 

core/java/com/android/internal/os/KernelWakelockReader.java

@@ -18,6 +18,8 @@ package com.android.internal.os;
 import android.os.Process;
 import android.util.Slog;
 
+import com.android.internal.annotations.VisibleForTesting;
+
 import java.io.FileInputStream;
 import java.util.Iterator;
 
@@ -106,14 +108,14 @@ public class KernelWakelockReader {
     /**
      * Reads the wakelocks and updates the staleStats with the new information.
      */
-    private KernelWakelockStats parseProcWakelocks(byte[] wlBuffer, int len, boolean wakeup_sources,
+    @VisibleForTesting
-                                                   final KernelWakelockStats staleStats) {
+    public KernelWakelockStats parseProcWakelocks(byte[] wlBuffer, int len, boolean wakeup_sources,
+                                                  final KernelWakelockStats staleStats) {
         String name;
         int count;
         long totalTime;
         int startIndex;
         int endIndex;
-        int numUpdatedWlNames = 0;
 
         // Advance past the first line.
         int i;
@@ -126,11 +128,10 @@ public class KernelWakelockReader {
                 for (endIndex=startIndex;
                         endIndex < len && wlBuffer[endIndex] != '\n' && wlBuffer[endIndex] != '\0';
                         endIndex++);
-                endIndex++; // endIndex is an exclusive upper bound.
                 // Don't go over the end of the buffer, Process.parseProcLine might
                 // write to wlBuffer[endIndex]
-                if (endIndex >= (len - 1) ) {
+                if (endIndex > (len - 1) ) {
-                    return staleStats;
+                    break;
                 }
 
                 String[] nameStringArray = mProcWakelocksName;
@@ -161,7 +162,6 @@ public class KernelWakelockReader {
                     if (!staleStats.containsKey(name)) {
                         staleStats.put(name, new KernelWakelockStats.Entry(count, totalTime,
                                 sKernelWakelockUpdateVersion));
-                        numUpdatedWlNames++;
                     } else {
                         KernelWakelockStats.Entry kwlStats = staleStats.get(name);
                         if (kwlStats.mVersion == sKernelWakelockUpdateVersion) {
@@ -171,7 +171,6 @@ public class KernelWakelockReader {
                             kwlStats.mCount = count;
                             kwlStats.mTotalTime = totalTime;
                             kwlStats.mVersion = sKernelWakelockUpdateVersion;
-                            numUpdatedWlNames++;
                         }
                     }
                 } else if (!parsed) {
@@ -182,16 +181,14 @@ public class KernelWakelockReader {
                         Slog.wtf(TAG, "Failed to parse proc line!");
                     }
                 }
-                startIndex = endIndex;
+                startIndex = endIndex + 1;
             }
 
-            if (staleStats.size() != numUpdatedWlNames) {
+            // Don't report old data.
-                // Don't report old data.
+            Iterator<KernelWakelockStats.Entry> itr = staleStats.values().iterator();
-                Iterator<KernelWakelockStats.Entry> itr = staleStats.values().iterator();
+            while (itr.hasNext()) {
-                while (itr.hasNext()) {
+                if (itr.next().mVersion != sKernelWakelockUpdateVersion) {
-                    if (itr.next().mVersion != sKernelWakelockUpdateVersion) {
+                    itr.remove();
-                        itr.remove();
-                    }
                 }
             }
 

core/tests/coretests/src/com/android/internal/os/KernelWakelockReaderTest.java

@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.android.internal.os;
+
+import android.support.test.filters.SmallTest;
+
+import junit.framework.TestCase;
+
+import java.nio.charset.Charset;
+
+public class KernelWakelockReaderTest extends TestCase {
+    /**
+     * Helper class that builds the mock Kernel module file /d/wakeup_sources.
+     */
+    private static class ProcFileBuilder {
+        private final static String sHeader = "name\t\tactive_count\tevent_count\twakeup_count\t" +
+                "expire_count\tactive_since\ttotal_time\tmax_time\tlast_change\t" +
+                "prevent_suspend_time\n";
+
+        private StringBuilder mStringBuilder;
+
+        private void ensureHeader() {
+            if (mStringBuilder == null) {
+                mStringBuilder = new StringBuilder();
+                mStringBuilder.append(sHeader);
+            }
+        }
+
+        public ProcFileBuilder addLine(String name, int count, long timeMillis) {
+            ensureHeader();
+            mStringBuilder.append(name).append("\t").append(count).append("\t0\t0\t0\t0\t")
+                    .append(timeMillis).append("\t0\t0\t0\n");
+            return this;
+        }
+
+        public byte[] getBytes() throws Exception {
+            ensureHeader();
+            byte[] data = mStringBuilder.toString().getBytes(Charset.forName("UTF-8"));
+
+            // The Kernel puts a \0 at the end of the data. Since each of our lines ends with \n,
+            // we override the last \n with a \0.
+            data[data.length - 1] = 0;
+            return data;
+        }
+    }
+
+    private KernelWakelockReader mReader;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        mReader = new KernelWakelockReader();
+    }
+
+    @SmallTest
+    public void testParseEmptyFile() throws Exception {
+        KernelWakelockStats staleStats = mReader.parseProcWakelocks(new byte[0], 0, true,
+                new KernelWakelockStats());
+        assertTrue(staleStats.isEmpty());
+    }
+
+    @SmallTest
+    public void testOnlyHeader() throws Exception {
+        byte[] buffer = new ProcFileBuilder().getBytes();
+        KernelWakelockStats staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true,
+                new KernelWakelockStats());
+        assertTrue(staleStats.isEmpty());
+    }
+
+    @SmallTest
+    public void testOneWakelock() throws Exception {
+        byte[] buffer = new ProcFileBuilder()
+                .addLine("Wakelock", 34, 123) // Milliseconds
+                .getBytes();
+        KernelWakelockStats staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true,
+                new KernelWakelockStats());
+        assertEquals(1, staleStats.size());
+        assertTrue(staleStats.containsKey("Wakelock"));
+
+        KernelWakelockStats.Entry entry = staleStats.get("Wakelock");
+        assertEquals(34, entry.mCount);
+        assertEquals(123 * 1000, entry.mTotalTime); // Microseconds
+    }
+
+    @SmallTest
+    public void testTwoWakelocks() throws Exception {
+        byte[] buffer = new ProcFileBuilder()
+                .addLine("Wakelock", 1, 10)
+                .addLine("Fakelock", 2, 20)
+                .getBytes();
+        KernelWakelockStats staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true,
+                new KernelWakelockStats());
+        assertEquals(2, staleStats.size());
+        assertTrue(staleStats.containsKey("Wakelock"));
+        assertTrue(staleStats.containsKey("Fakelock"));
+    }
+
+    @SmallTest
+    public void testDuplicateWakelocksAccumulate() throws Exception {
+        byte[] buffer = new ProcFileBuilder()
+                .addLine("Wakelock", 1, 10) // Milliseconds
+                .addLine("Wakelock", 1, 10) // Milliseconds
+                .getBytes();
+        KernelWakelockStats staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true,
+                new KernelWakelockStats());
+        assertEquals(1, staleStats.size());
+        assertTrue(staleStats.containsKey("Wakelock"));
+
+        KernelWakelockStats.Entry entry = staleStats.get("Wakelock");
+        assertEquals(2, entry.mCount);
+        assertEquals(20 * 1000, entry.mTotalTime); // Microseconds
+    }
+
+    @SmallTest
+    public void testWakelocksBecomeStale() throws Exception {
+        byte[] buffer = new ProcFileBuilder()
+                .addLine("Fakelock", 3, 30)
+                .getBytes();
+        KernelWakelockStats staleStats = new KernelWakelockStats();
+
+        staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true, staleStats);
+        assertEquals(1, staleStats.size());
+        assertTrue(staleStats.containsKey("Fakelock"));
+
+        buffer = new ProcFileBuilder()
+                .addLine("Wakelock", 1, 10)
+                .getBytes();
+
+        staleStats = mReader.parseProcWakelocks(buffer, buffer.length, true, staleStats);
+        assertEquals(1, staleStats.size());
+        assertTrue(staleStats.containsKey("Wakelock"));
+        assertFalse(staleStats.containsKey("Fakelock"));
+    }
+}