Merge changes Ia47e566b,Ib94d79a9

* changes:
  Separate connectivity event buffer for bug reports
  Extract RingBuffer class from NetdEventListenerService

4 files changed, 262 insertions, 28 deletions

diff --git a/core/java/com/android/internal/util/RingBuffer.java b/core/java/com/android/internal/util/RingBuffer.java
new file mode 100644
index 000000000000..ad84353f23a9
--- /dev/null
+++ b/core/java/com/android/internal/util/RingBuffer.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2017 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.internal.util;
+
+import static com.android.internal.util.Preconditions.checkArgumentPositive;
+
+import java.lang.reflect.Array;
+import java.util.Arrays;
+
+/**
+ * A simple ring buffer structure with bounded capacity backed by an array.
+ * Events can always be added at the logical end of the buffer. If the buffer is
+ * full, oldest events are dropped when new events are added.
+ * {@hide}
+ */
+public class RingBuffer<T> {
+
+    // Array for storing events.
+    private final T[] mBuffer;
+    // Cursor keeping track of the logical end of the array. This cursor never
+    // wraps and instead keeps track of the total number of append() operations.
+    private long mCursor = 0;
+
+    public RingBuffer(Class<T> c, int capacity) {
+        checkArgumentPositive(capacity, "A RingBuffer cannot have 0 capacity");
+        // Java cannot create generic arrays without a runtime hint.
+        mBuffer = (T[]) Array.newInstance(c, capacity);
+    }
+
+    public int size() {
+        return (int) Math.min(mBuffer.length, (long) mCursor);
+    }
+
+    public void append(T t) {
+        mBuffer[indexOf(mCursor++)] = t;
+    }
+
+    public T[] toArray() {
+        // Only generic way to create a T[] from another T[]
+        T[] out = Arrays.copyOf(mBuffer, size(), (Class<T[]>) mBuffer.getClass());
+        // Reverse iteration from youngest event to oldest event.
+        long inCursor = mCursor - 1;
+        int outIdx = out.length - 1;
+        while (outIdx >= 0) {
+            out[outIdx--] = (T) mBuffer[indexOf(inCursor--)];
+        }
+        return out;
+    }
+
+    private int indexOf(long cursor) {
+        return (int) Math.abs(cursor % mBuffer.length);
+    }
+}
diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java
index 475d786aedd1..f2445fa36006 100644
--- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java
+++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java
@@ -34,6 +34,7 @@ import android.util.Base64;
 import android.util.Log;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.util.RingBuffer;
 import com.android.internal.util.TokenBucket;
 import com.android.server.SystemService;
 import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent;
@@ -44,7 +45,11 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.function.ToIntFunction;
 
-/** {@hide} */
+/**
+ * Event buffering service for core networking and connectivity metrics.
+ *
+ * {@hide}
+ */
 final public class IpConnectivityMetrics extends SystemService {
     private static final String TAG = IpConnectivityMetrics.class.getSimpleName();
     private static final boolean DBG = false;
@@ -58,7 +63,10 @@ final public class IpConnectivityMetrics extends SystemService {
 
     private static final String SERVICE_NAME = IpConnectivityLog.SERVICE_NAME;
 
-    // Default size of the event buffer. Once the buffer is full, incoming events are dropped.
+    // Default size of the event rolling log for bug report dumps.
+    private static final int DEFAULT_LOG_SIZE = 500;
+    // Default size of the event buffer for metrics reporting.
+    // Once the buffer is full, incoming events are dropped.
     private static final int DEFAULT_BUFFER_SIZE = 2000;
     // Maximum size of the event buffer.
     private static final int MAXIMUM_BUFFER_SIZE = DEFAULT_BUFFER_SIZE * 10;
@@ -67,24 +75,38 @@ final public class IpConnectivityMetrics extends SystemService {
 
     private static final int ERROR_RATE_LIMITED = -1;
 
-    // Lock ensuring that concurrent manipulations of the event buffer are correct.
+    // Lock ensuring that concurrent manipulations of the event buffers are correct.
     // There are three concurrent operations to synchronize:
     //  - appending events to the buffer.
     //  - iterating throught the buffer.
     //  - flushing the buffer content and replacing it by a new buffer.
     private final Object mLock = new Object();
 
+    // Implementation instance of IIpConnectivityMetrics.aidl.
     @VisibleForTesting
     public final Impl impl = new Impl();
+    // Subservice listening to Netd events via INetdEventListener.aidl.
     @VisibleForTesting
     NetdEventListenerService mNetdListener;
 
+    // Rolling log of the most recent events. This log is used for dumping
+    // connectivity events in bug reports.
+    @GuardedBy("mLock")
+    private final RingBuffer<ConnectivityMetricsEvent> mEventLog =
+            new RingBuffer(ConnectivityMetricsEvent.class, DEFAULT_LOG_SIZE);
+    // Buffer of connectivity events used for metrics reporting. This buffer
+    // does not rotate automatically and instead saturates when it becomes full.
+    // It is flushed at metrics reporting.
     @GuardedBy("mLock")
     private ArrayList<ConnectivityMetricsEvent> mBuffer;
+    // Total number of events dropped from mBuffer since last metrics reporting.
     @GuardedBy("mLock")
     private int mDropped;
+    // Capacity of mBuffer
     @GuardedBy("mLock")
     private int mCapacity;
+    // A list of rate limiting counters keyed by connectivity event types for
+    // metrics reporting mBuffer.
     @GuardedBy("mLock")
     private final ArrayMap<Class<?>, TokenBucket> mBuckets = makeRateLimitingBuckets();
 
@@ -132,6 +154,7 @@ final public class IpConnectivityMetrics extends SystemService {
     private int append(ConnectivityMetricsEvent event) {
         if (DBG) Log.d(TAG, "logEvent: " + event);
         synchronized (mLock) {
+            mEventLog.append(event);
             final int left = mCapacity - mBuffer.size();
             if (event == null) {
                 return left;
@@ -216,6 +239,23 @@ final public class IpConnectivityMetrics extends SystemService {
         }
     }
 
+    /**
+     * Prints for bug reports the content of the rolling event log and the
+     * content of Netd event listener.
+     */
+    private void cmdDumpsys(FileDescriptor fd, PrintWriter pw, String[] args) {
+        final ConnectivityMetricsEvent[] events;
+        synchronized (mLock) {
+            events = mEventLog.toArray();
+        }
+        for (ConnectivityMetricsEvent ev : events) {
+            pw.println(ev.toString());
+        }
+        if (mNetdListener != null) {
+            mNetdListener.list(pw);
+        }
+    }
+
     private void cmdStats(FileDescriptor fd, PrintWriter pw, String[] args) {
         synchronized (mLock) {
             pw.println("Buffered events: " + mBuffer.size());
@@ -258,7 +298,8 @@ final public class IpConnectivityMetrics extends SystemService {
                     cmdFlush(fd, pw, args);
                     return;
                 case CMD_DUMPSYS:
-                    // Fallthrough to CMD_LIST when dumpsys.cpp dumps services states (bug reports)
+                    cmdDumpsys(fd, pw, args);
+                    return;
                 case CMD_LIST:
                     cmdList(fd, pw, args);
                     return;
diff --git a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java
index 25dba3570e20..6206dfcd6622 100644
--- a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java
+++ b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java
@@ -38,6 +38,7 @@ import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.BitUtils;
 import com.android.internal.util.IndentingPrintWriter;
+import com.android.internal.util.RingBuffer;
 import com.android.internal.util.TokenBucket;
 import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent;
 import java.io.PrintWriter;
@@ -82,9 +83,8 @@ public class NetdEventListenerService extends INetdEventListener.Stub {
     private final ArrayMap<String, WakeupStats> mWakeupStats = new ArrayMap<>();
     // Ring buffer array for storing packet wake up events sent by Netd.
     @GuardedBy("this")
-    private final WakeupEvent[] mWakeupEvents = new WakeupEvent[WAKEUP_EVENT_BUFFER_LENGTH];
-    @GuardedBy("this")
-    private long mWakeupEventCursor = 0;
+    private final RingBuffer<WakeupEvent> mWakeupEvents =
+            new RingBuffer(WakeupEvent.class, WAKEUP_EVENT_BUFFER_LENGTH);
 
     private final ConnectivityManager mCm;
 
@@ -175,13 +175,11 @@ public class NetdEventListenerService extends INetdEventListener.Stub {
 
     @GuardedBy("this")
     private void addWakeupEvent(String iface, long timestampMs, int uid) {
-        int index = wakeupEventIndex(mWakeupEventCursor);
-        mWakeupEventCursor++;
         WakeupEvent event = new WakeupEvent();
         event.iface = iface;
         event.timestampMs = timestampMs;
         event.uid = uid;
-        mWakeupEvents[index] = event;
+        mWakeupEvents.append(event);
         WakeupStats stats = mWakeupStats.get(iface);
         if (stats == null) {
             stats = new WakeupStats(iface);
@@ -190,23 +188,6 @@ public class NetdEventListenerService extends INetdEventListener.Stub {
         stats.countEvent(event);
     }
 
-    @GuardedBy("this")
-    private WakeupEvent[] getWakeupEvents() {
-        int length = (int) Math.min(mWakeupEventCursor, (long) mWakeupEvents.length);
-        WakeupEvent[] out = new WakeupEvent[length];
-        // Reverse iteration from youngest event to oldest event.
-        long inCursor = mWakeupEventCursor - 1;
-        int outIdx = out.length - 1;
-        while (outIdx >= 0) {
-            out[outIdx--] = mWakeupEvents[wakeupEventIndex(inCursor--)];
-        }
-        return out;
-    }
-
-    private static int wakeupEventIndex(long cursor) {
-        return (int) Math.abs(cursor % WAKEUP_EVENT_BUFFER_LENGTH);
-    }
-
     public synchronized void flushStatistics(List<IpConnectivityEvent> events) {
         flushProtos(events, mConnectEvents, IpConnectivityEventBuilder::toProto);
         flushProtos(events, mDnsEvents, IpConnectivityEventBuilder::toProto);
@@ -230,7 +211,7 @@ public class NetdEventListenerService extends INetdEventListener.Stub {
         for (int i = 0; i < mWakeupStats.size(); i++) {
             pw.println(mWakeupStats.valueAt(i));
         }
-        for (WakeupEvent wakeup : getWakeupEvents()) {
+        for (WakeupEvent wakeup : mWakeupEvents.toArray()) {
             pw.println(wakeup);
         }
     }
diff --git a/tests/net/java/com/android/internal/util/RingBufferTest.java b/tests/net/java/com/android/internal/util/RingBufferTest.java
new file mode 100644
index 000000000000..7a2344317223
--- /dev/null
+++ b/tests/net/java/com/android/internal/util/RingBufferTest.java
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2017 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.internal.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import java.util.Arrays;
+import java.util.Objects;
+
+
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class RingBufferTest {
+
+    @Test
+    public void testEmptyRingBuffer() {
+        RingBuffer<String> buffer = new RingBuffer<>(String.class, 100);
+
+        assertArraysEqual(new String[0], buffer.toArray());
+    }
+
+    @Test
+    public void testIncorrectConstructorArguments() {
+        try {
+            RingBuffer<String> buffer = new RingBuffer<>(String.class, -10);
+            fail("Should not be able to create a negative capacity RingBuffer");
+        } catch (IllegalArgumentException expected) {
+        }
+
+        try {
+            RingBuffer<String> buffer = new RingBuffer<>(String.class, 0);
+            fail("Should not be able to create a 0 capacity RingBuffer");
+        } catch (IllegalArgumentException expected) {
+        }
+    }
+
+    @Test
+    public void testRingBufferWithNoWrapping() {
+        RingBuffer<String> buffer = new RingBuffer<>(String.class, 100);
+
+        buffer.append("a");
+        buffer.append("b");
+        buffer.append("c");
+        buffer.append("d");
+        buffer.append("e");
+
+        String[] expected = {"a", "b", "c", "d", "e"};
+        assertArraysEqual(expected, buffer.toArray());
+    }
+
+    @Test
+    public void testRingBufferWithCapacity1() {
+        RingBuffer<String> buffer = new RingBuffer<>(String.class, 1);
+
+        buffer.append("a");
+        assertArraysEqual(new String[]{"a"}, buffer.toArray());
+
+        buffer.append("b");
+        assertArraysEqual(new String[]{"b"}, buffer.toArray());
+
+        buffer.append("c");
+        assertArraysEqual(new String[]{"c"}, buffer.toArray());
+
+        buffer.append("d");
+        assertArraysEqual(new String[]{"d"}, buffer.toArray());
+
+        buffer.append("e");
+        assertArraysEqual(new String[]{"e"}, buffer.toArray());
+    }
+
+    @Test
+    public void testRingBufferWithWrapping() {
+        int capacity = 100;
+        RingBuffer<String> buffer = new RingBuffer<>(String.class, capacity);
+
+        buffer.append("a");
+        buffer.append("b");
+        buffer.append("c");
+        buffer.append("d");
+        buffer.append("e");
+
+        String[] expected1 = {"a", "b", "c", "d", "e"};
+        assertArraysEqual(expected1, buffer.toArray());
+
+        String[] expected2 = new String[capacity];
+        int firstIndex = 0;
+        int lastIndex = capacity - 1;
+
+        expected2[firstIndex] = "e";
+        for (int i = 1; i < capacity; i++) {
+            buffer.append("x");
+            expected2[i] = "x";
+        }
+        assertArraysEqual(expected2, buffer.toArray());
+
+        buffer.append("x");
+        expected2[firstIndex] = "x";
+        assertArraysEqual(expected2, buffer.toArray());
+
+        for (int i = 0; i < 10; i++) {
+            for (String s : expected2) {
+                buffer.append(s);
+            }
+        }
+        assertArraysEqual(expected2, buffer.toArray());
+
+        buffer.append("a");
+        expected2[lastIndex] = "a";
+        assertArraysEqual(expected2, buffer.toArray());
+    }
+
+    static <T> void assertArraysEqual(T[] expected, T[] got) {
+        if (expected.length != got.length) {
+            fail(Arrays.toString(expected) + " and " + Arrays.toString(got)
+                    + " did not have the same length");
+        }
+
+        for (int i = 0; i < expected.length; i++) {
+            if (!Objects.equals(expected[i], got[i])) {
+                fail(Arrays.toString(expected) + " and " + Arrays.toString(got)
+                        + " were not equal");
+            }
+        }
+    }
+}