From 613096a244af13712bffe97a60393cd82b59c36b Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Thu, 2 Mar 2023 23:57:40 +0000 Subject: Revert "Reset BatteryStats when device has been plugged in for a..." Revert submission 21485850 Reason for revert: introduced deadlock Reverted changes: /q/submissionid:21485850 Change-Id: Ib8dfc28154e9004e48f5537aef677b77724072b6 (cherry picked from commit 806d71bf7e2b833d6185527c7177057aae680d54) Merged-In: Ib8dfc28154e9004e48f5537aef677b77724072b6 --- .../com/android/internal/os/BatteryStatsImpl.java | 115 +-------------------- .../android/internal/os/BatteryStatsResetTest.java | 64 ------------ .../internal/os/BatteryUsageStatsProviderTest.java | 8 +- .../internal/os/BatteryUsageStatsStoreTest.java | 6 +- .../com/android/server/am/BatteryStatsService.java | 45 +------- 5 files changed, 11 insertions(+), 227 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 3a0e09d1fc77..49537549a988 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -81,7 +81,6 @@ import android.telephony.ServiceState.RegState; import android.telephony.SignalStrength; import android.telephony.TelephonyManager; import android.text.TextUtils; -import android.text.format.DateUtils; import android.util.ArrayMap; import android.util.ArraySet; import android.util.AtomicFile; @@ -202,7 +201,6 @@ public class BatteryStatsImpl extends BatteryStats { public static final int RESET_REASON_ADB_COMMAND = 2; public static final int RESET_REASON_FULL_CHARGE = 3; public static final int RESET_REASON_MEASURED_ENERGY_BUCKETS_CHANGE = 4; - public static final int RESET_REASON_PLUGGED_IN_FOR_LONG_DURATION = 5; protected Clock mClock; @@ -464,14 +462,6 @@ public class BatteryStatsImpl extends BatteryStats { } - /** Handles calls to AlarmManager */ - public interface AlarmInterface { - /** Schedule an RTC alarm */ - void schedule(long rtcTimeMs, long windowLengthMs); - /** Cancel the previously scheduled alarm */ - void cancel(); - } - private final PlatformIdleStateCallback mPlatformIdleStateCallback; private final Runnable mDeferSetCharging = new Runnable() { @@ -802,7 +792,6 @@ public class BatteryStatsImpl extends BatteryStats { protected boolean mHaveBatteryLevel = false; protected boolean mRecordingHistory = false; int mNumHistoryItems; - private long mBatteryPluggedInRealTimeMs = 0; private static final int HISTORY_TAG_INDEX_LIMIT = 0x7ffe; private static final int MAX_HISTORY_TAG_STRING_LENGTH = 1024; @@ -1549,9 +1538,6 @@ public class BatteryStatsImpl extends BatteryStats { @GuardedBy("this") protected BatteryStatsConfig mBatteryStatsConfig = new BatteryStatsConfig.Builder().build(); - @VisibleForTesting - protected AlarmInterface mLongPlugInAlarmInterface = null; - /* * Holds a SamplingTimer associated with each Resource Power Manager state and voter, * recording their times when on-battery (regardless of screen state). @@ -12650,18 +12636,6 @@ public class BatteryStatsImpl extends BatteryStats { } } - /** - * Injects a LongPlugInAlarmHandler - */ - public void setLongPlugInAlarmInterface(AlarmInterface longPlugInAlarmInterface) { - synchronized (this) { - mLongPlugInAlarmInterface = longPlugInAlarmInterface; - if (!mOnBattery) { - scheduleNextResetWhilePluggedInCheck(); - } - } - } - /** * Starts tracking CPU time-in-state for threads of the system server process, * keeping a separate account of threads receiving incoming binder calls. @@ -13134,12 +13108,12 @@ public class BatteryStatsImpl extends BatteryStats { } @GuardedBy("this") - public void resetAllStatsAndHistoryLocked(int reason) { + public void resetAllStatsCmdLocked() { final long mSecUptime = mClock.uptimeMillis(); long uptimeUs = mSecUptime * 1000; long mSecRealtime = mClock.elapsedRealtime(); long realtimeUs = mSecRealtime * 1000; - resetAllStatsLocked(mSecUptime, mSecRealtime, reason); + resetAllStatsLocked(mSecUptime, mSecRealtime, RESET_REASON_ADB_COMMAND); mDischargeStartLevel = mHistoryCur.batteryLevel; pullPendingStateUpdatesLocked(); addHistoryRecordLocked(mSecRealtime, mSecUptime); @@ -15612,73 +15586,6 @@ public class BatteryStatsImpl extends BatteryStats { return false; } - /** - * Might reset battery stats if conditions are met. Assumed the device is currently plugged in. - */ - @GuardedBy("this") - public void maybeResetWhilePluggedInLocked() { - final long elapsedRealtimeMs = mClock.elapsedRealtime(); - if (shouldResetWhilePluggedInLocked(elapsedRealtimeMs)) { - Slog.i(TAG, - "Resetting due to long plug in duration. elapsed time = " + elapsedRealtimeMs - + " ms, last plug in time = " + mBatteryPluggedInRealTimeMs - + " ms, last reset time = " + mRealtimeStartUs / 1000); - resetAllStatsAndHistoryLocked(RESET_REASON_PLUGGED_IN_FOR_LONG_DURATION); - } - - scheduleNextResetWhilePluggedInCheck(); - } - - @GuardedBy("this") - private void scheduleNextResetWhilePluggedInCheck() { - if (mLongPlugInAlarmInterface != null) { - final long timeoutMs = mClock.currentTimeMillis() - + mConstants.RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS - * DateUtils.HOUR_IN_MILLIS; - Calendar nextAlarm = Calendar.getInstance(); - nextAlarm.setTimeInMillis(timeoutMs); - - // Find the 2 AM the same day as the end of the minimum duration. - // This logic does not handle a Daylight Savings transition, or a timezone change - // while the alarm has been set. The need to reset after a long period while plugged - // in is not strict enough to warrant a well architected out solution. - nextAlarm.set(Calendar.MILLISECOND, 0); - nextAlarm.set(Calendar.SECOND, 0); - nextAlarm.set(Calendar.MINUTE, 0); - nextAlarm.set(Calendar.HOUR_OF_DAY, 2); - long nextTimeMs = nextAlarm.getTimeInMillis(); - if (nextTimeMs < timeoutMs) { - // The 2AM on the day of the timeout, move on the next day. - nextTimeMs += DateUtils.DAY_IN_MILLIS; - } - mLongPlugInAlarmInterface.schedule(nextTimeMs, DateUtils.HOUR_IN_MILLIS); - } - } - - - @GuardedBy("this") - private boolean shouldResetWhilePluggedInLocked(long elapsedRealtimeMs) { - if (mNoAutoReset) return false; - if (!mSystemReady) return false; - - final long pluggedInThresholdMs = mBatteryPluggedInRealTimeMs - + mConstants.RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS - * DateUtils.HOUR_IN_MILLIS; - if (elapsedRealtimeMs >= pluggedInThresholdMs) { - // The device has been plugged in for a long time. - final long resetThresholdMs = mRealtimeStartUs / 1000 - + mConstants.RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS - * DateUtils.HOUR_IN_MILLIS; - if (elapsedRealtimeMs >= resetThresholdMs) { - // And it has been a long time since the last reset. - return true; - } - } - - return false; - } - - /** * Notifies BatteryStatsImpl that the system server is ready. */ @@ -15784,9 +15691,6 @@ public class BatteryStatsImpl extends BatteryStats { mInitStepMode = mCurStepMode; mModStepMode = 0; pullPendingStateUpdatesLocked(); - if (mLongPlugInAlarmInterface != null) { - mLongPlugInAlarmInterface.cancel(); - } mHistoryCur.batteryLevel = (byte)level; mHistoryCur.states &= ~HistoryItem.STATE_BATTERY_PLUGGED_FLAG; if (DEBUG_HISTORY) Slog.v(TAG, "Battery unplugged to: " @@ -15818,7 +15722,6 @@ public class BatteryStatsImpl extends BatteryStats { mLastChargingStateLevel = level; mOnBattery = mOnBatteryInternal = false; pullPendingStateUpdatesLocked(); - mBatteryPluggedInRealTimeMs = mSecRealtime; mHistoryCur.batteryLevel = (byte)level; mHistoryCur.states |= HistoryItem.STATE_BATTERY_PLUGGED_FLAG; if (DEBUG_HISTORY) Slog.v(TAG, "Battery plugged to: " @@ -15836,7 +15739,6 @@ public class BatteryStatsImpl extends BatteryStats { mMaxChargeStepLevel = level; mInitStepMode = mCurStepMode; mModStepMode = 0; - scheduleNextResetWhilePluggedInCheck(); } if (doWrite || (mLastWriteTimeMs + (60 * 1000)) < mSecRealtime) { if (mStatsFile != null && mBatteryStatsHistory.getActiveFile() != null) { @@ -16851,8 +16753,6 @@ public class BatteryStatsImpl extends BatteryStats { public static final String KEY_MAX_HISTORY_BUFFER_KB = "max_history_buffer_kb"; public static final String KEY_BATTERY_CHARGED_DELAY_MS = "battery_charged_delay_ms"; - public static final String KEY_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS = - "reset_while_plugged_in_minimum_duration_hours"; private static final boolean DEFAULT_TRACK_CPU_ACTIVE_CLUSTER_TIME = true; private static final long DEFAULT_KERNEL_UID_READERS_THROTTLE_TIME = 1_000; @@ -16865,8 +16765,6 @@ public class BatteryStatsImpl extends BatteryStats { private static final int DEFAULT_MAX_HISTORY_FILES_LOW_RAM_DEVICE = 64; private static final int DEFAULT_MAX_HISTORY_BUFFER_LOW_RAM_DEVICE_KB = 64; /*Kilo Bytes*/ private static final int DEFAULT_BATTERY_CHARGED_DELAY_MS = 900000; /* 15 min */ - // Little less than 2 days - private static final int DEFAULT_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS = 47; public boolean TRACK_CPU_ACTIVE_CLUSTER_TIME = DEFAULT_TRACK_CPU_ACTIVE_CLUSTER_TIME; /* Do not set default value for KERNEL_UID_READERS_THROTTLE_TIME. Need to trigger an @@ -16882,8 +16780,6 @@ public class BatteryStatsImpl extends BatteryStats { public int MAX_HISTORY_FILES; public int MAX_HISTORY_BUFFER; /*Bytes*/ public int BATTERY_CHARGED_DELAY_MS = DEFAULT_BATTERY_CHARGED_DELAY_MS; - public int RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS = - DEFAULT_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS; private ContentResolver mResolver; private final KeyValueListParser mParser = new KeyValueListParser(','); @@ -16960,11 +16856,6 @@ public class BatteryStatsImpl extends BatteryStats { DEFAULT_MAX_HISTORY_BUFFER_LOW_RAM_DEVICE_KB : DEFAULT_MAX_HISTORY_BUFFER_KB) * 1024; - - RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS = mParser.getInt( - KEY_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS, - DEFAULT_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS); - updateBatteryChargedDelayMsLocked(); } } @@ -17019,8 +16910,6 @@ public class BatteryStatsImpl extends BatteryStats { pw.println(MAX_HISTORY_BUFFER/1024); pw.print(KEY_BATTERY_CHARGED_DELAY_MS); pw.print("="); pw.println(BATTERY_CHARGED_DELAY_MS); - pw.print(KEY_RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS); pw.print("="); - pw.println(RESET_WHILE_PLUGGED_IN_MINIMUM_DURATION_HOURS); } } diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java index 74878ca0d78d..fb61ec5efd81 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java @@ -245,70 +245,6 @@ public class BatteryStatsResetTest { assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); } - @Test - public void testResetWhilePluggedIn_longPlugIn() { - // disable high battery level reset on unplug. - mBatteryStatsImpl.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugHighBatteryLevel(false) - .setResetOnUnplugAfterSignificantCharge(false) - .build()); - long expectedResetTimeUs = 0; - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset should still not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset 47 hour threshold crossed, reset should occur. - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset another 47 hour threshold crossed, reset should occur. - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - unplugBattery(); - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset should not occur, since unplug occurred recently. - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Increment time a day - incTimeMs(24L * 60L * 60L * 1000L); - mBatteryStatsImpl.maybeResetWhilePluggedInLocked(); - // Reset another 47 hour threshold crossed, reset should occur. - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - } - private void dischargeToLevel(int targetLevel) { mBatteryStatus = BatteryManager.BATTERY_STATUS_DISCHARGING; for (int level = mBatteryLevel - 1; level >= targetLevel; level--) { diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java index ca19137b10a0..354b93709976 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsProviderTest.java @@ -320,7 +320,7 @@ public class BatteryUsageStatsProviderTest { Context context = InstrumentationRegistry.getContext(); BatteryStatsImpl batteryStats = mStatsRule.getBatteryStats(); mStatsRule.setCurrentTime(5 * MINUTE_IN_MS); - batteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + batteryStats.resetAllStatsCmdLocked(); BatteryUsageStatsStore batteryUsageStatsStore = new BatteryUsageStatsStore(context, batteryStats, new File(context.getCacheDir(), "BatteryUsageStatsProviderTest"), @@ -335,14 +335,14 @@ public class BatteryUsageStatsProviderTest { batteryStats.noteFlashlightOffLocked(APP_UID, 20 * MINUTE_IN_MS, 20 * MINUTE_IN_MS); mStatsRule.setCurrentTime(25 * MINUTE_IN_MS); - batteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + batteryStats.resetAllStatsCmdLocked(); batteryStats.noteFlashlightOnLocked(APP_UID, 30 * MINUTE_IN_MS, 30 * MINUTE_IN_MS); batteryStats.noteFlashlightOffLocked(APP_UID, 50 * MINUTE_IN_MS, 50 * MINUTE_IN_MS); mStatsRule.setCurrentTime(55 * MINUTE_IN_MS); - batteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + batteryStats.resetAllStatsCmdLocked(); // This section should be ignored because the timestamp is out or range batteryStats.noteFlashlightOnLocked(APP_UID, @@ -350,7 +350,7 @@ public class BatteryUsageStatsProviderTest { batteryStats.noteFlashlightOffLocked(APP_UID, 70 * MINUTE_IN_MS, 70 * MINUTE_IN_MS); mStatsRule.setCurrentTime(75 * MINUTE_IN_MS); - batteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + batteryStats.resetAllStatsCmdLocked(); // This section should be ignored because it represents the current stats session batteryStats.noteFlashlightOnLocked(APP_UID, diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java index 11b9047fab7f..c9729fab3b5e 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryUsageStatsStoreTest.java @@ -84,7 +84,7 @@ public class BatteryUsageStatsStoreTest { mMockClock.realtime = 1_000_000; mMockClock.uptime = 1_000_000; - mBatteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + mBatteryStats.resetAllStatsCmdLocked(); final long[] timestamps = mBatteryUsageStatsStore.listBatteryUsageStatsTimestamps(); assertThat(timestamps).hasLength(1); @@ -114,7 +114,7 @@ public class BatteryUsageStatsStoreTest { final int numberOfSnapshots = (int) (MAX_BATTERY_STATS_SNAPSHOT_STORAGE_BYTES / snapshotFileSize); for (int i = 0; i < numberOfSnapshots + 2; i++) { - mBatteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + mBatteryStats.resetAllStatsCmdLocked(); mMockClock.realtime += 10_000_000; mMockClock.uptime += 10_000_000; @@ -141,7 +141,7 @@ public class BatteryUsageStatsStoreTest { mMockClock.currentTime += 10_000_000; prepareBatteryStats(); - mBatteryStats.resetAllStatsAndHistoryLocked(BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + mBatteryStats.resetAllStatsCmdLocked(); } assertThat(getDirectorySize(mStoreDirectory)).isNotEqualTo(0); diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index 2f95716639f0..c971d9221218 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -21,7 +21,6 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; import static android.os.BatteryStats.POWER_DATA_UNAVAILABLE; import android.annotation.NonNull; -import android.app.AlarmManager; import android.app.StatsManager; import android.app.usage.NetworkStatsManager; import android.bluetooth.BluetoothActivityEnergyInfo; @@ -397,18 +396,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub Slog.e(TAG, "Could not register INetworkManagement event observer " + e); } - final AlarmManager am = mContext.getSystemService(AlarmManager.class); - mHandler.post(() -> { - synchronized (mStats) { - mStats.setLongPlugInAlarmInterface(new AlarmInterface(am, () -> { - synchronized (mStats) { - if (mStats.isOnBattery()) return; - mStats.maybeResetWhilePluggedInLocked(); - } - })); - } - }); - synchronized (mPowerStatsLock) { mPowerStatsInternal = LocalServices.getService(PowerStatsInternal.class); if (mPowerStatsInternal != null) { @@ -2282,32 +2269,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub } } - final class AlarmInterface implements BatteryStatsImpl.AlarmInterface, - AlarmManager.OnAlarmListener { - private AlarmManager mAm; - private Runnable mOnAlarm; - - AlarmInterface(AlarmManager am, Runnable onAlarm) { - mAm = am; - mOnAlarm = onAlarm; - } - - @Override - public void schedule(long rtcTimeMs, long windowLengthMs) { - mAm.setWindow(AlarmManager.RTC, rtcTimeMs, windowLengthMs, TAG, this, mHandler); - } - - @Override - public void cancel() { - mAm.cancel(this); - } - - @Override - public void onAlarm() { - mOnAlarm.run(); - } - } - private static native int nativeWaitWakeup(ByteBuffer outBuffer); private void dumpHelp(PrintWriter pw) { @@ -2494,8 +2455,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub } else if ("--reset-all".equals(arg)) { awaitCompletion(); synchronized (mStats) { - mStats.resetAllStatsAndHistoryLocked( - BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + mStats.resetAllStatsCmdLocked(); mBatteryUsageStatsStore.removeAllSnapshots(); pw.println("Battery stats and history reset."); noOutput = true; @@ -2503,8 +2463,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub } else if ("--reset".equals(arg)) { awaitCompletion(); synchronized (mStats) { - mStats.resetAllStatsAndHistoryLocked( - BatteryStatsImpl.RESET_REASON_ADB_COMMAND); + mStats.resetAllStatsCmdLocked(); pw.println("Battery stats reset."); noOutput = true; } -- cgit v1.2.3 From 20bcb0befb7ca1974d6add7fa12d2493d555f072 Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Thu, 2 Mar 2023 23:57:40 +0000 Subject: Revert "Add config to control BatteryStats reset logic" Revert submission 21485850 Reason for revert: introduced deadlock Reverted changes: /q/submissionid:21485850 Change-Id: Id3f920a130bc20d59abefc0a2f018873b08415ed (cherry picked from commit e01cead961206f75701326396435433b0301237f) Merged-In: Id3f920a130bc20d59abefc0a2f018873b08415ed (cherry picked from commit f70a3556ad8a6b5fd161773c699fe1c525908f1d) Merged-In: Id3f920a130bc20d59abefc0a2f018873b08415ed --- .../com/android/internal/os/BatteryStatsImpl.java | 134 ++-------- core/res/res/values/config.xml | 4 - core/res/res/values/symbols.xml | 2 - .../android/internal/os/BatteryStatsResetTest.java | 297 --------------------- .../com/android/server/am/BatteryStatsService.java | 10 - 5 files changed, 16 insertions(+), 431 deletions(-) delete mode 100644 core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 49537549a988..6fed26c4a81d 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -387,81 +387,6 @@ public class BatteryStatsImpl extends BatteryStats { } } - /** Provide BatteryStatsImpl configuration choices */ - public static class BatteryStatsConfig { - static final int RESET_ON_UNPLUG_HIGH_BATTERY_LEVEL_FLAG = 1 << 0; - static final int RESET_ON_UNPLUG_AFTER_SIGNIFICANT_CHARGE_FLAG = 1 << 1; - - private final int mFlags; - - private BatteryStatsConfig(Builder builder) { - int flags = 0; - if (builder.mResetOnUnplugHighBatteryLevel) { - flags |= RESET_ON_UNPLUG_HIGH_BATTERY_LEVEL_FLAG; - } - if (builder.mResetOnUnplugAfterSignificantCharge) { - flags |= RESET_ON_UNPLUG_AFTER_SIGNIFICANT_CHARGE_FLAG; - } - mFlags = flags; - } - - /** - * Returns whether a BatteryStats reset should occur on unplug when the battery level is - * high. - */ - boolean shouldResetOnUnplugHighBatteryLevel() { - return (mFlags & RESET_ON_UNPLUG_HIGH_BATTERY_LEVEL_FLAG) - == RESET_ON_UNPLUG_HIGH_BATTERY_LEVEL_FLAG; - } - - /** - * Returns whether a BatteryStats reset should occur on unplug if the battery charge a - * significant amount since it has been plugged in. - */ - boolean shouldResetOnUnplugAfterSignificantCharge() { - return (mFlags & RESET_ON_UNPLUG_AFTER_SIGNIFICANT_CHARGE_FLAG) - == RESET_ON_UNPLUG_AFTER_SIGNIFICANT_CHARGE_FLAG; - } - - /** - * Builder for BatteryStatsConfig - */ - public static class Builder { - private boolean mResetOnUnplugHighBatteryLevel; - private boolean mResetOnUnplugAfterSignificantCharge; - public Builder() { - mResetOnUnplugHighBatteryLevel = true; - mResetOnUnplugAfterSignificantCharge = true; - } - - /** - * Build the BatteryStatsConfig. - */ - public BatteryStatsConfig build() { - return new BatteryStatsConfig(this); - } - - /** - * Set whether a BatteryStats reset should occur on unplug when the battery level is - * high. - */ - public Builder setResetOnUnplugHighBatteryLevel(boolean reset) { - mResetOnUnplugHighBatteryLevel = reset; - return this; - } - - /** - * Set whether a BatteryStats reset should occur on unplug if the battery charge a - * significant amount since it has been plugged in. - */ - public Builder setResetOnUnplugAfterSignificantCharge(boolean reset) { - mResetOnUnplugAfterSignificantCharge = reset; - return this; - } - } - - } - private final PlatformIdleStateCallback mPlatformIdleStateCallback; private final Runnable mDeferSetCharging = new Runnable() { @@ -1534,10 +1459,6 @@ public class BatteryStatsImpl extends BatteryStats { @GuardedBy("this") protected final Constants mConstants; - @VisibleForTesting - @GuardedBy("this") - protected BatteryStatsConfig mBatteryStatsConfig = new BatteryStatsConfig.Builder().build(); - /* * Holds a SamplingTimer associated with each Resource Power Manager state and voter, * recording their times when on-battery (regardless of screen state). @@ -1704,13 +1625,12 @@ public class BatteryStatsImpl extends BatteryStats { public BatteryStatsImpl(Clock clock, File historyDirectory) { init(clock); mStartClockTimeMs = clock.currentTimeMillis(); + mCheckinFile = null; mDailyFile = null; if (historyDirectory == null) { - mCheckinFile = null; mStatsFile = null; mBatteryStatsHistory = new BatteryStatsHistory(mHistoryBuffer); } else { - mCheckinFile = new AtomicFile(new File(historyDirectory, "batterystats-checkin.bin")); mStatsFile = new AtomicFile(new File(historyDirectory, "batterystats.bin")); mBatteryStatsHistory = new BatteryStatsHistory(this, historyDirectory, mHistoryBuffer); } @@ -12627,15 +12547,6 @@ public class BatteryStatsImpl extends BatteryStats { return mPowerProfile; } - /** - * Injects BatteryStatsConfig - */ - public void setBatteryStatsConfig(BatteryStatsConfig config) { - synchronized (this) { - mBatteryStatsConfig = config; - } - } - /** * Starts tracking CPU time-in-state for threads of the system server process, * keeping a separate account of threads receiving incoming binder calls. @@ -15593,32 +15504,6 @@ public class BatteryStatsImpl extends BatteryStats { mSystemReady = true; } - @GuardedBy("this") - private boolean shouldResetOnUnplugLocked(int batteryStatus, int batteryLevel) { - if (mNoAutoReset) return false; - if (!mSystemReady) return false; - if (mBatteryStatsConfig.shouldResetOnUnplugHighBatteryLevel()) { - // Allow resetting due to currently being at high battery level - if (batteryStatus == BatteryManager.BATTERY_STATUS_FULL) return true; - if (batteryLevel >= 90) return true; - } - if (mBatteryStatsConfig.shouldResetOnUnplugAfterSignificantCharge()) { - // Allow resetting after a significant charge (from a very low level to a now very - // high level). - if (mDischargePlugLevel < 20 && batteryLevel >= 80) return true; - } - if (getHighDischargeAmountSinceCharge() >= 200) { - // Reset the stats if battery got partially charged and discharged repeatedly without - // ever reaching the full charge. - // This reset is done in order to prevent stats sessions from going on forever. - // Exceedingly long battery sessions would lead to an overflow of - // data structures such as mWakeupReasonStats. - return true; - } - - return false; - } - @GuardedBy("this") protected void setOnBatteryLocked(final long mSecRealtime, final long mSecUptime, final boolean onBattery, final int oldStatus, final int level, final int chargeUah) { @@ -15631,10 +15516,23 @@ public class BatteryStatsImpl extends BatteryStats { final long realtimeUs = mSecRealtime * 1000; final int screenState = mScreenState; if (onBattery) { + // We will reset our status if we are unplugging after the + // battery was last full, or the level is at 100, or + // we have gone through a significant charge (from a very low + // level to a now very high level). + // Also, we will reset the stats if battery got partially charged + // and discharged repeatedly without ever reaching the full charge. + // This reset is done in order to prevent stats sessions from going on forever. + // Exceedingly long battery sessions would lead to an overflow of + // data structures such as mWakeupReasonStats. boolean reset = false; - if (shouldResetOnUnplugLocked(oldStatus, level)) { + if (!mNoAutoReset && mSystemReady + && (oldStatus == BatteryManager.BATTERY_STATUS_FULL + || level >= 90 + || (mDischargeCurrentLevel < 20 && level >= 80) + || getHighDischargeAmountSinceCharge() >= 200)) { Slog.i(TAG, "Resetting battery stats: level=" + level + " status=" + oldStatus - + " dischargeLevel=" + mDischargePlugLevel + + " dischargeLevel=" + mDischargeCurrentLevel + " lowAmount=" + getLowDischargeAmountSinceCharge() + " highAmount=" + getHighDischargeAmountSinceCharge()); // Before we write, collect a snapshot of the final aggregated diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 416b7c35b25d..521877353dea 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -6084,8 +6084,4 @@ different from the home screen wallpaper. --> false - - true - - true diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index ccf6e9acb4a6..93cbe9c863a9 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -4893,6 +4893,4 @@ - - diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java deleted file mode 100644 index fb61ec5efd81..000000000000 --- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsResetTest.java +++ /dev/null @@ -1,297 +0,0 @@ -/* - * Copyright (C) 2023 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 static com.google.common.truth.Truth.assertThat; - -import android.content.Context; -import android.os.BatteryManager; - -import androidx.test.InstrumentationRegistry; -import androidx.test.filters.SmallTest; -import androidx.test.runner.AndroidJUnit4; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -@SmallTest -@RunWith(AndroidJUnit4.class) -public class BatteryStatsResetTest { - - private static final int BATTERY_NOMINAL_VOLTAGE_MV = 3700; - private static final int BATTERY_CAPACITY_UAH = 4_000_000; - private static final int BATTERY_CHARGE_RATE_SECONDS_PER_LEVEL = 100; - - private MockClock mMockClock; - private MockBatteryStatsImpl mBatteryStatsImpl; - - - /** - * Battery status. Must be one of the following: - * {@link BatteryManager#BATTERY_STATUS_UNKNOWN} - * {@link BatteryManager#BATTERY_STATUS_CHARGING} - * {@link BatteryManager#BATTERY_STATUS_DISCHARGING} - * {@link BatteryManager#BATTERY_STATUS_NOT_CHARGING} - * {@link BatteryManager#BATTERY_STATUS_FULL} - */ - private int mBatteryStatus; - /** - * Battery health. Must be one of the following: - * {@link BatteryManager#BATTERY_HEALTH_UNKNOWN} - * {@link BatteryManager#BATTERY_HEALTH_GOOD} - * {@link BatteryManager#BATTERY_HEALTH_OVERHEAT} - * {@link BatteryManager#BATTERY_HEALTH_DEAD} - * {@link BatteryManager#BATTERY_HEALTH_OVER_VOLTAGE} - * {@link BatteryManager#BATTERY_HEALTH_UNSPECIFIED_FAILURE} - * {@link BatteryManager#BATTERY_HEALTH_COLD} - */ - private int mBatteryHealth; - /** - * Battery plug type. Can be the union of any number of the following flags: - * {@link BatteryManager#BATTERY_PLUGGED_AC} - * {@link BatteryManager#BATTERY_PLUGGED_USB} - * {@link BatteryManager#BATTERY_PLUGGED_WIRELESS} - * {@link BatteryManager#BATTERY_PLUGGED_DOCK} - * - * Zero means the device is unplugged. - */ - private int mBatteryPlugType; - private int mBatteryLevel; - private int mBatteryTemp; - private int mBatteryVoltageMv; - private int mBatteryChargeUah; - private int mBatteryChargeFullUah; - private long mBatteryChargeTimeToFullSeconds; - - @Before - public void setUp() { - final Context context = InstrumentationRegistry.getContext(); - - mMockClock = new MockClock(); - mBatteryStatsImpl = new MockBatteryStatsImpl(mMockClock, context.getFilesDir()); - mBatteryStatsImpl.onSystemReady(); - - - // Set up the battery state. Start off with a fully charged plugged in battery. - mBatteryStatus = BatteryManager.BATTERY_STATUS_FULL; - mBatteryHealth = BatteryManager.BATTERY_HEALTH_GOOD; - mBatteryPlugType = BatteryManager.BATTERY_PLUGGED_USB; - mBatteryLevel = 100; - mBatteryTemp = 70; // Arbitrary reasonable temperature. - mBatteryVoltageMv = BATTERY_NOMINAL_VOLTAGE_MV; - mBatteryChargeUah = BATTERY_CAPACITY_UAH; - mBatteryChargeFullUah = BATTERY_CAPACITY_UAH; - mBatteryChargeTimeToFullSeconds = 0; - } - - @Test - public void testResetOnUnplug_highBatteryLevel() { - mBatteryStatsImpl.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugHighBatteryLevel(true) - .build()); - - long expectedResetTimeUs = 0; - - unplugBattery(); - dischargeToLevel(60); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Reset should not occur until battery level above 90. - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(95); - // Reset should not occur until unplug. - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - unplugBattery(); - // Reset should occur on unplug now that battery level is high (>=90) - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // disable high battery level reset on unplug. - mBatteryStatsImpl.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugHighBatteryLevel(false) - .build()); - - dischargeToLevel(60); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(95); - unplugBattery(); - // Reset should not occur since the high battery level logic has been disabled. - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - } - - @Test - public void testResetOnUnplug_significantCharge() { - mBatteryStatsImpl.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugAfterSignificantCharge(true) - .build()); - long expectedResetTimeUs = 0; - - unplugBattery(); - // Battery level dropped below 20%. - dischargeToLevel(15); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(50); - unplugBattery(); - // Reset should not occur until battery level above 80 - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(85); - unplugBattery(); - // Reset should not occur because the charge session did not go from 20% to 80% - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Battery level dropped below 20%. - dischargeToLevel(15); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(85); - unplugBattery(); - // Reset should occur after significant charge amount. - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // disable reset on unplug after significant charge. - mBatteryStatsImpl.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugAfterSignificantCharge(false) - .build()); - - // Battery level dropped below 20%. - dischargeToLevel(15); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(85); - unplugBattery(); - // Reset should not occur after significant charge amount. - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - } - - @Test - public void testResetOnUnplug_manyPartialCharges() { - long expectedResetTimeUs = 0; - - unplugBattery(); - // Cumulative battery discharged: 60%. - dischargeToLevel(40); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Cumulative battery discharged: 100%. - dischargeToLevel(40); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Cumulative battery discharged: 140%. - dischargeToLevel(40); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Cumulative battery discharged: 180%. - dischargeToLevel(40); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Reset should not occur - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - - // Cumulative battery discharged: 220%. - dischargeToLevel(40); - - plugBattery(BatteryManager.BATTERY_PLUGGED_USB); - chargeToLevel(80); - unplugBattery(); - // Should reset after >200% of cumulative battery discharge - expectedResetTimeUs = mMockClock.elapsedRealtime() * 1000; - assertThat(mBatteryStatsImpl.getStatsStartRealtime()).isEqualTo(expectedResetTimeUs); - } - - private void dischargeToLevel(int targetLevel) { - mBatteryStatus = BatteryManager.BATTERY_STATUS_DISCHARGING; - for (int level = mBatteryLevel - 1; level >= targetLevel; level--) { - prepareBatteryLevel(level); - incTimeMs(5000); // Arbitrary discharge rate. - updateBatteryState(); - } - } - - private void chargeToLevel(int targetLevel) { - mBatteryStatus = BatteryManager.BATTERY_STATUS_CHARGING; - for (int level = mBatteryLevel + 1; level <= targetLevel; level++) { - if (level >= 100) mBatteryStatus = BatteryManager.BATTERY_STATUS_FULL; - prepareBatteryLevel(level); - incTimeMs(BATTERY_CHARGE_RATE_SECONDS_PER_LEVEL * 1000); - updateBatteryState(); - } - } - - private void unplugBattery() { - mBatteryPlugType = 0; - updateBatteryState(); - } - - private void plugBattery(int type) { - mBatteryPlugType |= type; - updateBatteryState(); - } - - private void prepareBatteryLevel(int level) { - mBatteryLevel = level; - mBatteryChargeUah = mBatteryLevel * mBatteryChargeFullUah / 100; - mBatteryChargeTimeToFullSeconds = - (100 - mBatteryLevel) * BATTERY_CHARGE_RATE_SECONDS_PER_LEVEL; - } - - private void incTimeMs(long milliseconds) { - mMockClock.realtime += milliseconds; - mMockClock.uptime += milliseconds / 2; // Arbitrary slower uptime accumulation - mMockClock.currentTime += milliseconds; - } - - private void updateBatteryState() { - mBatteryStatsImpl.setBatteryStateLocked(mBatteryStatus, mBatteryHealth, mBatteryPlugType, - mBatteryLevel, mBatteryTemp, mBatteryVoltageMv, mBatteryChargeUah, - mBatteryChargeFullUah, mBatteryChargeTimeToFullSeconds, - mMockClock.elapsedRealtime(), mMockClock.uptimeMillis(), - mMockClock.currentTimeMillis()); - } -} diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index c971d9221218..207c10c44c9b 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -356,16 +356,6 @@ public final class BatteryStatsService extends IBatteryStats.Stub mStats.setRadioScanningTimeoutLocked(mContext.getResources().getInteger( com.android.internal.R.integer.config_radioScanningTimeout) * 1000L); mStats.setPowerProfileLocked(mPowerProfile); - - final boolean resetOnUnplugHighBatteryLevel = context.getResources().getBoolean( - com.android.internal.R.bool.config_batteryStatsResetOnUnplugHighBatteryLevel); - final boolean resetOnUnplugAfterSignificantCharge = context.getResources().getBoolean( - com.android.internal.R.bool.config_batteryStatsResetOnUnplugAfterSignificantCharge); - mStats.setBatteryStatsConfig( - new BatteryStatsImpl.BatteryStatsConfig.Builder() - .setResetOnUnplugHighBatteryLevel(resetOnUnplugHighBatteryLevel) - .setResetOnUnplugAfterSignificantCharge(resetOnUnplugAfterSignificantCharge) - .build()); mStats.startTrackingSystemServerCpuTime(); if (BATTERY_USAGE_STORE_ENABLED) { -- cgit v1.2.3