diff options
3 files changed, 128 insertions, 15 deletions
diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index e4dec563f580..3f87de2e0f8a 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/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); - getProcStateScreenOffTimeCounter().setState(procState, elapsedTimeMs); + getProcStateTimeCounter(elapsedTimeMs).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() { - ensureMultiStateCounters(); + private TimeInFreqMultiStateCounter getProcStateTimeCounter(long timestampMs) { + ensureMultiStateCounters(timestampMs); return mProcStateTimeMs; } @GuardedBy("mBsi") - private TimeInFreqMultiStateCounter getProcStateScreenOffTimeCounter() { - ensureMultiStateCounters(); + private TimeInFreqMultiStateCounter getProcStateScreenOffTimeCounter(long timestampMs) { + 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); diff --git a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java index 81c6ee71e060..09e409bd934e 100644 --- a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java +++ b/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) { diff --git a/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java b/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java index 8540adb96171..e717ebbc997c 100644 --- a/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java +++ b/core/tests/coretests/src/com/android/internal/os/LongMultiStateCounterTest.java @@ -75,6 +75,73 @@ public class LongMultiStateCounterTest { } @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); counter.setState(0, 1000); |