Merge "Address proportional counting for out-of-order timestamps" into tm-dev am: 860fe90fbd am: 892520724e am: d945a44841

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/18062923

Change-Id: I692c28411fdf00d57d4f1315bae7ff9b75bfe131
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

@@ -472,9 +472,9 @@ public class BatteryStatsImpl extends BatteryStats {
         mNumSingleUidCpuTimeReads++;
 
         LongArrayMultiStateCounter onBatteryCounter =
-                u.getProcStateTimeCounter().getCounter();
+                u.getProcStateTimeCounter(timestampMs).getCounter();
         LongArrayMultiStateCounter onBatteryScreenOffCounter =
-                u.getProcStateScreenOffTimeCounter().getCounter();
+                u.getProcStateScreenOffTimeCounter(timestampMs).getCounter();
 
         mKernelSingleUidTimeReader.addDelta(uid, onBatteryCounter, timestampMs);
         mKernelSingleUidTimeReader.addDelta(uid, onBatteryScreenOffCounter, timestampMs);
@@ -543,9 +543,9 @@ public class BatteryStatsImpl extends BatteryStats {
 
                 final long timestampMs = mClock.elapsedRealtime();
                 final LongArrayMultiStateCounter onBatteryCounter =
-                        u.getProcStateTimeCounter().getCounter();
+                        u.getProcStateTimeCounter(timestampMs).getCounter();
                 final LongArrayMultiStateCounter onBatteryScreenOffCounter =
-                        u.getProcStateScreenOffTimeCounter().getCounter();
+                        u.getProcStateScreenOffTimeCounter(timestampMs).getCounter();
 
                 if (uid == parentUid || Process.isSdkSandboxUid(uid)) {
                     mKernelSingleUidTimeReader.addDelta(parentUid, onBatteryCounter, timestampMs);
@@ -8935,8 +8935,8 @@ public class BatteryStatsImpl extends BatteryStats {
         @VisibleForTesting
         public void setProcessStateForTest(int procState, long elapsedTimeMs) {
             mProcessState = procState;
-            getProcStateTimeCounter().setState(procState, elapsedTimeMs);
+            getProcStateTimeCounter(elapsedTimeMs).setState(procState, elapsedTimeMs);
-            getProcStateScreenOffTimeCounter().setState(procState, elapsedTimeMs);
+            getProcStateScreenOffTimeCounter(elapsedTimeMs).setState(procState, elapsedTimeMs);
             final int batteryConsumerProcessState =
                     mapUidProcessStateToBatteryConsumerProcessState(procState);
             getCpuActiveTimeCounter().setState(batteryConsumerProcessState, elapsedTimeMs);
@@ -9095,12 +9095,11 @@ public class BatteryStatsImpl extends BatteryStats {
         }
 
         @GuardedBy("mBsi")
-        private void ensureMultiStateCounters() {
+        private void ensureMultiStateCounters(long timestampMs) {
             if (mProcStateTimeMs != null) {
                 return;
             }
 
-            final long timestampMs = mBsi.mClock.elapsedRealtime();
             mProcStateTimeMs =
                     new TimeInFreqMultiStateCounter(mBsi.mOnBatteryTimeBase,
                             PROC_STATE_TIME_COUNTER_STATE_COUNT, mBsi.getCpuFreqCount(),
@@ -9112,14 +9111,14 @@ public class BatteryStatsImpl extends BatteryStats {
         }
 
         @GuardedBy("mBsi")
-        private TimeInFreqMultiStateCounter getProcStateTimeCounter() {
+        private TimeInFreqMultiStateCounter getProcStateTimeCounter(long timestampMs) {
-            ensureMultiStateCounters();
+            ensureMultiStateCounters(timestampMs);
             return mProcStateTimeMs;
         }
 
         @GuardedBy("mBsi")
-        private TimeInFreqMultiStateCounter getProcStateScreenOffTimeCounter() {
+        private TimeInFreqMultiStateCounter getProcStateScreenOffTimeCounter(long timestampMs) {
-            ensureMultiStateCounters();
+            ensureMultiStateCounters(timestampMs);
             return mProcStateScreenOffTimeMs;
         }
 
@@ -11972,9 +11971,9 @@ public class BatteryStatsImpl extends BatteryStats {
                     mBsi.updateProcStateCpuTimesLocked(mUid, elapsedRealtimeMs);
 
                     LongArrayMultiStateCounter onBatteryCounter =
-                            getProcStateTimeCounter().getCounter();
+                            getProcStateTimeCounter(elapsedRealtimeMs).getCounter();
                     LongArrayMultiStateCounter onBatteryScreenOffCounter =
-                            getProcStateScreenOffTimeCounter().getCounter();
+                            getProcStateScreenOffTimeCounter(elapsedRealtimeMs).getCounter();
 
                     onBatteryCounter.setState(uidRunningState, elapsedRealtimeMs);
                     onBatteryScreenOffCounter.setState(uidRunningState, elapsedRealtimeMs);

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

@@ -18,6 +18,7 @@ package com.android.internal.os;
 
 import android.content.Context;
 import android.hardware.SensorManager;
+import android.os.BatteryConsumer;
 import android.os.BatteryStats;
 import android.os.BatteryUsageStats;
 import android.os.BatteryUsageStatsQuery;
@@ -26,6 +27,7 @@ import android.os.Process;
 import android.os.SystemClock;
 import android.os.UidBatteryConsumer;
 import android.util.Log;
+import android.util.Slog;
 import android.util.SparseArray;
 
 import com.android.internal.annotations.GuardedBy;
@@ -230,7 +232,52 @@ public class BatteryUsageStatsProvider {
             batteryUsageStatsBuilder.setBatteryHistory(batteryStatsHistory);
         }
 
-        return batteryUsageStatsBuilder.build();
+        BatteryUsageStats stats = batteryUsageStatsBuilder.build();
+        if (includeProcessStateData) {
+            verify(stats);
+        }
+        return stats;
+    }
+
+    // STOPSHIP(b/229906525): remove verification before shipping
+    private static boolean sErrorReported;
+    private void verify(BatteryUsageStats stats) {
+        if (sErrorReported) {
+            return;
+        }
+
+        final double precision = 2.0;   // Allow rounding errors up to 2 mAh
+        final int[] components =
+                {BatteryConsumer.POWER_COMPONENT_CPU,
+                        BatteryConsumer.POWER_COMPONENT_MOBILE_RADIO,
+                        BatteryConsumer.POWER_COMPONENT_WIFI,
+                        BatteryConsumer.POWER_COMPONENT_BLUETOOTH};
+        final int[] states =
+                {BatteryConsumer.PROCESS_STATE_FOREGROUND,
+                        BatteryConsumer.PROCESS_STATE_BACKGROUND,
+                        BatteryConsumer.PROCESS_STATE_FOREGROUND_SERVICE,
+                        BatteryConsumer.PROCESS_STATE_CACHED};
+        for (UidBatteryConsumer ubc : stats.getUidBatteryConsumers()) {
+            for (int component : components) {
+                double consumedPower = ubc.getConsumedPower(ubc.getKey(component));
+                double sumStates = 0;
+                for (int state : states) {
+                    sumStates += ubc.getConsumedPower(ubc.getKey(component, state));
+                }
+                if (sumStates > consumedPower + precision) {
+                    String error = "Sum of states exceeds total. UID = " + ubc.getUid() + " "
+                            + BatteryConsumer.powerComponentIdToString(component)
+                            + " total = " + consumedPower + " states = " + sumStates;
+                    if (!sErrorReported) {
+                        Slog.wtf(TAG, error);
+                        sErrorReported = true;
+                    } else {
+                        Slog.e(TAG, error);
+                    }
+                    return;
+                }
+            }
+        }
     }
 
     private long getProcessForegroundTimeMs(BatteryStats.Uid uid, long realtimeUs) {

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

@@ -74,6 +74,73 @@ public class LongMultiStateCounterTest {
         assertThat(counter.getCount(0)).isEqualTo(175);
     }
 
+    @Test
+    public void updateThenSetState_timestampOutOfOrder() {
+        LongMultiStateCounter counter = new LongMultiStateCounter(2);
+        counter.setState(0, 1000);
+        counter.updateValue(0, 1000);
+        counter.updateValue(100, 3000);
+        counter.setState(0, 2000);  // Note out-of-order timestamp
+        counter.updateValue(200, 4000);
+
+        // If we did not explicitly handle this out-of-order update scenario, we would get
+        // this result:
+        //  1. Time in state-0 at this point is (4000-2000) = 2000
+        //  2. Time since last update is (4000-3000) = 1000.
+        //  3. Counter delta: 100
+        //  4. Proportion of count-0
+        //          = prevValue + delta * time-in-state / time-since-last-update
+        //          = 100 + 100 * 2000 / 1000
+        //          = 300
+        // This would be problematic, because the part (300) would exceed the total (200)
+        assertThat(counter.getCount(0)).isEqualTo(200);
+    }
+
+    @Test
+    public void disableThenUpdate_timestampOutOfOrder() {
+        LongMultiStateCounter counter = new LongMultiStateCounter(2);
+        counter.setState(0, 1000);
+        counter.updateValue(0, 1000);
+        counter.setEnabled(false, 2000);
+        counter.updateValue(123, 1001);  // Note out-of-order timestamp
+
+        // If we did not explicitly handle this out-of-order update scenario, we would get
+        // this result:
+        //  1. Time in state-0 at this point is (2000-1000) = 1000
+        //  2. Time since last update is (1001-1000) = 1.
+        //  3. Counter delta: 100
+        //  4. Proportion of count-0
+        //          = delta * time-in-state / time-since-last-update
+        //          = 123 * 1000 / 1
+        //          = 123,000
+        // This would be very very wrong, because the part (123,000) would exceed the total (123)
+        assertThat(counter.getCount(0)).isEqualTo(123);
+    }
+
+    @Test
+    public void updateThenEnable_timestampOutOfOrder() {
+        LongMultiStateCounter counter = new LongMultiStateCounter(2);
+        counter.setState(0, 1000);
+        counter.updateValue(0, 1000);
+        counter.setEnabled(false, 3000);
+        counter.updateValue(100, 5000);
+        // At this point the counter is 50, because it was disabled for half of the time
+        counter.setEnabled(true, 4000); // Note out-of-order timestamp
+        counter.updateValue(200, 6000);
+
+        // If we did not explicitly handle this out-of-order update scenario, we would get
+        // this result:
+        //  1. Time in state-0 at this point is (6000-4000) = 2000
+        //  2. Time since last update is (6000-5000) = 1000.
+        //  3. Counter delta: 100
+        //  4. Proportion of count-0
+        //          = prevValue + delta * time-in-state / time-since-last-update
+        //          = 50 + 100 * 2000 / 1000
+        //          = 250
+        // This would not be great, because the part (250) would exceed the total (200)
+        assertThat(counter.getCount(0)).isEqualTo(150);
+    }
+
     @Test
     public void reset() {
         LongMultiStateCounter counter = new LongMultiStateCounter(2);