From c22fb33347291512d081a833839c2a4d9a50c2d6 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Mon, 8 May 2023 14:40:59 -0700 Subject: Avoid redundant overwrite of value in TimeSparseArray TimeSparseArray complains when a value for a given time is overwritten to warn of possible mistakes. WakingActivityHistory#recordActivity was overwriting the value for a given time by itself. Added unit tests to exercise this code path. Test: atest FrameworksServicesTests:WakingActivityHistoryTest Bug: 279599693 Change-Id: Icf95b8964981b891a83e461aee620071497b81f2 --- .../server/power/stats/wakeups/CpuWakeupStats.java | 7 +- .../stats/wakeups/WakingActivityHistoryTest.java | 154 +++++++++++++++++++++ 2 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/power/stats/wakeups/WakingActivityHistoryTest.java diff --git a/services/core/java/com/android/server/power/stats/wakeups/CpuWakeupStats.java b/services/core/java/com/android/server/power/stats/wakeups/CpuWakeupStats.java index eb6d28e76ff8..73ab7822ac39 100644 --- a/services/core/java/com/android/server/power/stats/wakeups/CpuWakeupStats.java +++ b/services/core/java/com/android/server/power/stats/wakeups/CpuWakeupStats.java @@ -403,10 +403,12 @@ public class CpuWakeupStats { * This class stores recent unattributed activity history per subsystem. * The activity is stored as a mapping of subsystem to timestamp to uid to procstate. */ - private static final class WakingActivityHistory { + @VisibleForTesting + static final class WakingActivityHistory { private static final long WAKING_ACTIVITY_RETENTION_MS = TimeUnit.MINUTES.toMillis(10); - private SparseArray> mWakingActivity = + @VisibleForTesting + final SparseArray> mWakingActivity = new SparseArray<>(); void recordActivity(int subsystem, long elapsedRealtime, SparseIntArray uidProcStates) { @@ -430,7 +432,6 @@ public class CpuWakeupStats { uidsToBlame.put(uid, uidProcStates.valueAt(i)); } } - wakingActivity.put(elapsedRealtime, uidsToBlame); } // Limit activity history per subsystem to the last WAKING_ACTIVITY_RETENTION_MS. // Note that the last activity is always present, even if it occurred before diff --git a/services/tests/servicestests/src/com/android/server/power/stats/wakeups/WakingActivityHistoryTest.java b/services/tests/servicestests/src/com/android/server/power/stats/wakeups/WakingActivityHistoryTest.java new file mode 100644 index 000000000000..cba7dbe2d02b --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/power/stats/wakeups/WakingActivityHistoryTest.java @@ -0,0 +1,154 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.power.stats.wakeups; + +import static com.google.common.truth.Truth.assertThat; + +import android.util.SparseIntArray; +import android.util.TimeSparseArray; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import com.android.server.power.stats.wakeups.CpuWakeupStats.WakingActivityHistory; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class WakingActivityHistoryTest { + + private static boolean areSame(SparseIntArray a, SparseIntArray b) { + if (a == b) { + return true; + } + if (a == null || b == null) { + return false; + } + final int lenA = a.size(); + if (b.size() != lenA) { + return false; + } + for (int i = 0; i < lenA; i++) { + if (a.keyAt(i) != b.keyAt(i)) { + return false; + } + if (a.valueAt(i) != b.valueAt(i)) { + return false; + } + } + return true; + } + + @Test + public void recordActivityAppendsUids() { + final WakingActivityHistory history = new WakingActivityHistory(); + final int subsystem = 42; + final long timestamp = 54; + + final SparseIntArray uids = new SparseIntArray(); + uids.put(1, 3); + uids.put(5, 2); + + history.recordActivity(subsystem, timestamp, uids); + + assertThat(history.mWakingActivity.size()).isEqualTo(1); + assertThat(history.mWakingActivity.contains(subsystem)).isTrue(); + assertThat(history.mWakingActivity.contains(subsystem + 1)).isFalse(); + assertThat(history.mWakingActivity.contains(subsystem - 1)).isFalse(); + + final TimeSparseArray recordedHistory = history.mWakingActivity.get( + subsystem); + + assertThat(recordedHistory.size()).isEqualTo(1); + assertThat(recordedHistory.indexOfKey(timestamp - 1)).isLessThan(0); + assertThat(recordedHistory.indexOfKey(timestamp)).isAtLeast(0); + assertThat(recordedHistory.indexOfKey(timestamp + 1)).isLessThan(0); + + SparseIntArray recordedUids = recordedHistory.get(timestamp); + assertThat(recordedUids).isNotSameInstanceAs(uids); + assertThat(areSame(recordedUids, uids)).isTrue(); + + uids.put(1, 7); + uids.clear(); + uids.put(10, 12); + uids.put(17, 53); + + history.recordActivity(subsystem, timestamp, uids); + + recordedUids = recordedHistory.get(timestamp); + + assertThat(recordedUids.size()).isEqualTo(4); + assertThat(recordedUids.indexOfKey(1)).isAtLeast(0); + assertThat(recordedUids.get(5, -1)).isEqualTo(2); + assertThat(recordedUids.get(10, -1)).isEqualTo(12); + assertThat(recordedUids.get(17, -1)).isEqualTo(53); + } + + @Test + public void recordActivityDoesNotDeleteExistingUids() { + final WakingActivityHistory history = new WakingActivityHistory(); + final int subsystem = 42; + long timestamp = 101; + + final SparseIntArray uids = new SparseIntArray(); + uids.put(1, 17); + uids.put(15, 2); + uids.put(62, 31); + + history.recordActivity(subsystem, timestamp, uids); + + assertThat(history.mWakingActivity.size()).isEqualTo(1); + assertThat(history.mWakingActivity.contains(subsystem)).isTrue(); + assertThat(history.mWakingActivity.contains(subsystem + 1)).isFalse(); + assertThat(history.mWakingActivity.contains(subsystem - 1)).isFalse(); + + final TimeSparseArray recordedHistory = history.mWakingActivity.get( + subsystem); + + assertThat(recordedHistory.size()).isEqualTo(1); + assertThat(recordedHistory.indexOfKey(timestamp - 1)).isLessThan(0); + assertThat(recordedHistory.indexOfKey(timestamp)).isAtLeast(0); + assertThat(recordedHistory.indexOfKey(timestamp + 1)).isLessThan(0); + + SparseIntArray recordedUids = recordedHistory.get(timestamp); + assertThat(recordedUids).isNotSameInstanceAs(uids); + assertThat(areSame(recordedUids, uids)).isTrue(); + + uids.delete(1); + uids.delete(15); + uids.put(85, 39); + + history.recordActivity(subsystem, timestamp, uids); + recordedUids = recordedHistory.get(timestamp); + + assertThat(recordedUids.size()).isEqualTo(4); + assertThat(recordedUids.get(1, -1)).isEqualTo(17); + assertThat(recordedUids.get(15, -1)).isEqualTo(2); + assertThat(recordedUids.get(62, -1)).isEqualTo(31); + assertThat(recordedUids.get(85, -1)).isEqualTo(39); + + uids.clear(); + history.recordActivity(subsystem, timestamp, uids); + recordedUids = recordedHistory.get(timestamp); + + assertThat(recordedUids.size()).isEqualTo(4); + assertThat(recordedUids.get(1, -1)).isEqualTo(17); + assertThat(recordedUids.get(15, -1)).isEqualTo(2); + assertThat(recordedUids.get(62, -1)).isEqualTo(31); + assertThat(recordedUids.get(85, -1)).isEqualTo(39); + } +} -- cgit v1.2.3-59-g8ed1b