From 442b6a80a410e974315f08657d275fa8a1a6d28b Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Tue, 20 Apr 2021 22:06:59 +0000 Subject: [PATCH] Save power component names in the summary parcel for MeasuredEnergySnapshot This is the second attempt at Ibb68bb2ee62678db5f3b30c4f2ef607006ccc81b, which broke a test Bug: 185534915 Test: mp :BatteryStatsViewer && adb shell am start -n com.android.frameworks.core.batterystatsviewer/.BatteryStatsViewerActivity and check that custom component is displayed as "GPU", not "CUSTOM_10000" Test: atest FrameworksServicesTests:com.android.server.am.MeasuredEnergySnapshotTest Change-Id: I9f65488a682233c49d47d73fc9426b76aab07627 --- .../android/internal/os/BatteryStatsImpl.java | 6 ++-- .../internal/power/MeasuredEnergyStats.java | 29 +++++++++++++++---- .../power/MeasuredEnergyStatsTest.java | 14 +++++---- .../server/am/MeasuredEnergySnapshot.java | 19 +++++++++++- .../server/am/MeasuredEnergySnapshotTest.java | 5 ++-- 5 files changed, 55 insertions(+), 18 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 82ec737de267c..02a290850b6f7 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -161,7 +161,7 @@ public class BatteryStatsImpl extends BatteryStats { private static final int MAGIC = 0xBA757475; // 'BATSTATS' // Current on-disk Parcel version - static final int VERSION = 197; + static final int VERSION = 198; // The maximum number of names wakelocks we will keep track of // per uid; once the limit is reached, we batch the remaining wakelocks @@ -15603,7 +15603,7 @@ public class BatteryStatsImpl extends BatteryStats { out.writeLong(mNextMaxDailyDeadlineMs); out.writeLong(mBatteryTimeToFullSeconds); - MeasuredEnergyStats.writeSummaryToParcel(mGlobalMeasuredEnergyStats, out, false); + MeasuredEnergyStats.writeSummaryToParcel(mGlobalMeasuredEnergyStats, out, false, false); mScreenOnTimer.writeSummaryFromParcelLocked(out, NOWREAL_SYS); mScreenDozeTimer.writeSummaryFromParcelLocked(out, NOWREAL_SYS); @@ -15928,7 +15928,7 @@ public class BatteryStatsImpl extends BatteryStats { out.writeInt(0); } - MeasuredEnergyStats.writeSummaryToParcel(u.mUidMeasuredEnergyStats, out, true); + MeasuredEnergyStats.writeSummaryToParcel(u.mUidMeasuredEnergyStats, out, true, true); final ArrayMap wakeStats = u.mWakelockStats.getMap(); int NW = wakeStats.size(); diff --git a/core/java/com/android/internal/power/MeasuredEnergyStats.java b/core/java/com/android/internal/power/MeasuredEnergyStats.java index 3153071fd75d3..42fb3f4529492 100644 --- a/core/java/com/android/internal/power/MeasuredEnergyStats.java +++ b/core/java/com/android/internal/power/MeasuredEnergyStats.java @@ -140,7 +140,7 @@ public class MeasuredEnergyStats { */ private MeasuredEnergyStats(int numIndices) { mAccumulatedChargeMicroCoulomb = new long[numIndices]; - mCustomBucketNames = new String[0]; + mCustomBucketNames = new String[numIndices - NUMBER_STANDARD_POWER_BUCKETS]; } /** Construct from parcel. */ @@ -290,7 +290,7 @@ public class MeasuredEnergyStats { * Create a MeasuredEnergyStats object from a summary parcel. * * Corresponding write performed by - * {@link #writeSummaryToParcel(MeasuredEnergyStats, Parcel, boolean)}. + * {@link #writeSummaryToParcel(MeasuredEnergyStats, Parcel, boolean, boolean)}. * * @return a new MeasuredEnergyStats object as described. * Returns null if the parcel indicates there is no data to populate. @@ -300,9 +300,14 @@ public class MeasuredEnergyStats { // Check if any MeasuredEnergyStats exists on the parcel if (arraySize == 0) return null; - final int numCustomBuckets = arraySize - NUMBER_STANDARD_POWER_BUCKETS; + final String[] customBucketNames; + if (in.readBoolean()) { + customBucketNames = in.readStringArray(); + } else { + customBucketNames = new String[0]; + } final MeasuredEnergyStats stats = new MeasuredEnergyStats( - new boolean[NUMBER_STANDARD_POWER_BUCKETS], new String[numCustomBuckets]); + new boolean[NUMBER_STANDARD_POWER_BUCKETS], customBucketNames); stats.readSummaryFromParcel(in, true); return stats; } @@ -315,7 +320,7 @@ public class MeasuredEnergyStats { * possible (not necessarily supported) standard and custom buckets. * * Corresponding write performed by - * {@link #writeSummaryToParcel(MeasuredEnergyStats, Parcel, boolean)}. + * {@link #writeSummaryToParcel(MeasuredEnergyStats, Parcel, boolean, boolean)}. * * @return a new MeasuredEnergyStats object as described. * Returns null if the stats contain no non-0 information (such as if template is null @@ -329,6 +334,12 @@ public class MeasuredEnergyStats { // Check if any MeasuredEnergyStats exists on the parcel if (arraySize == 0) return null; + boolean includesCustomBucketNames = in.readBoolean(); + if (includesCustomBucketNames) { + // Consume the array of custom bucket names. They are already included in the + // template. + in.readStringArray(); + } if (template == null) { // Nothing supported anymore. Create placeholder object just to consume the parcel data. final MeasuredEnergyStats mes = new MeasuredEnergyStats(arraySize); @@ -370,12 +381,18 @@ public class MeasuredEnergyStats { * and {@link #createAndReadSummaryFromParcel(Parcel, MeasuredEnergyStats)}. */ public static void writeSummaryToParcel(@Nullable MeasuredEnergyStats stats, - Parcel dest, boolean skipZero) { + Parcel dest, boolean skipZero, boolean skipCustomBucketNames) { if (stats == null) { dest.writeInt(0); return; } dest.writeInt(stats.getNumberOfIndices()); + if (!skipCustomBucketNames) { + dest.writeBoolean(true); + dest.writeStringArray(stats.getCustomBucketNames()); + } else { + dest.writeBoolean(false); + } stats.writeSummaryToParcel(dest, skipZero); } diff --git a/core/tests/coretests/src/com/android/internal/power/MeasuredEnergyStatsTest.java b/core/tests/coretests/src/com/android/internal/power/MeasuredEnergyStatsTest.java index f1edc871a56c1..2d894f59ee42f 100644 --- a/core/tests/coretests/src/com/android/internal/power/MeasuredEnergyStatsTest.java +++ b/core/tests/coretests/src/com/android/internal/power/MeasuredEnergyStatsTest.java @@ -159,7 +159,7 @@ public class MeasuredEnergyStatsTest { stats.updateCustomBucket(1, 60); final Parcel parcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false); + MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false, false); parcel.setDataPosition(0); MeasuredEnergyStats newStats = MeasuredEnergyStats.createAndReadSummaryFromParcel(parcel); @@ -175,6 +175,7 @@ public class MeasuredEnergyStatsTest { } assertEquals(POWER_DATA_UNAVAILABLE, newStats.getAccumulatedCustomBucketCharge(customBucketNames.length + 1)); + assertThat(newStats.getCustomBucketNames()).asList().containsExactly("A", "B"); parcel.recycle(); } @@ -201,7 +202,7 @@ public class MeasuredEnergyStatsTest { stats.updateCustomBucket(1, 316); final Parcel parcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false); + MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false, true); final boolean[] newsupportedStandardBuckets = new boolean[NUMBER_STANDARD_POWER_BUCKETS]; newsupportedStandardBuckets[POWER_BUCKET_SCREEN_ON] = true; @@ -234,6 +235,7 @@ public class MeasuredEnergyStatsTest { } assertEquals(POWER_DATA_UNAVAILABLE, newStats.getAccumulatedCustomBucketCharge(customBucketNames.length + 1)); + assertThat(newStats.getCustomBucketNames()).asList().containsExactly("A", "B"); parcel.recycle(); } @@ -251,7 +253,7 @@ public class MeasuredEnergyStatsTest { // Let's try parcelling with including zeros final Parcel includeZerosParcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, includeZerosParcel, false); + MeasuredEnergyStats.writeSummaryToParcel(stats, includeZerosParcel, false, false); includeZerosParcel.setDataPosition(0); MeasuredEnergyStats newStats = MeasuredEnergyStats.createAndReadSummaryFromParcel( @@ -275,7 +277,7 @@ public class MeasuredEnergyStatsTest { // Now let's try parcelling with skipping zeros final Parcel skipZerosParcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, skipZerosParcel, true); + MeasuredEnergyStats.writeSummaryToParcel(stats, skipZerosParcel, true, false); skipZerosParcel.setDataPosition(0); newStats = MeasuredEnergyStats.createAndReadSummaryFromParcel(skipZerosParcel); @@ -315,7 +317,7 @@ public class MeasuredEnergyStatsTest { stats.updateCustomBucket(1, 60); final Parcel parcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false); + MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false, true); parcel.setDataPosition(0); MeasuredEnergyStats newStats = @@ -344,7 +346,7 @@ public class MeasuredEnergyStatsTest { stats.updateStandardBucket(POWER_BUCKET_SCREEN_OTHER, 7L); final Parcel parcel = Parcel.obtain(); - MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false); + MeasuredEnergyStats.writeSummaryToParcel(stats, parcel, false, true); final boolean[] newSupportedStandardBuckets = new boolean[NUMBER_STANDARD_POWER_BUCKETS]; newSupportedStandardBuckets[POWER_BUCKET_SCREEN_ON] = true; diff --git a/services/core/java/com/android/server/am/MeasuredEnergySnapshot.java b/services/core/java/com/android/server/am/MeasuredEnergySnapshot.java index 65d47550a2edc..a9fca4f240269 100644 --- a/services/core/java/com/android/server/am/MeasuredEnergySnapshot.java +++ b/services/core/java/com/android/server/am/MeasuredEnergySnapshot.java @@ -345,12 +345,29 @@ public class MeasuredEnergySnapshot { for (int idx = 0; idx < size; idx++) { final EnergyConsumer consumer = mEnergyConsumers.valueAt(idx); if (consumer.type == (int) EnergyConsumerType.OTHER) { - names[consumerIndex++] = consumer.name; + names[consumerIndex++] = sanitizeCustomBucketName(consumer.name); } } return names; } + private String sanitizeCustomBucketName(String bucketName) { + if (bucketName == null) { + return ""; + } + StringBuilder sb = new StringBuilder(bucketName.length()); + for (char c : bucketName.toCharArray()) { + if (Character.isWhitespace(c)) { + sb.append(' '); + } else if (Character.isISOControl(c)) { + sb.append('_'); + } else { + sb.append(c); + } + } + return sb.toString(); + } + /** Determines the number of ordinals for a given {@link EnergyConsumerType}. */ private static int calculateNumOrdinals(@EnergyConsumerType int type, SparseArray idToConsumer) { diff --git a/services/tests/servicestests/src/com/android/server/am/MeasuredEnergySnapshotTest.java b/services/tests/servicestests/src/com/android/server/am/MeasuredEnergySnapshotTest.java index 6ca1102b23ef6..8c87506295f30 100644 --- a/services/tests/servicestests/src/com/android/server/am/MeasuredEnergySnapshotTest.java +++ b/services/tests/servicestests/src/com/android/server/am/MeasuredEnergySnapshotTest.java @@ -52,7 +52,7 @@ public final class MeasuredEnergySnapshotTest { private static final EnergyConsumer CONSUMER_OTHER_1 = createEnergyConsumer( 1, 1, EnergyConsumerType.OTHER, "HPU"); private static final EnergyConsumer CONSUMER_OTHER_2 = createEnergyConsumer( - 436, 2, EnergyConsumerType.OTHER, "IPU"); + 436, 2, EnergyConsumerType.OTHER, "IPU\n&\005"); private static final SparseArray ALL_ID_CONSUMER_MAP = createIdToConsumerMap( CONSUMER_DISPLAY, CONSUMER_OTHER_0, CONSUMER_OTHER_1, CONSUMER_OTHER_2); @@ -228,7 +228,8 @@ public final class MeasuredEnergySnapshotTest { @Test public void testGetOtherOrdinalNames() { final MeasuredEnergySnapshot snapshot = new MeasuredEnergySnapshot(ALL_ID_CONSUMER_MAP); - assertThat(snapshot.getOtherOrdinalNames()).asList().containsExactly("GPU", "HPU", "IPU"); + assertThat(snapshot.getOtherOrdinalNames()).asList() + .containsExactly("GPU", "HPU", "IPU &_"); } @Test