Handle reuse of isolated UIDs in PowerStatsCollectors

Bug: 333762314
Test: atest PowerStatsTests
Change-Id: I3c21ee152af9ed3c7df43c6436fc0d9af87267fd

5 files changed, 77 insertions, 24 deletions

diff --git a/services/core/java/com/android/server/power/stats/CpuPowerStatsCollector.java b/services/core/java/com/android/server/power/stats/CpuPowerStatsCollector.java
index b1b2cc91d379..f53a1b0682c0 100644
--- a/services/core/java/com/android/server/power/stats/CpuPowerStatsCollector.java
+++ b/services/core/java/com/android/server/power/stats/CpuPowerStatsCollector.java
@@ -49,7 +49,6 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
     private static final long ENERGY_UNSPECIFIED = -1;
     private static final int DEFAULT_CPU_POWER_BRACKETS = 3;
     private static final int DEFAULT_CPU_POWER_BRACKETS_PER_ENERGY_CONSUMER = 2;
-    private static final long POWER_STATS_ENERGY_CONSUMERS_TIMEOUT = 20000;
 
     interface Injector {
         Handler getHandler();
@@ -76,7 +75,6 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
     private CpuScalingPolicies mCpuScalingPolicies;
     private PowerProfile mPowerProfile;
     private KernelCpuStatsReader mKernelCpuStatsReader;
-    private PowerStatsUidResolver mUidResolver;
     private ConsumedEnergyRetriever mConsumedEnergyRetriever;
     private IntSupplier mVoltageSupplier;
     private int mDefaultCpuPowerBrackets;
@@ -97,7 +95,8 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
     private long[] mLastConsumedEnergyUws;
 
     public CpuPowerStatsCollector(Injector injector, long throttlePeriodMs) {
-        super(injector.getHandler(), throttlePeriodMs, injector.getClock());
+        super(injector.getHandler(), throttlePeriodMs, injector.getUidResolver(),
+                injector.getClock());
         mInjector = injector;
     }
 
@@ -113,7 +112,6 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
         mCpuScalingPolicies = mInjector.getCpuScalingPolicies();
         mPowerProfile = mInjector.getPowerProfile();
         mKernelCpuStatsReader = mInjector.getKernelCpuStatsReader();
-        mUidResolver = mInjector.getUidResolver();
         mConsumedEnergyRetriever = mInjector.getConsumedEnergyRetriever();
         mVoltageSupplier = mInjector.getVoltageSupplier();
         mDefaultCpuPowerBrackets = mInjector.getDefaultCpuPowerBrackets();
@@ -421,7 +419,8 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
 
         boolean nonzero = false;
         for (int bracket = powerBracketCount - 1; bracket >= 0; bracket--) {
-            long delta = timeByPowerBracket[bracket] - uidStats.timeByPowerBracket[bracket];
+            long delta = Math.max(0,
+                    timeByPowerBracket[bracket] - uidStats.timeByPowerBracket[bracket]);
             if (delta != 0) {
                 nonzero = true;
             }
@@ -447,6 +446,12 @@ public class CpuPowerStatsCollector extends PowerStatsCollector {
         }
     }
 
+    @Override
+    protected void onUidRemoved(int uid) {
+        super.onUidRemoved(uid);
+        mUidStats.remove(uid);
+    }
+
     /**
      * Native class that retrieves CPU stats from the kernel.
      */
diff --git a/services/core/java/com/android/server/power/stats/MobileRadioPowerStatsCollector.java b/services/core/java/com/android/server/power/stats/MobileRadioPowerStatsCollector.java
index 8c154e4a0875..7bc681752802 100644
--- a/services/core/java/com/android/server/power/stats/MobileRadioPowerStatsCollector.java
+++ b/services/core/java/com/android/server/power/stats/MobileRadioPowerStatsCollector.java
@@ -89,7 +89,6 @@ public class MobileRadioPowerStatsCollector extends PowerStatsCollector {
 
     private PowerStats mPowerStats;
     private long[] mDeviceStats;
-    private PowerStatsUidResolver mPowerStatsUidResolver;
     private volatile TelephonyManager mTelephonyManager;
     private LongSupplier mCallDurationSupplier;
     private LongSupplier mScanDurationSupplier;
@@ -106,7 +105,8 @@ public class MobileRadioPowerStatsCollector extends PowerStatsCollector {
     private long mLastScanDuration;
 
     public MobileRadioPowerStatsCollector(Injector injector, long throttlePeriodMs) {
-        super(injector.getHandler(), throttlePeriodMs, injector.getClock());
+        super(injector.getHandler(), throttlePeriodMs, injector.getUidResolver(),
+                injector.getClock());
         mInjector = injector;
     }
 
@@ -130,7 +130,6 @@ public class MobileRadioPowerStatsCollector extends PowerStatsCollector {
             return false;
         }
 
-        mPowerStatsUidResolver = mInjector.getUidResolver();
         mConsumedEnergyRetriever = mInjector.getConsumedEnergyRetriever();
         mVoltageSupplier = mInjector.getVoltageSupplier();
 
@@ -310,7 +309,7 @@ public class MobileRadioPowerStatsCollector extends PowerStatsCollector {
                 continue;
             }
 
-            int uid = mPowerStatsUidResolver.mapUid(uidDelta.getUid());
+            int uid = mUidResolver.mapUid(uidDelta.getUid());
             long[] stats = mPowerStats.uidStats.get(uid);
             if (stats == null) {
                 stats = new long[mLayout.getUidStatsArrayLength()];
diff --git a/services/core/java/com/android/server/power/stats/PowerStatsCollector.java b/services/core/java/com/android/server/power/stats/PowerStatsCollector.java
index 5dd11db2a2fc..b82c0215013c 100644
--- a/services/core/java/com/android/server/power/stats/PowerStatsCollector.java
+++ b/services/core/java/com/android/server/power/stats/PowerStatsCollector.java
@@ -53,6 +53,7 @@ public abstract class PowerStatsCollector {
     private static final int MILLIVOLTS_PER_VOLT = 1000;
     private static final long POWER_STATS_ENERGY_CONSUMERS_TIMEOUT = 20000;
     private final Handler mHandler;
+    protected final PowerStatsUidResolver mUidResolver;
     protected final Clock mClock;
     private final long mThrottlePeriodMs;
     private final Runnable mCollectAndDeliverStats = this::collectAndDeliverStats;
@@ -63,9 +64,25 @@ public abstract class PowerStatsCollector {
     @SuppressWarnings("unchecked")
     private volatile List<Consumer<PowerStats>> mConsumerList = Collections.emptyList();
 
-    public PowerStatsCollector(Handler handler, long throttlePeriodMs, Clock clock) {
+    public PowerStatsCollector(Handler handler, long throttlePeriodMs,
+            PowerStatsUidResolver uidResolver, Clock clock) {
         mHandler = handler;
         mThrottlePeriodMs = throttlePeriodMs;
+        mUidResolver = uidResolver;
+        mUidResolver.addListener(new PowerStatsUidResolver.Listener() {
+            @Override
+            public void onIsolatedUidAdded(int isolatedUid, int parentUid) {
+            }
+
+            @Override
+            public void onBeforeIsolatedUidRemoved(int isolatedUid, int parentUid) {
+            }
+
+            @Override
+            public void onAfterIsolatedUidRemoved(int isolatedUid, int parentUid) {
+                mHandler.post(()->onUidRemoved(isolatedUid));
+            }
+        });
         mClock = clock;
     }
 
@@ -203,6 +220,9 @@ public abstract class PowerStatsCollector {
         done.block();
     }
 
+    protected void onUidRemoved(int uid) {
+    }
+
     /** Calculate charge consumption (in microcoulombs) from a given energy and voltage */
     protected static long uJtoUc(long deltaEnergyUj, int avgVoltageMv) {
         // To overflow, a 3.7V 10000mAh battery would need to completely drain 69244 times
diff --git a/services/tests/powerstatstests/src/com/android/server/power/stats/CpuPowerStatsCollectorTest.java b/services/tests/powerstatstests/src/com/android/server/power/stats/CpuPowerStatsCollectorTest.java
index ad2939284471..d51828e418c6 100644
--- a/services/tests/powerstatstests/src/com/android/server/power/stats/CpuPowerStatsCollectorTest.java
+++ b/services/tests/powerstatstests/src/com/android/server/power/stats/CpuPowerStatsCollectorTest.java
@@ -67,12 +67,11 @@ public class CpuPowerStatsCollectorTest {
     private static final int UID_2 = 99;
     private final MockClock mMockClock = new MockClock();
     private final HandlerThread mHandlerThread = new HandlerThread("test");
+    private final PowerStatsUidResolver mUidResolver = new PowerStatsUidResolver();
     private Handler mHandler;
     private PowerStats mCollectedStats;
     private PowerProfile mPowerProfile = new PowerProfile();
     @Mock
-    private PowerStatsUidResolver mUidResolver;
-    @Mock
     private CpuPowerStatsCollector.KernelCpuStatsReader mMockKernelCpuStatsReader;
     @Mock
     private PowerStatsCollector.ConsumedEnergyRetriever mConsumedEnergyRetriever;
@@ -144,15 +143,8 @@ public class CpuPowerStatsCollectorTest {
         mHandlerThread.start();
         mHandler = mHandlerThread.getThreadHandler();
         when(mMockKernelCpuStatsReader.isSupportedFeature()).thenReturn(true);
-        when(mUidResolver.mapUid(anyInt())).thenAnswer(invocation -> {
-            int uid = invocation.getArgument(0);
-            if (uid == ISOLATED_UID) {
-                return UID_2;
-            } else {
-                return uid;
-            }
-        });
         when(mConsumedEnergyRetriever.getEnergyConsumerIds(anyInt())).thenReturn(new int[0]);
+        mUidResolver.noteIsolatedUidAdded(ISOLATED_UID, UID_2);
     }
 
     @Test
@@ -268,8 +260,7 @@ public class CpuPowerStatsCollectorTest {
         mockEnergyConsumers();
 
         CpuPowerStatsCollector collector = createCollector(8, 0);
-        CpuPowerStatsLayout layout =
-                new CpuPowerStatsLayout();
+        CpuPowerStatsLayout layout = new CpuPowerStatsLayout();
         layout.fromExtras(collector.getPowerStatsDescriptor().extras);
 
         mockKernelCpuStats(new long[]{1111, 2222, 3333},
@@ -333,6 +324,45 @@ public class CpuPowerStatsCollectorTest {
                 .isEqualTo(78);
     }
 
+    @Test
+    public void isolatedUidReuse() {
+        mockCpuScalingPolicies(1);
+        mockPowerProfile();
+        mockEnergyConsumers();
+
+        CpuPowerStatsCollector collector = createCollector(8, 0);
+        CpuPowerStatsLayout layout = new CpuPowerStatsLayout();
+        layout.fromExtras(collector.getPowerStatsDescriptor().extras);
+
+        mockKernelCpuStats(new long[]{1111, 2222, 3333},
+                new SparseArray<>() {{
+                    put(UID_2, new long[]{100, 150});
+                    put(ISOLATED_UID, new long[]{10000, 20000});
+                }}, 0, 1234);
+
+        mMockClock.uptime = 1000;
+        collector.forceSchedule();
+        waitForIdle();
+
+        mUidResolver.noteIsolatedUidRemoved(ISOLATED_UID, UID_2);
+        mUidResolver.noteIsolatedUidAdded(ISOLATED_UID, UID_2);
+
+        mockKernelCpuStats(new long[]{5555, 4444, 3333},
+                new SparseArray<>() {{
+                    put(UID_2, new long[]{100, 150});
+                    put(ISOLATED_UID, new long[]{245, 528});
+                }}, 1234, 3421);
+
+        mMockClock.uptime = 2000;
+        collector.forceSchedule();
+        waitForIdle();
+
+        assertThat(layout.getUidTimeByPowerBracket(mCollectedStats.uidStats.get(UID_2), 0))
+                .isEqualTo(245);
+        assertThat(layout.getUidTimeByPowerBracket(mCollectedStats.uidStats.get(UID_2), 1))
+                .isEqualTo(528);
+    }
+
     private void mockCpuScalingPolicies(int clusterCount) {
         SparseArray<int[]> cpus = new SparseArray<>();
         SparseArray<int[]> freqs = new SparseArray<>();
diff --git a/services/tests/powerstatstests/src/com/android/server/power/stats/PowerStatsCollectorTest.java b/services/tests/powerstatstests/src/com/android/server/power/stats/PowerStatsCollectorTest.java
index df1200bb6b1a..89d6c1c66ca3 100644
--- a/services/tests/powerstatstests/src/com/android/server/power/stats/PowerStatsCollectorTest.java
+++ b/services/tests/powerstatstests/src/com/android/server/power/stats/PowerStatsCollectorTest.java
@@ -66,8 +66,7 @@ public class PowerStatsCollectorTest {
     public void setup() {
         mHandlerThread.start();
         mHandler = mHandlerThread.getThreadHandler();
-        mCollector = new PowerStatsCollector(mHandler,
-                60000,
+        mCollector = new PowerStatsCollector(mHandler, 60000, mock(PowerStatsUidResolver.class),
                 mMockClock) {
             @Override
             protected PowerStats collectStats() {