From c6dcd3e42e351538af687c0d6e669780097d0b04 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 5 Oct 2022 12:06:38 -0700 Subject: [PATCH] Refactoring and tweaking TimeSparseArray Tweaking the returned result from the boundary condition in closestIndexOnOrAfter to be conceptually similar to closestIndexOnOrBefore. This should also make it simpler to use by callers. Also exposing the class more broadly by moving it to android.util Test: atest FrameworksCoreTests:TimeSparseArrayTest Bug: 195684213 Change-Id: I44a06db8792d590be0eea4de39bc81387f2baac8 --- .../{app/usage => util}/TimeSparseArray.java | 47 +++----- .../src/android/util/TimeSparseArrayTest.java | 103 ++++++++++++++++++ .../server/usage/UsageStatsDatabaseTest.java | 2 +- .../server/usage/UsageStatsDatabase.java | 2 +- .../server/usage/UserUsageStatsService.java | 2 +- 5 files changed, 121 insertions(+), 35 deletions(-) rename core/java/android/{app/usage => util}/TimeSparseArray.java (65%) create mode 100644 core/tests/coretests/src/android/util/TimeSparseArrayTest.java diff --git a/core/java/android/app/usage/TimeSparseArray.java b/core/java/android/util/TimeSparseArray.java similarity index 65% rename from core/java/android/app/usage/TimeSparseArray.java rename to core/java/android/util/TimeSparseArray.java index 2bd6b24174721..6efc683ddf3d9 100644 --- a/core/java/android/app/usage/TimeSparseArray.java +++ b/core/java/android/util/TimeSparseArray.java @@ -14,13 +14,11 @@ * under the License. */ -package android.app.usage; - -import android.util.LongSparseArray; -import android.util.Slog; +package android.util; /** * An array that indexes by a long timestamp, representing milliseconds since the epoch. + * @param The type of values this container maps to a timestamp. * * {@hide} */ @@ -29,53 +27,41 @@ public class TimeSparseArray extends LongSparseArray { private boolean mWtfReported; - public TimeSparseArray() { - super(); - } - /** * Finds the index of the first element whose timestamp is greater or equal to * the given time. * * @param time The timestamp for which to search the array. - * @return The index of the matched element, or -1 if no such match exists. + * @return The smallest {@code index} for which {@code (keyAt(index) >= timeStamp)} is + * {@code true}, or {@link #size() size} if no such {@code index} exists. */ public int closestIndexOnOrAfter(long time) { - // This is essentially a binary search, except that if no match is found - // the closest index is returned. final int size = size(); + int result = size; int lo = 0; int hi = size - 1; - int mid = -1; - long key = -1; while (lo <= hi) { - mid = lo + ((hi - lo) / 2); - key = keyAt(mid); + final int mid = lo + ((hi - lo) / 2); + final long key = keyAt(mid); if (time > key) { lo = mid + 1; } else if (time < key) { hi = mid - 1; + result = mid; } else { return mid; } } - - if (time < key) { - return mid; - } else if (time > key && lo < size) { - return lo; - } else { - return -1; - } + return result; } /** * {@inheritDoc} * - *

As this container is being used only to keep {@link android.util.AtomicFile files}, - * there should not be any collisions. Reporting a {@link Slog#wtf(String, String)} in case that - * happens, as that will lead to one whole file being dropped. + *

This container can store only one value for each timestamp. And so ideally, the caller + * should ensure that there are no collisions. Reporting a {@link Slog#wtf(String, String)} + * if that happens, as that will lead to the previous value being overwritten. */ @Override public void put(long key, E value) { @@ -93,16 +79,13 @@ public class TimeSparseArray extends LongSparseArray { * the given time. * * @param time The timestamp for which to search the array. - * @return The index of the matched element, or -1 if no such match exists. + * @return The largest {@code index} for which {@code (keyAt(index) <= timeStamp)} is + * {@code true}, or -1 if no such {@code index} exists. */ public int closestIndexOnOrBefore(long time) { final int index = closestIndexOnOrAfter(time); - if (index < 0) { - // Everything is larger, so we use the last element, or -1 if the list is empty. - return size() - 1; - } - if (keyAt(index) == time) { + if (index < size() && keyAt(index) == time) { return index; } return index - 1; diff --git a/core/tests/coretests/src/android/util/TimeSparseArrayTest.java b/core/tests/coretests/src/android/util/TimeSparseArrayTest.java new file mode 100644 index 0000000000000..c8e2364c48cdc --- /dev/null +++ b/core/tests/coretests/src/android/util/TimeSparseArrayTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2020 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 android.util; + +import static com.google.common.truth.Truth.assertThat; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * Tests for {@link TimeSparseArray}. + * This class only tests subclass specific functionality. Tests for the super class + * {@link LongSparseArray} should be covered under {@link LongSparseArrayTest}. + */ +@SmallTest +@RunWith(AndroidJUnit4.class) +public class TimeSparseArrayTest { + + @Test + public void closestIndexOnOrAfter() { + final TimeSparseArray timeSparseArray = new TimeSparseArray<>(); + + // Values don't matter for this test. + timeSparseArray.put(51, new Object()); + timeSparseArray.put(10, new Object()); + timeSparseArray.put(59, new Object()); + + assertThat(timeSparseArray.size()).isEqualTo(3); + + // Testing any number arbitrarily smaller than 10. + assertThat(timeSparseArray.closestIndexOnOrAfter(-141213)).isEqualTo(0); + for (long time = -43; time <= 10; time++) { + assertThat(timeSparseArray.closestIndexOnOrAfter(time)).isEqualTo(0); + } + + for (long time = 11; time <= 51; time++) { + assertThat(timeSparseArray.closestIndexOnOrAfter(time)).isEqualTo(1); + } + + for (long time = 52; time <= 59; time++) { + assertThat(timeSparseArray.closestIndexOnOrAfter(time)).isEqualTo(2); + } + + for (long time = 60; time <= 102; time++) { + assertThat(timeSparseArray.closestIndexOnOrAfter(time)).isEqualTo(3); + } + // Testing any number arbitrarily larger than 59. + assertThat(timeSparseArray.closestIndexOnOrAfter(15332)).isEqualTo(3); + } + + @Test + public void closestIndexOnOrBefore() { + final TimeSparseArray timeSparseArray = new TimeSparseArray<>(); + + // Values don't matter for this test. + timeSparseArray.put(21, new Object()); + timeSparseArray.put(4, new Object()); + timeSparseArray.put(91, new Object()); + timeSparseArray.put(39, new Object()); + + assertThat(timeSparseArray.size()).isEqualTo(4); + + // Testing any number arbitrarily smaller than 4. + assertThat(timeSparseArray.closestIndexOnOrBefore(-1478133)).isEqualTo(-1); + for (long time = -42; time < 4; time++) { + assertThat(timeSparseArray.closestIndexOnOrBefore(time)).isEqualTo(-1); + } + + for (long time = 4; time < 21; time++) { + assertThat(timeSparseArray.closestIndexOnOrBefore(time)).isEqualTo(0); + } + + for (long time = 21; time < 39; time++) { + assertThat(timeSparseArray.closestIndexOnOrBefore(time)).isEqualTo(1); + } + + for (long time = 39; time < 91; time++) { + assertThat(timeSparseArray.closestIndexOnOrBefore(time)).isEqualTo(2); + } + + for (long time = 91; time < 109; time++) { + assertThat(timeSparseArray.closestIndexOnOrBefore(time)).isEqualTo(3); + } + // Testing any number arbitrarily larger than 91. + assertThat(timeSparseArray.closestIndexOnOrBefore(1980732)).isEqualTo(3); + } +} diff --git a/services/tests/servicestests/src/com/android/server/usage/UsageStatsDatabaseTest.java b/services/tests/servicestests/src/com/android/server/usage/UsageStatsDatabaseTest.java index bc2c57e8bf848..3adee0f06761e 100644 --- a/services/tests/servicestests/src/com/android/server/usage/UsageStatsDatabaseTest.java +++ b/services/tests/servicestests/src/com/android/server/usage/UsageStatsDatabaseTest.java @@ -24,7 +24,6 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; -import android.app.usage.TimeSparseArray; import android.app.usage.UsageEvents.Event; import android.app.usage.UsageStats; import android.app.usage.UsageStatsManager; @@ -32,6 +31,7 @@ import android.content.Context; import android.content.res.Configuration; import android.test.suitebuilder.annotation.SmallTest; import android.util.AtomicFile; +import android.util.TimeSparseArray; import androidx.test.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; diff --git a/services/usage/java/com/android/server/usage/UsageStatsDatabase.java b/services/usage/java/com/android/server/usage/UsageStatsDatabase.java index 26a1e9d8af114..361b1c06a6b5b 100644 --- a/services/usage/java/com/android/server/usage/UsageStatsDatabase.java +++ b/services/usage/java/com/android/server/usage/UsageStatsDatabase.java @@ -18,7 +18,6 @@ package com.android.server.usage; import android.annotation.NonNull; import android.annotation.Nullable; -import android.app.usage.TimeSparseArray; import android.app.usage.UsageEvents; import android.app.usage.UsageStats; import android.app.usage.UsageStatsManager; @@ -29,6 +28,7 @@ import android.util.ArraySet; import android.util.AtomicFile; import android.util.Slog; import android.util.SparseArray; +import android.util.TimeSparseArray; import android.util.TimeUtils; import com.android.internal.annotations.VisibleForTesting; diff --git a/services/usage/java/com/android/server/usage/UserUsageStatsService.java b/services/usage/java/com/android/server/usage/UserUsageStatsService.java index 34c6c1623eeca..92032086dc244 100644 --- a/services/usage/java/com/android/server/usage/UserUsageStatsService.java +++ b/services/usage/java/com/android/server/usage/UserUsageStatsService.java @@ -34,7 +34,6 @@ import android.annotation.Nullable; import android.app.usage.ConfigurationStats; import android.app.usage.EventList; import android.app.usage.EventStats; -import android.app.usage.TimeSparseArray; import android.app.usage.UsageEvents; import android.app.usage.UsageEvents.Event; import android.app.usage.UsageStats; @@ -50,6 +49,7 @@ import android.util.AtomicFile; import android.util.Slog; import android.util.SparseArrayMap; import android.util.SparseIntArray; +import android.util.TimeSparseArray; import com.android.internal.util.ArrayUtils; import com.android.internal.util.CollectionUtils;