From de73c05b89f09ce19784688ab6eafb587eb6451e Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Mon, 9 May 2022 20:10:26 -0700 Subject: [PATCH] Skip collecting radio data on procstate change when radio is off. Fixes: 229042376 Test: atest BatteryStatsNoteTest Change-Id: I6b3c5a5c9416bc47b8dbb00e28c81d93e3507613 --- .../android/internal/os/BatteryStatsImpl.java | 19 +++++-- .../internal/os/BatteryStatsNoteTest.java | 49 +++++++++++++++++++ .../internal/os/MockBatteryStatsImpl.java | 10 ++-- .../server/am/BatteryExternalStatsWorker.java | 5 +- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index e4dec563f580e..cc7f490ef0d47 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -644,7 +644,7 @@ public class BatteryStatsImpl extends BatteryStats { /** Schedule removal of UIDs corresponding to a removed user */ Future scheduleCleanupDueToRemovedUser(int userId); /** Schedule a sync because of a process state change */ - Future scheduleSyncDueToProcessStateChange(long delayMillis); + void scheduleSyncDueToProcessStateChange(int flags, long delayMillis); } public Handler mHandler; @@ -6216,9 +6216,7 @@ public class BatteryStatsImpl extends BatteryStats { long elapsedRealtimeMs, long uptimeMs) { if (mMobileRadioPowerState != powerState) { long realElapsedRealtimeMs; - final boolean active = - powerState == DataConnectionRealTimeInfo.DC_POWER_STATE_MEDIUM - || powerState == DataConnectionRealTimeInfo.DC_POWER_STATE_HIGH; + final boolean active = isActiveRadioPowerState(powerState); if (active) { if (uid > 0) { noteMobileRadioApWakeupLocked(elapsedRealtimeMs, uptimeMs, uid); @@ -6260,6 +6258,11 @@ public class BatteryStatsImpl extends BatteryStats { return false; } + private static boolean isActiveRadioPowerState(int powerState) { + return powerState == DataConnectionRealTimeInfo.DC_POWER_STATE_MEDIUM + || powerState == DataConnectionRealTimeInfo.DC_POWER_STATE_HIGH; + } + @GuardedBy("this") public void notePowerSaveModeLocked(boolean enabled) { notePowerSaveModeLocked(enabled, mClock.elapsedRealtime(), mClock.uptimeMillis()); @@ -12044,7 +12047,13 @@ public class BatteryStatsImpl extends BatteryStats { return; } - mBsi.mExternalSync.scheduleSyncDueToProcessStateChange( + int flags = ExternalStatsSync.UPDATE_ON_PROC_STATE_CHANGE; + // Skip querying for inactive radio, where power usage is probably negligible. + if (!BatteryStatsImpl.isActiveRadioPowerState(mBsi.mMobileRadioPowerState)) { + flags &= ~ExternalStatsSync.UPDATE_RADIO; + } + + mBsi.mExternalSync.scheduleSyncDueToProcessStateChange(flags, mBsi.mConstants.PROC_STATE_CHANGE_COLLECTION_DELAY_MS); } diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java index 87c45dccfa0ce..d19f9f5ea58f9 100644 --- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BatteryStatsNoteTest.java @@ -47,6 +47,7 @@ import android.telephony.DataConnectionRealTimeInfo; import android.telephony.ModemActivityInfo; import android.telephony.ServiceState; import android.telephony.TelephonyManager; +import android.util.MutableInt; import android.util.SparseIntArray; import android.util.SparseLongArray; import android.view.Display; @@ -1982,6 +1983,54 @@ public class BatteryStatsNoteTest extends TestCase { expectedTxDurationsMs, bi, state.currentTimeMs); } + @SmallTest + @SuppressWarnings("GuardedBy") + public void testProcStateSyncScheduling_mobileRadioActiveState() { + final MockClock clock = new MockClock(); // holds realtime and uptime in ms + final MockBatteryStatsImpl bi = new MockBatteryStatsImpl(clock); + final MutableInt lastProcStateChangeFlags = new MutableInt(0); + + MockBatteryStatsImpl.DummyExternalStatsSync externalStatsSync = + new MockBatteryStatsImpl.DummyExternalStatsSync() { + @Override + public void scheduleSyncDueToProcessStateChange(int flags, + long delayMillis) { + lastProcStateChangeFlags.value = flags; + } + }; + + bi.setDummyExternalStatsSync(externalStatsSync); + + bi.updateTimeBasesLocked(true, Display.STATE_OFF, 0, 0); + + // Note mobile radio is on. + long curr = 1000L * (clock.realtime = clock.uptime = 1001); + bi.noteMobileRadioPowerStateLocked(DataConnectionRealTimeInfo.DC_POWER_STATE_HIGH, curr, + UID); + + lastProcStateChangeFlags.value = 0; + clock.realtime = clock.uptime = 2002; + bi.noteUidProcessStateLocked(UID, ActivityManager.PROCESS_STATE_IMPORTANT_FOREGROUND); + + final int allProcFlags = BatteryStatsImpl.ExternalStatsSync.UPDATE_ON_PROC_STATE_CHANGE; + assertEquals(allProcFlags, lastProcStateChangeFlags.value); + + // Note mobile radio is off. + curr = 1000L * (clock.realtime = clock.uptime = 3003); + bi.noteMobileRadioPowerStateLocked(DataConnectionRealTimeInfo.DC_POWER_STATE_LOW, curr, + UID); + + lastProcStateChangeFlags.value = 0; + clock.realtime = clock.uptime = 4004; + bi.noteUidProcessStateLocked(UID, ActivityManager.PROCESS_STATE_CACHED_EMPTY); + + final int noRadioProcFlags = BatteryStatsImpl.ExternalStatsSync.UPDATE_ON_PROC_STATE_CHANGE + & ~BatteryStatsImpl.ExternalStatsSync.UPDATE_RADIO; + assertEquals( + "An inactive radio should not be queried on proc state change", + noRadioProcFlags, lastProcStateChangeFlags.value); + } + private void setFgState(int uid, boolean fgOn, MockBatteryStatsImpl bi) { // Note that noteUidProcessStateLocked uses ActivityManager process states. if (fgOn) { diff --git a/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java b/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java index 00154a3d23b06..edeb5e9f4834e 100644 --- a/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java +++ b/core/tests/coretests/src/com/android/internal/os/MockBatteryStatsImpl.java @@ -212,7 +212,12 @@ public class MockBatteryStatsImpl extends BatteryStatsImpl { return flags; } - private class DummyExternalStatsSync implements ExternalStatsSync { + public void setDummyExternalStatsSync(DummyExternalStatsSync externalStatsSync) { + mExternalStatsSync = externalStatsSync; + setExternalStatsSyncLocked(mExternalStatsSync); + } + + public static class DummyExternalStatsSync implements ExternalStatsSync { public int flags = 0; @Override @@ -257,8 +262,7 @@ public class MockBatteryStatsImpl extends BatteryStatsImpl { } @Override - public Future scheduleSyncDueToProcessStateChange(long delayMillis) { - return null; + public void scheduleSyncDueToProcessStateChange(int flags, long delayMillis) { } } } diff --git a/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java b/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java index 84969aa9e9163..702526a4beabe 100644 --- a/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java +++ b/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java @@ -320,12 +320,11 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { } @Override - public Future scheduleSyncDueToProcessStateChange(long delayMillis) { + public void scheduleSyncDueToProcessStateChange(int flags, long delayMillis) { synchronized (BatteryExternalStatsWorker.this) { mProcessStateSync = scheduleDelayedSyncLocked(mProcessStateSync, - () -> scheduleSync("procstate-change", UPDATE_ON_PROC_STATE_CHANGE), + () -> scheduleSync("procstate-change", flags), delayMillis); - return mProcessStateSync; } }