perf: use ring buffer for storing breadcrumbs

CHANGELOG.md

@@ -32,6 +32,9 @@
 * Enable ANR handling on immediately if started from the main thread
   [#1283](https://github.com/bugsnag/bugsnag-android/pull/1283)
 
+* Use ring buffer to store breadcrumbs
+  [#1286](https://github.com/bugsnag/bugsnag-android/pull/1286)
+
 ## 5.9.4 (2021-05-26)
 
 * Unity: add methods for setting autoNotify and autoDetectAnrs

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BreadcrumbFilterTest.kt

@@ -24,8 +24,7 @@ class BreadcrumbFilterTest {
         client = generateClient(configuration)
 
         client.leaveBreadcrumb("Hello World")
-
-        assertEquals(1, client.breadcrumbState.store.size)
+        assertEquals(1, client.breadcrumbState.copy().size)
     }
 
     @Test
@@ -36,6 +35,6 @@ class BreadcrumbFilterTest {
 
         client.leaveBreadcrumb("Hello World")
 
-        assertEquals(1, client.breadcrumbState.store.size)
+        assertEquals(1, client.breadcrumbState.copy().size)
     }
 }

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BreadcrumbStateTest.kt

@@ -31,7 +31,7 @@ class BreadcrumbStateTest {
     fun testClientMethods() {
         client = generateClient()
         client!!.leaveBreadcrumb("Hello World")
-        val store = client!!.breadcrumbState.store
+        val store = client!!.breadcrumbState.copy()
         var count = 0
 
         for (breadcrumb in store) {

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ClientTest.java

@@ -76,14 +76,14 @@ public void testMaxBreadcrumbs() {
         config.setEnabledBreadcrumbTypes(new HashSet<>(breadcrumbTypes));
         config.setMaxBreadcrumbs(2);
         client = generateClient(config);
-        assertEquals(0, client.breadcrumbState.getStore().size());
+        assertEquals(0, client.breadcrumbState.copy().size());
 
         client.leaveBreadcrumb("test");
         client.leaveBreadcrumb("another");
         client.leaveBreadcrumb("yet another");
-        assertEquals(2, client.breadcrumbState.getStore().size());
+        assertEquals(2, client.breadcrumbState.copy().size());
 
-        Breadcrumb poll = client.breadcrumbState.getStore().poll();
+        Breadcrumb poll = client.breadcrumbState.copy().get(0);
         assertEquals(BreadcrumbType.MANUAL, poll.getType());
         assertEquals("another", poll.getMessage());
     }
@@ -129,7 +129,7 @@ public void testClientBreadcrumbRetrieval() {
         client = generateClient();
         client.leaveBreadcrumb("Hello World");
         List<Breadcrumb> breadcrumbs = client.getBreadcrumbs();
-        List<Breadcrumb> store = new ArrayList<>(client.breadcrumbState.getStore());
+        List<Breadcrumb> store = new ArrayList<>(client.breadcrumbState.copy());
         assertEquals(store, breadcrumbs);
         assertNotSame(store, breadcrumbs);
     }
@@ -153,8 +153,8 @@ public void testBreadcrumbStoreNotModified() {
 
         breadcrumbs.clear(); // only the copy should be cleared
         assertTrue(breadcrumbs.isEmpty());
-        assertEquals(1, client.breadcrumbState.getStore().size());
-        assertEquals("Manual breadcrumb", client.breadcrumbState.getStore().remove().getMessage());
+        assertEquals(1, client.breadcrumbState.copy().size());
+        assertEquals("Manual breadcrumb", client.breadcrumbState.copy().get(0).getMessage());
     }
 
     @Test

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/OnBreadcrumbCallbackStateTest.java

@@ -34,7 +34,7 @@ public void setUp() {
         breadcrumbTypes.add(BreadcrumbType.USER);
         configuration.setEnabledBreadcrumbTypes(breadcrumbTypes);
         client = generateClient(configuration);
-        assertEquals(0, client.breadcrumbState.getStore().size());
+        assertEquals(0, client.breadcrumbState.copy().size());
     }
 
     @After
@@ -45,7 +45,7 @@ public void tearDown() {
     @Test
     public void noCallback() {
         client.leaveBreadcrumb("Hello");
-        assertEquals(1, client.breadcrumbState.getStore().size());
+        assertEquals(1, client.breadcrumbState.copy().size());
     }
 
     @Test
@@ -57,7 +57,7 @@ public boolean onBreadcrumb(@NonNull Breadcrumb breadcrumb) {
             }
         });
         client.leaveBreadcrumb("Hello");
-        assertEquals(0, client.breadcrumbState.getStore().size());
+        assertEquals(0, client.breadcrumbState.copy().size());
     }
 
     @Test
@@ -69,7 +69,7 @@ public boolean onBreadcrumb(@NonNull Breadcrumb breadcrumb) {
             }
         });
         client.leaveBreadcrumb("Hello");
-        assertEquals(1, client.breadcrumbState.getStore().size());
+        assertEquals(1, client.breadcrumbState.copy().size());
     }
 
     @Test
@@ -87,7 +87,7 @@ public boolean onBreadcrumb(@NonNull Breadcrumb breadcrumb) {
             }
         });
         client.leaveBreadcrumb("Hello");
-        assertEquals(0, client.breadcrumbState.getStore().size());
+        assertEquals(0, client.breadcrumbState.copy().size());
     }
 
     @Test
@@ -161,7 +161,7 @@ public boolean onBreadcrumb(@NonNull Breadcrumb breadcrumb) {
         client.leaveBreadcrumb("Hello");
         client.removeOnBreadcrumb(cb);
         client.leaveBreadcrumb("Hello");
-        assertEquals(1, client.breadcrumbState.getStore().size());
+        assertEquals(1, client.breadcrumbState.copy().size());
     }
 
 }

bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbState.kt

@@ -1,41 +1,41 @@
 package com.bugsnag.android
 
 import java.io.IOException
-import java.util.Queue
-import java.util.concurrent.ConcurrentLinkedQueue
+import java.util.concurrent.atomic.AtomicInteger
 
+/**
+ * Stores breadcrumbs added to the [Client] in a ring buffer. If the number of breadcrumbs exceeds
+ * the maximum configured limit then the oldest breadcrumb in the ring buffer will be overwritten.
+ *
+ * When the breadcrumbs are required for generation of an event a [List] is constructed and
+ * breadcrumbs added in the order of their addition.
+ */
 internal class BreadcrumbState(
-    maxBreadcrumbs: Int,
-    val callbackState: CallbackState,
-    val logger: Logger
+    private val maxBreadcrumbs: Int,
+    private val callbackState: CallbackState,
+    private val logger: Logger
 ) : BaseObservable(), JsonStream.Streamable {
 
-    val store: Queue<Breadcrumb> = ConcurrentLinkedQueue()
+    /*
+     * We use the `index` as both a pointer to the tail of our ring-buffer, and also as "cheat"
+     * semaphore. When the ring-buffer is being copied - the index is set to a negative number,
+     * which is an invalid array-index. By masking the `expected` value in a `compareAndSet` with
+     * `validIndexMask`: the CAS operation will only succeed if it wouldn't interrupt a concurrent
+     * `copy()` call.
+     */
+    private val validIndexMask: Int = Int.MAX_VALUE
 
-    private val maxBreadcrumbs: Int
-
-    init {
-        when {
-            maxBreadcrumbs > 0 -> this.maxBreadcrumbs = maxBreadcrumbs
-            else -> this.maxBreadcrumbs = 0
-        }
-    }
-
-    @Throws(IOException::class)
-    override fun toStream(writer: JsonStream) {
-        pruneBreadcrumbs()
-        writer.beginArray()
-        store.forEach { it.toStream(writer) }
-        writer.endArray()
-    }
+    private val store = arrayOfNulls<Breadcrumb?>(maxBreadcrumbs)
+    private val index = AtomicInteger(0)
 
     fun add(breadcrumb: Breadcrumb) {
-        if (!callbackState.runOnBreadcrumbTasks(breadcrumb, logger)) {
+        if (maxBreadcrumbs == 0 || !callbackState.runOnBreadcrumbTasks(breadcrumb, logger)) {
             return
         }
 
-        store.add(breadcrumb)
-        pruneBreadcrumbs()
+        // store the breadcrumb in the ring buffer
+        val position = getBreadcrumbIndex()
+        store[position] = breadcrumb
 
         updateState {
             // use direct field access to avoid overhead of accessor method
@@ -48,10 +48,49 @@ internal class BreadcrumbState(
         }
     }
 
-    private fun pruneBreadcrumbs() {
-        // Remove oldest breadcrumbState until new max size reached
-        while (store.size > maxBreadcrumbs) {
-            store.poll()
+    /**
+     * Retrieves the index in the ring buffer where the breadcrumb should be stored.
+     */
+    private fun getBreadcrumbIndex(): Int {
+        while (true) {
+            val currentValue = index.get() and validIndexMask
+            val nextValue = (currentValue + 1) % maxBreadcrumbs
+            if (index.compareAndSet(currentValue, nextValue)) {
+                return currentValue
+            }
+        }
+    }
+
+    /**
+     * Creates a copy of the breadcrumbs in the order of their addition.
+     */
+    fun copy(): List<Breadcrumb> {
+        if (maxBreadcrumbs == 0) {
+            return emptyList()
+        }
+
+        // Set a negative value that stops any other thread from adding a breadcrumb.
+        // This handles reentrancy by waiting here until the old value has been reset.
+        var tail = -1
+        while (tail == -1) {
+            tail = index.getAndSet(-1)
         }
+
+        try {
+            val result = arrayOfNulls<Breadcrumb>(maxBreadcrumbs)
+            store.copyInto(result, 0, tail, maxBreadcrumbs)
+            store.copyInto(result, maxBreadcrumbs - tail, 0, tail)
+            return result.filterNotNull()
+        } finally {
+            index.set(tail)
+        }
+    }
+
+    @Throws(IOException::class)
+    override fun toStream(writer: JsonStream) {
+        val crumbs = copy()
+        writer.beginArray()
+        crumbs.forEach { it.toStream(writer) }
+        writer.endArray()
     }
 }

bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java

@@ -711,7 +711,7 @@ void populateAndNotifyAndroidEvent(@NonNull Event event,
         event.addMetadata("app", appDataCollector.getAppDataMetadata());
 
         // Attach breadcrumbState to the event
-        event.setBreadcrumbs(new ArrayList<>(breadcrumbState.getStore()));
+        event.setBreadcrumbs(breadcrumbState.copy());
 
         // Attach user info to the event
         User user = userState.getUser();
@@ -765,7 +765,7 @@ void notifyInternal(@NonNull Event event,
      */
     @NonNull
     public List<Breadcrumb> getBreadcrumbs() {
-        return new ArrayList<>(breadcrumbState.getStore());
+        return breadcrumbState.copy();
     }
 
     @NonNull