Return sorted NetworkStatsHistory am: 25c6d60f8a

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/18581718

Change-Id: I6335ae844d2fbbc1c325a92592da4f8d009c6fd3
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

framework-t/src/android/net/NetworkStatsHistory.java

@@ -58,6 +58,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Random;
+import java.util.TreeMap;
 
 /**
  * Collection of historical network statistics, recorded into equally-sized
@@ -253,6 +254,28 @@ public final class NetworkStatsHistory implements Parcelable {
                     + ", operations=" + operations
                     + "}";
         }
+
+        /**
+         * Add the given {@link Entry} with this instance and return a new {@link Entry}
+         * instance as the result.
+         *
+         * @hide
+         */
+        @NonNull
+        public Entry plus(@NonNull Entry another, long bucketDuration) {
+            if (this.bucketStart != another.bucketStart) {
+                throw new IllegalArgumentException("bucketStart " + this.bucketStart
+                        + " is not equal to " + another.bucketStart);
+            }
+            return new Entry(this.bucketStart,
+                    // Active time should not go over bucket duration.
+                    Math.min(this.activeTime + another.activeTime, bucketDuration),
+                    this.rxBytes + another.rxBytes,
+                    this.rxPackets + another.rxPackets,
+                    this.txBytes + another.txBytes,
+                    this.txPackets + another.txPackets,
+                    this.operations + another.operations);
+        }
     }
 
     /** @hide */
@@ -1109,14 +1132,8 @@ public final class NetworkStatsHistory implements Parcelable {
      * Builder class for {@link NetworkStatsHistory}.
      */
     public static final class Builder {
+        private final TreeMap<Long, Entry> mEntries;
         private final long mBucketDuration;
-        private final List<Long> mBucketStart;
-        private final List<Long> mActiveTime;
-        private final List<Long> mRxBytes;
-        private final List<Long> mRxPackets;
-        private final List<Long> mTxBytes;
-        private final List<Long> mTxPackets;
-        private final List<Long> mOperations;
 
         /**
          * Creates a new Builder with given bucket duration and initial capacity to construct
@@ -1127,66 +1144,31 @@ public final class NetworkStatsHistory implements Parcelable {
          */
         public Builder(long bucketDuration, int initialCapacity) {
             mBucketDuration = bucketDuration;
-            mBucketStart = new ArrayList<>(initialCapacity);
+            // Create a collection that is always sorted and can deduplicate items by the timestamp.
-            mActiveTime = new ArrayList<>(initialCapacity);
+            mEntries = new TreeMap<>();
-            mRxBytes = new ArrayList<>(initialCapacity);
-            mRxPackets = new ArrayList<>(initialCapacity);
-            mTxBytes = new ArrayList<>(initialCapacity);
-            mTxPackets = new ArrayList<>(initialCapacity);
-            mOperations = new ArrayList<>(initialCapacity);
-        }
-
-        private void addToElement(List<Long> list, int pos, long value) {
-            list.set(pos, list.get(pos) + value);
         }
 
         /**
-         * Add an {@link Entry} into the {@link NetworkStatsHistory} instance.
+         * Add an {@link Entry} into the {@link NetworkStatsHistory} instance. If the timestamp
+         * already exists, the given {@link Entry} will be combined into existing entry.
          *
-         * @param entry The target {@link Entry} object. The entry timestamp must be greater than
+         * @param entry The target {@link Entry} object.
-         *              that of any previously-added entry.
          * @return The builder object.
          */
         @NonNull
         public Builder addEntry(@NonNull Entry entry) {
-            final int lastBucket = mBucketStart.size() - 1;
+            final Entry existing = mEntries.get(entry.bucketStart);
-            final long lastBucketStart = (lastBucket != -1) ? mBucketStart.get(lastBucket) : 0;
+            if (existing != null) {
-
+                mEntries.put(entry.bucketStart, existing.plus(entry, mBucketDuration));
-            // If last bucket has the same timestamp, modify it instead of adding another bucket.
+            } else {
-            // This allows callers to pass in the same bucket twice (e.g., to accumulate
+                mEntries.put(entry.bucketStart, entry);
-            // data over time), but still requires that entries must be sorted.
+            }
-            // The importer will do this in case a rotated file has the same timestamp as
-            // the previous file.
-            if (lastBucket != -1 && entry.bucketStart == lastBucketStart) {
-                addToElement(mActiveTime, lastBucket, entry.activeTime);
-                addToElement(mRxBytes, lastBucket, entry.rxBytes);
-                addToElement(mRxPackets, lastBucket, entry.rxPackets);
-                addToElement(mTxBytes, lastBucket, entry.txBytes);
-                addToElement(mTxPackets, lastBucket, entry.txPackets);
-                addToElement(mOperations, lastBucket, entry.operations);
             return this;
         }
 
-            // Inserting in the middle is prohibited for performance reasons.
+        private static long sum(@NonNull long[] array) {
-            if (entry.bucketStart <= lastBucketStart) {
+            long sum = 0L;
-                throw new IllegalArgumentException("new bucket start " + entry.bucketStart
+            for (long entry : array) {
-                        + " must be greater than last bucket start " + lastBucketStart);
-            }
-
-            // Common case: add entries at the end of the list.
-            mBucketStart.add(entry.bucketStart);
-            mActiveTime.add(entry.activeTime);
-            mRxBytes.add(entry.rxBytes);
-            mRxPackets.add(entry.rxPackets);
-            mTxBytes.add(entry.txBytes);
-            mTxPackets.add(entry.txPackets);
-            mOperations.add(entry.operations);
-            return this;
-        }
-
-        private static long sum(@NonNull List<Long> list) {
-            long sum = 0;
-            for (long entry : list) {
                 sum += entry;
             }
             return sum;
@@ -1199,16 +1181,30 @@ public final class NetworkStatsHistory implements Parcelable {
          */
         @NonNull
         public NetworkStatsHistory build() {
-            return new NetworkStatsHistory(mBucketDuration,
+            int size = mEntries.size();
-                    CollectionUtils.toLongArray(mBucketStart),
+            final long[] bucketStart = new long[size];
-                    CollectionUtils.toLongArray(mActiveTime),
+            final long[] activeTime = new long[size];
-                    CollectionUtils.toLongArray(mRxBytes),
+            final long[] rxBytes = new long[size];
-                    CollectionUtils.toLongArray(mRxPackets),
+            final long[] rxPackets = new long[size];
-                    CollectionUtils.toLongArray(mTxBytes),
+            final long[] txBytes = new long[size];
-                    CollectionUtils.toLongArray(mTxPackets),
+            final long[] txPackets = new long[size];
-                    CollectionUtils.toLongArray(mOperations),
+            final long[] operations = new long[size];
-                    mBucketStart.size(),
+
-                    sum(mRxBytes) + sum(mTxBytes));
+            int i = 0;
+            for (Entry entry : mEntries.values()) {
+                bucketStart[i] = entry.bucketStart;
+                activeTime[i] = entry.activeTime;
+                rxBytes[i] = entry.rxBytes;
+                rxPackets[i] = entry.rxPackets;
+                txBytes[i] = entry.txBytes;
+                txPackets[i] = entry.txPackets;
+                operations[i] = entry.operations;
+                i++;
+            }
+
+            return new NetworkStatsHistory(mBucketDuration, bucketStart, activeTime,
+                    rxBytes, rxPackets, txBytes, txPackets, operations,
+                    size, sum(rxBytes) + sum(txBytes));
         }
     }
 }

service-t/src/com/android/server/net/NetworkStatsService.java

@@ -958,8 +958,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
             // First, read all legacy collections. This is OEM code and it can throw. Don't
             // commit any data to disk until all are read.
             for (int i = 0; i < migrations.length; i++) {
+                String errMsg = null;
+                Throwable exception = null;
                 final MigrationInfo migration = migrations[i];
+
+                // Read the collection from platform code, and using fallback method if throws.
+                try {
                     migration.collection = readPlatformCollectionForRecorder(migration.recorder);
+                } catch (Throwable e) {
+                    errMsg = "Failed to read stats from platform";
+                    exception = e;
+                }
 
                 // Also read the collection with legacy method
                 final NetworkStatsRecorder legacyRecorder = legacyRecorders[i];
@@ -968,19 +977,23 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
                 try {
                     legacyStats = legacyRecorder.getOrLoadCompleteLocked();
                 } catch (Throwable e) {
-                    Log.wtf(TAG, "Failed to read stats with legacy method", e);
+                    Log.wtf(TAG, "Failed to read stats with legacy method for recorder " + i, e);
-                    // Newer stats will be used here; that's the only thing that is usable
+                    if (exception != null) {
+                        throw exception;
+                    } else {
+                        // Use newer stats, since that's all that is available
                         continue;
                     }
+                }
 
-                String errMsg;
+                if (errMsg == null) {
-                Throwable exception = null;
                     try {
                         errMsg = compareStats(migration.collection, legacyStats);
                     } catch (Throwable e) {
                         errMsg = "Failed to compare migrated stats with all stats";
                         exception = e;
                     }
+                }
 
                 if (errMsg != null) {
                     Log.wtf(TAG, "NetworkStats import for migration " + i

tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt

@@ -27,7 +27,6 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import kotlin.test.assertEquals
-import kotlin.test.assertFailsWith
 
 @ConnectivityModuleTest
 @RunWith(JUnit4::class)
@@ -53,21 +52,36 @@ class NetworkStatsHistoryTest {
         statsSingle.assertEntriesEqual(entry1)
         assertEquals(DateUtils.HOUR_IN_MILLIS, statsSingle.bucketDuration)
 
-        // Verify the builder throws if the timestamp of added entry is not greater than
+        val statsMultiple = NetworkStatsHistory
-        // that of any previously-added entry.
-        assertFailsWith(IllegalArgumentException::class) {
-            NetworkStatsHistory
                 .Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0)
                 .addEntry(entry1).addEntry(entry2).addEntry(entry3)
                 .build()
+        assertEquals(DateUtils.SECOND_IN_MILLIS, statsMultiple.bucketDuration)
+        // Verify the entries exist and sorted.
+        statsMultiple.assertEntriesEqual(entry3, entry1, entry2)
     }
 
+    @Test
+    fun testBuilderSortAndDeduplicate() {
+        val entry1 = NetworkStatsHistory.Entry(10, 30, 40, 4, 50, 5, 60)
+        val entry2 = NetworkStatsHistory.Entry(30, 15, 3, 41, 7, 1, 0)
+        val entry3 = NetworkStatsHistory.Entry(30, 999, 11, 14, 31, 2, 80)
+        val entry4 = NetworkStatsHistory.Entry(10, 15, 1, 17, 5, 33, 10)
+        val entry5 = NetworkStatsHistory.Entry(6, 1, 9, 11, 29, 1, 7)
+
+        // Entries for verification.
+        // Note that active time of 2 + 3 is truncated to bucket duration since the active time
+        // should not go over bucket duration.
+        val entry2and3 = NetworkStatsHistory.Entry(30, 1000, 14, 55, 38, 3, 80)
+        val entry1and4 = NetworkStatsHistory.Entry(10, 45, 41, 21, 55, 38, 70)
+
         val statsMultiple = NetworkStatsHistory
                 .Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0)
-                .addEntry(entry3).addEntry(entry1).addEntry(entry2)
+                .addEntry(entry1).addEntry(entry2).addEntry(entry3)
-                .build()
+                .addEntry(entry4).addEntry(entry5).build()
         assertEquals(DateUtils.SECOND_IN_MILLIS, statsMultiple.bucketDuration)
-        statsMultiple.assertEntriesEqual(entry3, entry1, entry2)
+        // Verify the entries sorted and deduplicated.
+        statsMultiple.assertEntriesEqual(entry5, entry1and4, entry2and3)
     }
 
     fun NetworkStatsHistory.assertEntriesEqual(vararg entries: NetworkStatsHistory.Entry) {