Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit efe2cbf1 authored by Dmitri Plotnikov's avatar Dmitri Plotnikov
Browse files

Fix concurrency and logic issues in BatteryUsageStats throttling logic 

Bug: 403677569
Test: atest --rerun-until-failure 100 PowerStatsTests
Flag: EXEMPT bug fix
Change-Id: I984eb5115a7ff4a4cdfd450786a46ff5d9ca9735
parent 52f8f787

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

+4 −1
Original line number Diff line number Diff line
@@ -362,6 +362,7 @@ public class BatteryStatsHistory { 
    // Monotonic time when the last event was written to the history buffer

    private long mHistoryMonotonicEndTime;

    // Monotonically increasing size of written history

    @GuardedBy("this")

    private long mMonotonicHistorySize;

    private final ArraySet<PowerStats.Descriptor> mWrittenPowerStatsDescriptors = new ArraySet<>();

    private boolean mMutable = true;
@@ -2357,8 +2358,10 @@ public class BatteryStatsHistory { 
     * that have already been discarded.

     */

    public long getMonotonicHistorySize() {

        synchronized (this) {

            return mMonotonicHistorySize;

        }

    }



    /**

     * Prints battery stats history for debugging.

services/core/java/com/android/server/power/stats/BatteryUsageStatsProvider.java

+2 −2
Original line number Diff line number Diff line
@@ -192,8 +192,8 @@ public class BatteryUsageStatsProvider { 
    public void accumulateBatteryUsageStatsAsync(BatteryStatsImpl stats, Handler handler) {

        synchronized (this) {

            long historySize = stats.getHistory().getMonotonicHistorySize();

            if (historySize - mLastAccumulationMonotonicHistorySize

                    < mAccumulatedBatteryUsageStatsSpanSize) {

            long delta = historySize - mLastAccumulationMonotonicHistorySize;

            if (delta >= 0 && delta < mAccumulatedBatteryUsageStatsSpanSize) {

                return;

            }

            mLastAccumulationMonotonicHistorySize = historySize;

services/core/java/com/android/server/power/stats/processor/BinaryStatePowerStatsProcessor.java

+25 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.power.stats.processor;



import android.annotation.IntDef;

import android.os.BatteryConsumer;

import android.os.BatteryStats;

import android.os.PersistableBundle;

import android.os.Process;
@@ -44,10 +45,12 @@ abstract class BinaryStatePowerStatsProcessor extends PowerStatsProcessor { 
    }



    private final int mPowerComponentId;

    private final String mPowerComponentName;

    private final UsageBasedPowerEstimator mUsageBasedPowerEstimator;

    private boolean mEnergyConsumerSupported;

    private int mInitiatingUid = Process.INVALID_UID;

    private @BinaryState int mLastState = STATE_OFF;

    private boolean mLastStateKnown;

    private long mLastStateTimestamp;

    private long mLastUpdateTimestamp;
@@ -65,6 +68,7 @@ abstract class BinaryStatePowerStatsProcessor extends PowerStatsProcessor { 
    BinaryStatePowerStatsProcessor(int powerComponentId, double averagePowerMilliAmp,

            BinaryStatePowerStatsLayout statsLayout) {

        mPowerComponentId = powerComponentId;

        mPowerComponentName = BatteryConsumer.powerComponentIdToString(powerComponentId);

        mUsageBasedPowerEstimator = new UsageBasedPowerEstimator(averagePowerMilliAmp);

        mStatsLayout = statsLayout;

    }
@@ -93,6 +97,7 @@ abstract class BinaryStatePowerStatsProcessor extends PowerStatsProcessor { 


        // Establish a baseline at the beginning of an accumulation pass

        mLastState = STATE_OFF;

        mLastStateKnown = false;

        mLastStateTimestamp = timestampMs;

        mInitiatingUid = Process.INVALID_UID;

        flushPowerStats(stats, mLastStateTimestamp);
@@ -102,6 +107,26 @@ abstract class BinaryStatePowerStatsProcessor extends PowerStatsProcessor { 
    void noteStateChange(PowerComponentAggregatedPowerStats stats,

            BatteryStats.HistoryItem item) {

        @BinaryState int state = getBinaryState(item);



        if (!mLastStateKnown) {

            // If the aggregation span starts with this power component in the ON state,

            // we can sometimes recognize that fact by detecting an explicit "OFF" event

            // in history. This is not a 100% bullet-proof solution. We should really preserve

            // the initial states for all power components between aggregation sessions,

            // TODO(b/316044609): preserve state between aggregation sessions.

            if ((item.eventCode & BatteryStats.HistoryItem.EVENT_TYPE_MASK)

                    == BatteryStats.HistoryItem.EVENT_STATE_CHANGE

                    && item.eventTag.string.equals(mPowerComponentName)) {

                // The component got turned OFF, which means that it had been ON since the start

                if (state == STATE_OFF) {

                    mLastState = STATE_ON;

                }

            } else {

                mLastState = state;

            }

            mLastStateKnown = true;

        }



        if (state == mLastState) {

            return;

        }

services/core/java/com/android/server/power/stats/processor/PowerStatsAggregator.java

+5 −4
Original line number Diff line number Diff line
@@ -97,7 +97,7 @@ public class PowerStatsAggregator { 
                    BatteryStats.HistoryItem item = iterator.next();



                    if (!startedSession) {

                        mStats.start(item.time);

                        mStats.start(startTimeMs > 0 ? startTimeMs : item.time);

                        if (!mStats.addClockUpdate(item.time, item.currentTime)) {

                            break;

                        }
@@ -114,8 +114,7 @@ public class PowerStatsAggregator { 


                    lastTime = item.time;



                    if (item.cmd == BatteryStats.HistoryItem.CMD_UPDATE

                            && item.batteryLevel != lastBatteryLevel) {

                    if (item.batteryLevel != lastBatteryLevel && item.batteryLevel != 0) {

                        mStats.noteBatteryLevel(item.batteryLevel, item.batteryChargeUah,

                                item.time);

                        lastBatteryLevel = item.batteryLevel;
@@ -146,7 +145,9 @@ public class PowerStatsAggregator { 
                            != lastStates

                            || (item.states2

                            & BatteryStats.HistoryItem.IMPORTANT_FOR_POWER_STATS_STATES2)

                            != lastStates2) {

                            != lastStates2

                            || (item.eventCode & BatteryStats.HistoryItem.EVENT_TYPE_MASK)

                            == BatteryStats.HistoryItem.EVENT_STATE_CHANGE) {

                        mStats.noteStateChange(item);

                        lastStates = item.states

                                & BatteryStats.HistoryItem.IMPORTANT_FOR_POWER_STATS_STATES;

services/tests/powerstatstests/src/com/android/server/power/stats/BatteryUsageStatsProviderTest.java

+22 −8
Original line number Diff line number Diff line
@@ -575,7 +575,7 @@ public class BatteryUsageStatsProviderTest { 
        accumulateBatteryUsageStats(batteryStats, 10000000, 0);

        // Accumulate every 200 bytes of battery history

        accumulateBatteryUsageStats(batteryStats, 200, 1);

        accumulateBatteryUsageStats(batteryStats, 50, 4);

        accumulateBatteryUsageStats(batteryStats, 50, 5);

        // Accumulate on every invocation of accumulateBatteryUsageStats

        accumulateBatteryUsageStats(batteryStats, 0, 7);

    }
@@ -597,7 +597,7 @@ public class BatteryUsageStatsProviderTest { 
                        .build());



        assertThat(stats.getStatsStartTimestamp()).isEqualTo(5 * MINUTE_IN_MS);

        assertThat(stats.getStatsEndTimestamp()).isEqualTo(30 * MINUTE_IN_MS);

        assertThat(stats.getStatsEndTimestamp()).isEqualTo(25 * MINUTE_IN_MS);

        assertBatteryConsumer(stats, 60.0, 10 * MINUTE_IN_MS);

        assertBatteryConsumer(stats, APP_UID, 60.0, 10 * MINUTE_IN_MS);
@@ -649,8 +649,12 @@ public class BatteryUsageStatsProviderTest { 
        BatteryUsageStatsProvider batteryUsageStatsProvider = createBatteryUsageStatsProvider(

                accumulatedBatteryUsageStatsSpanSize);



        // This produces zero accumulated power, because we haven't recorded any event yet.

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);



        // Make sure the accumulated stats are computed and saved before generating more history

        mStatsRule.waitForBackgroundThread();



        setTime(10 * MINUTE_IN_MS);

        synchronized (batteryStats) {

            batteryStats.setBatteryStateLocked(BatteryManager.BATTERY_STATUS_DISCHARGING, 100,
@@ -660,10 +664,14 @@ public class BatteryUsageStatsProviderTest { 
                    10 * MINUTE_IN_MS, 10 * MINUTE_IN_MS);

        }



        setTime(15 * MINUTE_IN_MS);

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);

        mStatsRule.waitForBackgroundThread();



        setTime(20 * MINUTE_IN_MS);

        synchronized (batteryStats) {

            // Write an event that recaptures the last known battery level.

            batteryStats.noteWakeUpLocked("test", 0, 20 * MINUTE_IN_MS, 20 * MINUTE_IN_MS);

            batteryStats.setBatteryStateLocked(BatteryManager.BATTERY_STATUS_DISCHARGING, 100,

                    /* plugType */ 0, 85, 72, 3700, 3_000_000, 4_000_000, 0,

                    20 * MINUTE_IN_MS, 20 * MINUTE_IN_MS, 20 * MINUTE_IN_MS);
@@ -671,7 +679,12 @@ public class BatteryUsageStatsProviderTest { 
                    20 * MINUTE_IN_MS, 20 * MINUTE_IN_MS);

        }



        setTime(25 * MINUTE_IN_MS);



        // The battery charge has dropped from 3_600_000 to 3_000_000, so at this point we should

        // see 600 mAh of total drain

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);

        mStatsRule.waitForBackgroundThread();



        setTime(30 * MINUTE_IN_MS);

        synchronized (batteryStats) {
@@ -679,22 +692,23 @@ public class BatteryUsageStatsProviderTest { 
                    30 * MINUTE_IN_MS, 30 * MINUTE_IN_MS);

        }



        setTime(35 * MINUTE_IN_MS);

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);



        // Make sure the accumulated stats are computed and saved before generating more history

        mStatsRule.waitForBackgroundThread();



        setTime(50 * MINUTE_IN_MS);

        synchronized (batteryStats) {

            batteryStats.noteFlashlightOffLocked(APP_UID,

                    50 * MINUTE_IN_MS, 50 * MINUTE_IN_MS);

            batteryStats.setBatteryStateLocked(BatteryManager.BATTERY_STATUS_DISCHARGING, 100,

                    /* plugType */ 0, 80, 72, 3700, 2_400_000, 4_000_000, 0,

                    50 * MINUTE_IN_MS, 50 * MINUTE_IN_MS, 50 * MINUTE_IN_MS);

            batteryStats.noteFlashlightOffLocked(APP_UID,

                    50 * MINUTE_IN_MS, 50 * MINUTE_IN_MS);

        }

        setTime(55 * MINUTE_IN_MS);



        // Battery discharged another 600 mAh

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);

        mStatsRule.waitForBackgroundThread();



        // This section has not been saved yet, but should be added to the accumulated totals

        setTime(80 * MINUTE_IN_MS);
@@ -703,7 +717,9 @@ public class BatteryUsageStatsProviderTest { 
                    80 * MINUTE_IN_MS, 80 * MINUTE_IN_MS);

        }



        setTime(85 * MINUTE_IN_MS);

        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);

        mStatsRule.waitForBackgroundThread();



        setTime(110 * MINUTE_IN_MS);

        synchronized (batteryStats) {
@@ -711,9 +727,7 @@ public class BatteryUsageStatsProviderTest { 
                    110 * MINUTE_IN_MS, 110 * MINUTE_IN_MS);

        }

        setTime(115 * MINUTE_IN_MS);



        batteryUsageStatsProvider.accumulateBatteryUsageStatsAsync(batteryStats, handler);



        mStatsRule.waitForBackgroundThread();



        BatteryUsageStats stats = batteryUsageStatsProvider.getBatteryUsageStats(batteryStats,