From 0726d59bc541c1fd09553f0adb31793e7c8e258c Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Fri, 12 Feb 2021 00:08:30 -0800 Subject: [PATCH 1/3] Add cpu proc file read throttling enabling/disabling To properly attribute the new MeasureEnergy data for CPU, the cpu kernel uid file reads needs to occur everytime the CPU MeasuredEnergy is read. If throttling is necessary, it will need to be handled by BatteryExternalStatsWorker for both the cpu kernel file reads and the MeasuredEnergy read to make sure the data is in sync. Bug: 180079165 Test: atest FrameworksCoreTests:com.android.internal.os.KernelCpuUidUserSysTimeReaderTest Change-Id: I90ebad48d780872a611605689e0d25678da66f27 --- .../android/internal/os/BatteryStatsImpl.java | 16 +++--- .../internal/os/KernelCpuUidTimeReader.java | 13 ++++- .../os/KernelCpuUidUserSysTimeReaderTest.java | 49 ++++++++++++++----- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 87820a89280c2..e599888c3bee4 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -12706,12 +12706,12 @@ public class BatteryStatsImpl extends BatteryStats { // When the battery is not on, we don't attribute the cpu times to any timers but we still // need to take the snapshots. if (!onBattery) { - mCpuUidUserSysTimeReader.readDelta(null); - mCpuUidFreqTimeReader.readDelta(null); + mCpuUidUserSysTimeReader.readDelta(false, null); + mCpuUidFreqTimeReader.readDelta(false, null); mNumAllUidCpuTimeReads += 2; if (mConstants.TRACK_CPU_ACTIVE_CLUSTER_TIME) { - mCpuUidActiveTimeReader.readDelta(null); - mCpuUidClusterTimeReader.readDelta(null); + mCpuUidActiveTimeReader.readDelta(false, null); + mCpuUidClusterTimeReader.readDelta(false, null); mNumAllUidCpuTimeReads += 2; } for (int cluster = mKernelCpuSpeedReaders.length - 1; cluster >= 0; --cluster) { @@ -12897,7 +12897,7 @@ public class BatteryStatsImpl extends BatteryStats { final long startTimeMs = mClocks.uptimeMillis(); final long elapsedRealtimeMs = mClocks.elapsedRealtime(); - mCpuUidUserSysTimeReader.readDelta((uid, timesUs) -> { + mCpuUidUserSysTimeReader.readDelta(false, (uid, timesUs) -> { long userTimeUs = timesUs[0], systemTimeUs = timesUs[1]; uid = mapUid(uid); @@ -13011,7 +13011,7 @@ public class BatteryStatsImpl extends BatteryStats { final long startTimeMs = mClocks.uptimeMillis(); final long elapsedRealtimeMs = mClocks.elapsedRealtime(); final List uidsToRemove = new ArrayList<>(); - mCpuUidFreqTimeReader.readDelta((uid, cpuFreqTimeMs) -> { + mCpuUidFreqTimeReader.readDelta(false, (uid, cpuFreqTimeMs) -> { uid = mapUid(uid); if (Process.isIsolated(uid)) { uidsToRemove.add(uid); @@ -13129,7 +13129,7 @@ public class BatteryStatsImpl extends BatteryStats { final long startTimeMs = mClocks.uptimeMillis(); final long elapsedRealtimeMs = mClocks.elapsedRealtime(); final List uidsToRemove = new ArrayList<>(); - mCpuUidActiveTimeReader.readDelta((uid, cpuActiveTimesMs) -> { + mCpuUidActiveTimeReader.readDelta(false, (uid, cpuActiveTimesMs) -> { uid = mapUid(uid); if (Process.isIsolated(uid)) { uidsToRemove.add(uid); @@ -13163,7 +13163,7 @@ public class BatteryStatsImpl extends BatteryStats { final long startTimeMs = mClocks.uptimeMillis(); final long elapsedRealtimeMs = mClocks.elapsedRealtime(); final List uidsToRemove = new ArrayList<>(); - mCpuUidClusterTimeReader.readDelta((uid, cpuClusterTimesMs) -> { + mCpuUidClusterTimeReader.readDelta(false, (uid, cpuClusterTimesMs) -> { uid = mapUid(uid); if (Process.isIsolated(uid)) { uidsToRemove.add(uid); diff --git a/core/java/com/android/internal/os/KernelCpuUidTimeReader.java b/core/java/com/android/internal/os/KernelCpuUidTimeReader.java index f7fad2c5bbaa1..4299f0936dae8 100644 --- a/core/java/com/android/internal/os/KernelCpuUidTimeReader.java +++ b/core/java/com/android/internal/os/KernelCpuUidTimeReader.java @@ -91,15 +91,24 @@ public abstract class KernelCpuUidTimeReader { * Reads the proc file, calling into the callback with a delta of time for each UID. * * @param cb The callback to invoke for each line of the proc file. If null,the data is - * consumed and subsequent calls to readDelta will provide a fresh delta. */ public void readDelta(@Nullable Callback cb) { + readDelta(false, cb); + } + + /** + * Reads the proc file, calling into the callback with a delta of time for each UID. + * + * @param force Ignore the throttling and force read the delta. + * @param cb The callback to invoke for each line of the proc file. If null,the data is + */ + public void readDelta(boolean force, @Nullable Callback cb) { if (!mThrottle) { readDeltaImpl(cb); return; } final long currTimeMs = SystemClock.elapsedRealtime(); - if (currTimeMs < mLastReadTimeMs + mMinTimeBetweenRead) { + if (!force && currTimeMs < mLastReadTimeMs + mMinTimeBetweenRead) { if (DEBUG) { Slog.d(mTag, "Throttle readDelta"); } diff --git a/core/tests/coretests/src/com/android/internal/os/KernelCpuUidUserSysTimeReaderTest.java b/core/tests/coretests/src/com/android/internal/os/KernelCpuUidUserSysTimeReaderTest.java index 8f81ea237a15e..7dca0cb92f9d5 100644 --- a/core/tests/coretests/src/com/android/internal/os/KernelCpuUidUserSysTimeReaderTest.java +++ b/core/tests/coretests/src/com/android/internal/os/KernelCpuUidUserSysTimeReaderTest.java @@ -93,35 +93,62 @@ public class KernelCpuUidUserSysTimeReaderTest { mReader.setThrottle(500); writeToFile(uidLines(mUids, mInitialTimes)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); assertEquals(6, mCallback.mData.size()); long[][] times1 = increaseTime(mInitialTimes); writeToFile(uidLines(mUids, times1)); mCallback.clear(); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); assertEquals(0, mCallback.mData.size()); + // TODO(b/180473895): Replace sleeps with injected simulated time. SystemClock.sleep(600); long[][] times2 = increaseTime(times1); writeToFile(uidLines(mUids, times2)); mCallback.clear(); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); assertEquals(6, mCallback.mData.size()); long[][] times3 = increaseTime(times2); writeToFile(uidLines(mUids, times3)); mCallback.clear(); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); assertEquals(0, mCallback.mData.size()); + + // Force the delta read, previously skipped increments should now be read + mCallback.clear(); + mReader.readDelta(true, mCallback); + assertEquals(6, mCallback.mData.size()); + + SystemClock.sleep(600); + + long[][] times4 = increaseTime(times3); + writeToFile(uidLines(mUids, times4)); + mCallback.clear(); + mReader.readDelta(true, mCallback); + assertEquals(6, mCallback.mData.size()); + + // Don't force the delta read, throttle should be set from last read. + long[][] times5 = increaseTime(times4); + writeToFile(uidLines(mUids, times5)); + mCallback.clear(); + mReader.readDelta(false, mCallback); + assertEquals(0, mCallback.mData.size()); + + SystemClock.sleep(600); + + mCallback.clear(); + mReader.readDelta(false, mCallback); + assertEquals(6, mCallback.mData.size()); } @Test public void testReadDelta() throws Exception { final long[][] times1 = mInitialTimes; writeToFile(uidLines(mUids, times1)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); for (int i = 0; i < mUids.length; i++) { mCallback.verify(mUids[i], times1[i]); } @@ -131,7 +158,7 @@ public class KernelCpuUidUserSysTimeReaderTest { // Verify that a second call will only return deltas. final long[][] times2 = increaseTime(times1); writeToFile(uidLines(mUids, times2)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); for (int i = 0; i < mUids.length; i++) { mCallback.verify(mUids[i], subtract(times2[i], times1[i])); } @@ -139,20 +166,20 @@ public class KernelCpuUidUserSysTimeReaderTest { mCallback.clear(); // Verify that there won't be a callback if the proc file values didn't change. - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); mCallback.verifyNoMoreInteractions(); mCallback.clear(); // Verify that calling with a null callback doesn't result in any crashes final long[][] times3 = increaseTime(times2); writeToFile(uidLines(mUids, times3)); - mReader.readDelta(null); + mReader.readDelta(false, null); // Verify that the readDelta call will only return deltas when // the previous call had null callback. final long[][] times4 = increaseTime(times3); writeToFile(uidLines(mUids, times4)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); for (int i = 0; i < mUids.length; i++) { mCallback.verify(mUids[i], subtract(times4[i], times3[i])); } @@ -165,7 +192,7 @@ public class KernelCpuUidUserSysTimeReaderTest { public void testReadDeltaWrongData() throws Exception { final long[][] times1 = mInitialTimes; writeToFile(uidLines(mUids, times1)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); for (int i = 0; i < mUids.length; i++) { mCallback.verify(mUids[i], times1[i]); } @@ -176,7 +203,7 @@ public class KernelCpuUidUserSysTimeReaderTest { final long[][] times2 = increaseTime(times1); times2[0][0] = 1000; writeToFile(uidLines(mUids, times2)); - mReader.readDelta(mCallback); + mReader.readDelta(false, mCallback); for (int i = 1; i < mUids.length; i++) { mCallback.verify(mUids[i], subtract(times2[i], times1[i])); } From 580d86680d7c50846e61ce07e0423943b296b56f Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Fri, 12 Feb 2021 03:10:02 -0800 Subject: [PATCH 2/3] Implement EnergyConsumer type to ids map for targeted querying The EnergyConsumer type to id map is built at boot, based on the EnergyConsumer info query. Using the map, BatteryExernalStatsWorker can selectively choose which EnergyConsumers to query based on EnergyConsumer type. Also, collect the EnergyConsumerResults asynchronously with a CompletableFuture. Fixes: 180029015 Bug: 180079165 Test: atest FrameworksServicesTests:BatteryExternalStatsWorkerTest Change-Id: I8d87c1da9170897065f866e7e1e14f72e0c73543 --- .../server/am/BatteryExternalStatsWorker.java | 116 ++++++++++++------ .../am/BatteryExternalStatsWorkerTest.java | 94 ++++++++++++-- 2 files changed, 168 insertions(+), 42 deletions(-) diff --git a/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java b/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java index 52bb55f12d799..fc28bfbea7109 100644 --- a/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java +++ b/services/core/java/com/android/server/am/BatteryExternalStatsWorker.java @@ -50,8 +50,6 @@ import com.android.server.LocalServices; import libcore.util.EmptyArray; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; @@ -149,7 +147,6 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { * Maps an {@link EnergyConsumerType} to it's corresponding {@link EnergyConsumer#id}s, * unless it is of {@link EnergyConsumer#type}=={@link EnergyConsumerType#OTHER} */ - // TODO(b/180029015): Hook this up (it isn't used yet) @GuardedBy("mWorkerLock") private @Nullable SparseArray mEnergyConsumerTypeToIdMap = null; @@ -209,11 +206,22 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { = populateEnergyConsumerSubsystemMapsLocked(); if (idToConsumer != null) { mMeasuredEnergySnapshot = new MeasuredEnergySnapshot(idToConsumer); - final EnergyConsumerResult[] initialEcrs = getEnergyConsumptionData(); - // According to spec, initialEcrs will include 0s for consumers that haven't - // used any energy yet, as long as they are supported; however, attributed uid - // energies will be absent if their energy is 0. - mMeasuredEnergySnapshot.updateAndGetDelta(initialEcrs); + try { + final EnergyConsumerResult[] initialEcrs = getEnergyConsumptionData().get( + EXTERNAL_STATS_SYNC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); + // According to spec, initialEcrs will include 0s for consumers that haven't + // used any energy yet, as long as they are supported; however, + // attributed uid energies will be absent if their energy is 0. + mMeasuredEnergySnapshot.updateAndGetDelta(initialEcrs); + } catch (TimeoutException | InterruptedException e) { + Slog.w(TAG, "timeout or interrupt reading initial getEnergyConsumedAsync: " + + e); + // Continue running, later attempts to query may be successful. + } catch (ExecutionException e) { + Slog.wtf(TAG, "exception reading initial getEnergyConsumedAsync: " + + e.getCause()); + // Continue running, later attempts to query may be successful. + } numCustomBuckets = mMeasuredEnergySnapshot.getNumOtherOrdinals(); supportedStdBuckets = getSupportedEnergyBuckets(idToConsumer); } @@ -498,6 +506,8 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { CompletableFuture modemFuture = CompletableFuture.completedFuture(null); boolean railUpdated = false; + CompletableFuture futureECRs = getMeasuredEnergyLocked(updateFlags); + if ((updateFlags & BatteryStatsImpl.ExternalStatsSync.UPDATE_WIFI) != 0) { // We were asked to fetch WiFi data. // Only fetch WiFi power data if it is supported. @@ -574,9 +584,23 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { Slog.w(TAG, "exception reading modem stats: " + e.getCause()); } - final MeasuredEnergySnapshot.MeasuredEnergyDeltaData measuredEnergyDeltas = - mMeasuredEnergySnapshot == null ? null : - mMeasuredEnergySnapshot.updateAndGetDelta(getMeasuredEnergyLocked(updateFlags)); + final MeasuredEnergySnapshot.MeasuredEnergyDeltaData measuredEnergyDeltas; + if (mMeasuredEnergySnapshot == null || futureECRs == null) { + measuredEnergyDeltas = null; + } else { + EnergyConsumerResult[] ecrs; + try { + ecrs = futureECRs.get(EXTERNAL_STATS_SYNC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); + } catch (TimeoutException | InterruptedException e) { + // TODO (b/180519623): Invalidate the MeasuredEnergy derived data until next reset. + Slog.w(TAG, "timeout or interrupt reading getEnergyConsumedAsync: " + e); + ecrs = null; + } catch (ExecutionException e) { + Slog.wtf(TAG, "exception reading getEnergyConsumedAsync: " + e.getCause()); + ecrs = null; + } + measuredEnergyDeltas = mMeasuredEnergySnapshot.updateAndGetDelta(ecrs); + } final long elapsedRealtime = SystemClock.elapsedRealtime(); final long uptime = SystemClock.uptimeMillis(); @@ -786,22 +810,29 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { return buckets; } - /** Get {@link EnergyConsumerResult}s with the latest energy usage since boot. */ + /** Get all {@link EnergyConsumerResult}s with the latest energy usage since boot. */ @GuardedBy("mWorkerLock") - private @Nullable EnergyConsumerResult[] getEnergyConsumptionData() { - try { - return mPowerStatsInternal.getEnergyConsumedAsync(new int[0]) - .get(EXTERNAL_STATS_SYNC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); - } catch (Exception e) { - Slog.e(TAG, "Failed to getEnergyConsumedAsync", e); - return null; - } + @Nullable + private CompletableFuture getEnergyConsumptionData() { + return getEnergyConsumptionData(new int[0]); + } + + /** + * Get {@link EnergyConsumerResult}s of the specified {@link EnergyConsumer} ids with the latest + * energy usage since boot. + */ + @GuardedBy("mWorkerLock") + @Nullable + private CompletableFuture getEnergyConsumptionData(int[] consumerIds) { + return mPowerStatsInternal.getEnergyConsumedAsync(consumerIds); } /** Fetch EnergyConsumerResult[] for supported subsystems based on the given updateFlags. */ + @VisibleForTesting @GuardedBy("mWorkerLock") - private @Nullable EnergyConsumerResult[] getMeasuredEnergyLocked(@ExternalUpdateFlag int flags) - { + @Nullable + public CompletableFuture getMeasuredEnergyLocked( + @ExternalUpdateFlag int flags) { if (mMeasuredEnergySnapshot == null || mPowerStatsInternal == null) return null; if (flags == UPDATE_ALL) { @@ -809,24 +840,27 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { return getEnergyConsumptionData(); } - final List energyConsumerIds = new ArrayList<>(); + final IntArray energyConsumerIds = new IntArray(); + if ((flags & UPDATE_CPU) != 0) { + addEnergyConsumerIdLocked(energyConsumerIds, EnergyConsumerType.CPU_CLUSTER); + } if ((flags & UPDATE_DISPLAY) != 0) { addEnergyConsumerIdLocked(energyConsumerIds, EnergyConsumerType.DISPLAY); } // TODO: Wifi, Bluetooth, etc., go here - if (energyConsumerIds.isEmpty()) { + if (energyConsumerIds.size() == 0) { return null; } - // TODO(b/180029015): Query specific subsystems from HAL based on energyConsumerIds.toArray - return getEnergyConsumptionData(); + return getEnergyConsumptionData(energyConsumerIds.toArray()); } @GuardedBy("mWorkerLock") private void addEnergyConsumerIdLocked( - List energyConsumerIds, @EnergyConsumerType int type) { - final int consumerId = 0; // TODO(b/180029015): Use mEnergyConsumerTypeToIdMap to get this - energyConsumerIds.add(consumerId); + IntArray energyConsumerIds, @EnergyConsumerType int type) { + final int[] consumerIds = mEnergyConsumerTypeToIdMap.get(type); + if (consumerIds == null) return; + energyConsumerIds.addAll(consumerIds); } /** Populates the cached type->ids map, and returns the (inverse) id->EnergyConsumer map. */ @@ -840,12 +874,10 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { return null; } - // TODO(b/180029015): Initialize typeToIds - // Maps type -> {ids} (1:n map, since multiple ids might have the same type) - // final SparseArray typeToIds = new SparseArray<>(); - // Maps id -> EnergyConsumer (1:1 map) final SparseArray idToConsumer = new SparseArray<>(energyConsumers.length); + // Maps type -> {ids} (1:n map, since multiple ids might have the same type) + final SparseArray tempTypeToId = new SparseArray<>(); // Add all expected EnergyConsumers to the maps for (final EnergyConsumer consumer : energyConsumers) { @@ -862,9 +894,23 @@ class BatteryExternalStatsWorker implements BatteryStatsImpl.ExternalStatsSync { } } idToConsumer.put(consumer.id, consumer); - // TODO(b/180029015): Also populate typeToIds map + + IntArray ids = tempTypeToId.get(consumer.type); + if (ids == null) { + ids = new IntArray(); + tempTypeToId.put(consumer.type, ids); + } + ids.add(consumer.id); + } + + mEnergyConsumerTypeToIdMap = new SparseArray<>(tempTypeToId.size()); + // Populate mEnergyConsumerTypeToIdMap with EnergyConsumer type to ids mappings + final int size = tempTypeToId.size(); + for (int i = 0; i < size; i++) { + final int consumerType = tempTypeToId.keyAt(i); + final int[] consumerIds = tempTypeToId.valueAt(i).toArray(); + mEnergyConsumerTypeToIdMap.put(consumerType, consumerIds); } - // TODO(b/180029015): Store typeToIds in mEnergyConsumerTypeToIdMap. return idToConsumer; } } diff --git a/services/tests/servicestests/src/com/android/server/am/BatteryExternalStatsWorkerTest.java b/services/tests/servicestests/src/com/android/server/am/BatteryExternalStatsWorkerTest.java index a946534f4ecdd..8d54ead757616 100644 --- a/services/tests/servicestests/src/com/android/server/am/BatteryExternalStatsWorkerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/BatteryExternalStatsWorkerTest.java @@ -16,6 +16,12 @@ package com.android.server.am; +import static com.android.internal.os.BatteryStatsImpl.ExternalStatsSync.UPDATE_ALL; +import static com.android.internal.os.BatteryStatsImpl.ExternalStatsSync.UPDATE_CPU; +import static com.android.internal.os.BatteryStatsImpl.ExternalStatsSync.UPDATE_DISPLAY; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import android.content.Context; @@ -27,6 +33,7 @@ import android.hardware.power.stats.EnergyMeasurement; import android.hardware.power.stats.PowerEntity; import android.hardware.power.stats.StateResidencyResult; import android.power.PowerStatsInternal; +import android.util.IntArray; import android.util.SparseArray; import androidx.test.InstrumentationRegistry; @@ -34,7 +41,9 @@ import androidx.test.InstrumentationRegistry; import com.android.internal.os.BatteryStatsImpl; import org.junit.Before; +import org.junit.Test; +import java.util.Arrays; import java.util.concurrent.CompletableFuture; /** @@ -58,6 +67,69 @@ public class BatteryExternalStatsWorkerTest { mBatteryStatsImpl); } + @Test + public void testTargetedEnergyConsumerQuerying() { + final int numCpuClusters = 4; + final int numOther = 3; + + final IntArray tempAllIds = new IntArray(); + // Add some energy consumers used by BatteryExternalStatsWorker. + final int displayId = mPowerStatsInternal.addEnergyConsumer(EnergyConsumerType.DISPLAY, 0, + "display"); + tempAllIds.add(displayId); + mPowerStatsInternal.incrementEnergyConsumption(displayId, 12345); + + final int[] cpuClusterIds = new int[numCpuClusters]; + for (int i = 0; i < numCpuClusters; i++) { + cpuClusterIds[i] = mPowerStatsInternal.addEnergyConsumer( + EnergyConsumerType.CPU_CLUSTER, i, "cpu_cluster" + i); + tempAllIds.add(cpuClusterIds[i]); + mPowerStatsInternal.incrementEnergyConsumption(cpuClusterIds[i], 1111 + i); + } + Arrays.sort(cpuClusterIds); + + final int[] otherIds = new int[numOther]; + for (int i = 0; i < numOther; i++) { + otherIds[i] = mPowerStatsInternal.addEnergyConsumer( + EnergyConsumerType.OTHER, i, "other" + i); + tempAllIds.add(otherIds[i]); + mPowerStatsInternal.incrementEnergyConsumption(otherIds[i], 3000 + i); + } + Arrays.sort(otherIds); + + final int[] allIds = tempAllIds.toArray(); + Arrays.sort(allIds); + + // Inform BESW that PowerStatsInternal is ready to query + mBatteryExternalStatsWorker.systemServicesReady(); + + final EnergyConsumerResult[] displayResults = + mBatteryExternalStatsWorker.getMeasuredEnergyLocked(UPDATE_DISPLAY).getNow(null); + // Results should only have the display energy consumer + assertEquals(1, displayResults.length); + assertEquals(displayId, displayResults[0].id); + + final EnergyConsumerResult[] cpuResults = + mBatteryExternalStatsWorker.getMeasuredEnergyLocked(UPDATE_CPU).getNow(null); + // Results should only have the cpu cluster energy consumers + final int[] receivedCpuIds = new int[cpuResults.length]; + for (int i = 0; i < cpuResults.length; i++) { + receivedCpuIds[i] = cpuResults[i].id; + } + Arrays.sort(receivedCpuIds); + assertArrayEquals(cpuClusterIds, receivedCpuIds); + + final EnergyConsumerResult[] allResults = + mBatteryExternalStatsWorker.getMeasuredEnergyLocked(UPDATE_ALL).getNow(null); + // All energy consumer results should be available + final int[] receivedAllIds = new int[allResults.length]; + for (int i = 0; i < allResults.length; i++) { + receivedAllIds[i] = allResults[i].id; + } + Arrays.sort(receivedAllIds); + assertArrayEquals(allIds, receivedAllIds); + } + public class TestInjector extends BatteryExternalStatsWorker.Injector { public TestInjector(Context context) { super(context); @@ -79,9 +151,8 @@ public class BatteryExternalStatsWorkerTest { } public class TestPowerStatsInternal extends PowerStatsInternal { - private final SparseArray mEnergyConsumers = new SparseArray<>(); - private final SparseArray mEnergyConsumerResults = - new SparseArray<>(); + private final SparseArray mEnergyConsumers = new SparseArray(); + private final SparseArray mEnergyConsumerResults = new SparseArray(); private final int mTimeSinceBoot = 0; @Override @@ -98,10 +169,19 @@ public class BatteryExternalStatsWorkerTest { public CompletableFuture getEnergyConsumedAsync( int[] energyConsumerIds) { final CompletableFuture future = new CompletableFuture(); - final int size = mEnergyConsumerResults.size(); - final EnergyConsumerResult[] results = new EnergyConsumerResult[size]; - for (int i = 0; i < size; i++) { - results[i] = mEnergyConsumerResults.valueAt(i); + final EnergyConsumerResult[] results; + final int length = energyConsumerIds.length; + if (length == 0) { + final int size = mEnergyConsumerResults.size(); + results = new EnergyConsumerResult[size]; + for (int i = 0; i < size; i++) { + results[i] = mEnergyConsumerResults.valueAt(i); + } + } else { + results = new EnergyConsumerResult[length]; + for (int i = 0; i < length; i++) { + results[i] = mEnergyConsumerResults.get(energyConsumerIds[i]); + } } future.complete(results); return future; From af23d81ff289a83fc4cbed7a250d9cfc7ad37abb Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Fri, 12 Feb 2021 10:30:35 -0800 Subject: [PATCH 3/3] Make CpuPowerCalculator's Estimators available to com.android.internal BatteryStats needs the power_profile Cpu constants to smear the measured CPU Cluster energies across UIDs. Using the CpuPowerProfileCalculator to get the calculation decreases the chance of someone accidentally diverging estimated and measured CPU power calculations. Bug: 180079165 Test: tbd Change-Id: I474cd019d177bd07f2ae48a47b83a82e4074ed4f --- .../internal/os/CpuPowerCalculator.java | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/core/java/com/android/internal/os/CpuPowerCalculator.java b/core/java/com/android/internal/os/CpuPowerCalculator.java index 45d81280af4ad..97f727ba72c5d 100644 --- a/core/java/com/android/internal/os/CpuPowerCalculator.java +++ b/core/java/com/android/internal/os/CpuPowerCalculator.java @@ -124,15 +124,15 @@ public class CpuPowerCalculator extends PowerCalculator { long durationMs = (u.getUserCpuTimeUs(statsType) + u.getSystemCpuTimeUs(statsType)) / 1000; // Constant battery drain when CPU is active - double powerMah = mCpuActivePowerEstimator.calculatePower(u.getCpuActiveTime()); + double powerMah = calculateActiveCpuPowerMah(u.getCpuActiveTime()); // Additional per-cluster battery drain long[] cpuClusterTimes = u.getCpuClusterTimes(); if (cpuClusterTimes != null) { if (cpuClusterTimes.length == mNumCpuClusters) { for (int cluster = 0; cluster < mNumCpuClusters; cluster++) { - double power = mPerClusterPowerEstimators[cluster] - .calculatePower(cpuClusterTimes[cluster]); + double power = calculatePerCpuClusterPowerMah(cluster, + cpuClusterTimes[cluster]); powerMah += power; if (DEBUG) { Log.d(TAG, "UID " + u.getUid() + ": CPU cluster #" + cluster @@ -151,8 +151,8 @@ public class CpuPowerCalculator extends PowerCalculator { final int speedsForCluster = mPerCpuFreqPowerEstimators[cluster].length; for (int speed = 0; speed < speedsForCluster; speed++) { final long timeUs = u.getTimeAtCpuSpeed(cluster, speed, statsType); - final double power = - mPerCpuFreqPowerEstimators[cluster][speed].calculatePower(timeUs / 1000); + final double power = calculatePerCpuFreqPowerMah(cluster, speed, + timeUs / 1000); if (DEBUG) { Log.d(TAG, "UID " + u.getUid() + ": CPU cluster #" + cluster + " step #" + speed + " timeUs=" + timeUs + " power=" @@ -207,4 +207,39 @@ public class CpuPowerCalculator extends PowerCalculator { result.powerMah = powerMah; result.packageWithHighestDrain = packageWithHighestDrain; } + + /** + * Calculates active CPU power consumption. + * + * @param durationsMs duration of CPU usage. + * @return a double in milliamp-hours of estimated active CPU power consumption. + */ + public double calculateActiveCpuPowerMah(long durationsMs) { + return mCpuActivePowerEstimator.calculatePower(durationsMs); + } + + /** + * Calculates CPU cluster power consumption. + * + * @param cluster CPU cluster used. + * @param clusterDurationMs duration of CPU cluster usage. + * @return a double in milliamp-hours of estimated CPU cluster power consumption. + */ + public double calculatePerCpuClusterPowerMah(int cluster, long clusterDurationMs) { + return mPerClusterPowerEstimators[cluster].calculatePower(clusterDurationMs); + } + + /** + * Calculates CPU cluster power consumption at a specific speedstep. + * + * @param cluster CPU cluster used. + * @param speedStep which speedstep used. + * @param clusterSpeedDurationsMs duration of CPU cluster usage at the specified speed step. + * @return a double in milliamp-hours of estimated CPU cluster-speed power consumption. + */ + public double calculatePerCpuFreqPowerMah(int cluster, int speedStep, + long clusterSpeedDurationsMs) { + return mPerCpuFreqPowerEstimators[cluster][speedStep].calculatePower( + clusterSpeedDurationsMs); + } }