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

Commit 0419d437 authored by Connor O'Brien's avatar Connor O'Brien
Browse files

Don't remove UID data in readKernelCpuUid*TimesLocked 



In both of these cases (removed isolated UID and nonexistent user) the
affected UID has already previously been enqueued in
mPendingRemovedUids and so all underlying cpu time data will be
cleared together after 5 minutes. Clearing the different sets of times
separately is buggy due to creating inconsistency among the
KernelCpuUid*TimeReaders. Removing data in these functions also
defeats the 5-minute delay that was intended to give statsd a chance
to read the data for removed UIDs.

Also update BatteryStatsCpuTimesTest to reflect the new behavior

Bug: 182272121
Test: atest com.android.internal.os.BatteryStatsTests shows no new
failures
Change-Id: I5719aecd5603cd9cecfbf75cede5eceeda8d0f5b
Signed-off-by: default avatarConnor O'Brien <connoro@google.com>
parent 3f8d3330

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

+0 −20
Original line number Diff line number Diff line
@@ -13091,13 +13091,11 @@ public class BatteryStatsImpl extends BatteryStats { 
            if (Process.isIsolated(uid)) {

                // This could happen if the isolated uid mapping was removed before that process

                // was actually killed.

                mCpuUidUserSysTimeReader.removeUid(uid);

                if (DEBUG) Slog.d(TAG, "Got readings for an isolated uid: " + uid);

                return;

            }

            if (!mUserInfoProvider.exists(UserHandle.getUserId(uid))) {

                if (DEBUG) Slog.d(TAG, "Got readings for an invalid user's uid " + uid);

                mCpuUidUserSysTimeReader.removeUid(uid);

                return;

            }

            final Uid u = getUidStatsLocked(uid, elapsedRealtimeMs, startTimeMs);
@@ -13202,19 +13200,16 @@ public class BatteryStatsImpl extends BatteryStats { 
        mWakeLockAllocationsUs = null;

        final long startTimeMs = mClocks.uptimeMillis();

        final long elapsedRealtimeMs = mClocks.elapsedRealtime();

        final List<Integer> uidsToRemove = new ArrayList<>();

        // If power is being accumulated for attribution, data needs to be read immediately.

        final boolean forceRead = powerAccumulator != null;

        mCpuUidFreqTimeReader.readDelta(forceRead, (uid, cpuFreqTimeMs) -> {

            uid = mapUid(uid);

            if (Process.isIsolated(uid)) {

                uidsToRemove.add(uid);

                if (DEBUG) Slog.d(TAG, "Got freq readings for an isolated uid: " + uid);

                return;

            }

            if (!mUserInfoProvider.exists(UserHandle.getUserId(uid))) {

                if (DEBUG) Slog.d(TAG, "Got freq readings for an invalid user's uid " + uid);

                uidsToRemove.add(uid);

                return;

            }

            final Uid u = getUidStatsLocked(uid, elapsedRealtimeMs, startTimeMs);
@@ -13278,9 +13273,6 @@ public class BatteryStatsImpl extends BatteryStats { 
                }

            }

        });

        for (int uid : uidsToRemove) {

            mCpuUidFreqTimeReader.removeUid(uid);

        }















        final long elapsedTimeMs = mClocks.uptimeMillis() - startTimeMs;















        if (DEBUG_ENERGY_CPU || elapsedTimeMs >= 100) {










@@ -13332,25 +13324,19 @@ public class BatteryStatsImpl extends BatteryStats {













    public void readKernelUidCpuActiveTimesLocked(boolean onBattery) {















        final long startTimeMs = mClocks.uptimeMillis();















        final long elapsedRealtimeMs = mClocks.elapsedRealtime();













        final List<Integer> uidsToRemove = new ArrayList<>();















        mCpuUidActiveTimeReader.readDelta(false, (uid, cpuActiveTimesMs) -> {















            uid = mapUid(uid);















            if (Process.isIsolated(uid)) {













                uidsToRemove.add(uid);















                if (DEBUG) Slog.w(TAG, "Got active times for an isolated uid: " + uid);















                return;















            }















            if (!mUserInfoProvider.exists(UserHandle.getUserId(uid))) {















                if (DEBUG) Slog.w(TAG, "Got active times for an invalid user's uid " + uid);













                uidsToRemove.add(uid);















                return;















            }















            final Uid u = getUidStatsLocked(uid, elapsedRealtimeMs, startTimeMs);















            u.mCpuActiveTimeMs.addCountLocked(cpuActiveTimesMs, onBattery);















        });













        for (int uid : uidsToRemove) {













            mCpuUidActiveTimeReader.removeUid(uid);













        }





























        final long elapsedTimeMs = mClocks.uptimeMillis() - startTimeMs;















        if (DEBUG_ENERGY_CPU || elapsedTimeMs >= 100) {









@@ -13371,19 +13357,16 @@ public class BatteryStatsImpl extends BatteryStats {













            @Nullable CpuDeltaPowerAccumulator powerAccumulator) {















        final long startTimeMs = mClocks.uptimeMillis();















        final long elapsedRealtimeMs = mClocks.elapsedRealtime();













        final List<Integer> uidsToRemove = new ArrayList<>();















        // If power is being accumulated for attribution, data needs to be read immediately.















        final boolean forceRead = powerAccumulator != null;















        mCpuUidClusterTimeReader.readDelta(forceRead, (uid, cpuClusterTimesMs) -> {















            uid = mapUid(uid);















            if (Process.isIsolated(uid)) {













                uidsToRemove.add(uid);















                if (DEBUG) Slog.w(TAG, "Got cluster times for an isolated uid: " + uid);















                return;















            }















            if (!mUserInfoProvider.exists(UserHandle.getUserId(uid))) {















                if (DEBUG) Slog.w(TAG, "Got cluster times for an invalid user's uid " + uid);













                uidsToRemove.add(uid);















                return;















            }















            final Uid u = getUidStatsLocked(uid, elapsedRealtimeMs, startTimeMs);









@@ -13393,9 +13376,6 @@ public class BatteryStatsImpl extends BatteryStats {













                powerAccumulator.addCpuClusterDurationsMs(u, cpuClusterTimesMs);















            }















        });













        for (int uid : uidsToRemove) {













            mCpuUidClusterTimeReader.removeUid(uid);













        }





























        final long elapsedTimeMs = mClocks.uptimeMillis() - startTimeMs;















        if (DEBUG_ENERGY_CPU || elapsedTimeMs >= 100) {

























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



+0
−6




























Original line number
Diff line number
Diff line








@@ -363,7 +363,6 @@ public class BatteryStatsCpuTimesTest {













            assertEquals("Unexpected system cpu time for uid=" + testUids[i],















                    uidTimesUs[i][1], u.getSystemCpuTimeUs(STATS_SINCE_CHARGED));















        }













        verify(mCpuUidUserSysTimeReader).removeUid(isolatedUid);































        // Add an isolated uid mapping and repeat the test.


























@@ -453,7 +452,6 @@ public class BatteryStatsCpuTimesTest {













        }















        assertNull("There shouldn't be an entry for invalid uid=" + invalidUid,















                mBatteryStatsImpl.getUidStats().get(invalidUid));













        verify(mCpuUidUserSysTimeReader).removeUid(invalidUid);















    }































    @Test










@@ -943,7 +941,6 @@ public class BatteryStatsCpuTimesTest {













            assertNull("Unexpected screen-off cpu times for uid=" + testUids[i],















                    u.getScreenOffCpuFreqTimes(STATS_SINCE_CHARGED));















        }













        verify(mCpuUidFreqTimeReader).removeUid(isolatedUid);















































        // Add an isolated uid mapping and repeat the test.










@@ -1038,7 +1035,6 @@ public class BatteryStatsCpuTimesTest {













        }















        assertNull("There shouldn't be an entry for invalid uid=" + invalidUid,















                mBatteryStatsImpl.getUidStats().get(invalidUid));













        verify(mCpuUidFreqTimeReader).removeUid(invalidUid);















    }































    @Test










@@ -1142,7 +1138,6 @@ public class BatteryStatsCpuTimesTest {













        }















        assertNull("There shouldn't be an entry for invalid uid=" + invalidUid,















                mBatteryStatsImpl.getUidStats().get(invalidUid));













        verify(mCpuUidActiveTimeReader).removeUid(invalidUid);















    }































    @Test










@@ -1259,7 +1254,6 @@ public class BatteryStatsCpuTimesTest {













        }















        assertNull("There shouldn't be an entry for invalid uid=" + invalidUid,















                mBatteryStatsImpl.getUidStats().get(invalidUid));













        verify(mCpuUidClusterTimeReader).removeUid(invalidUid);















    }































    @Test