feat(SpanTracker): lower-overhead SpanTracker.endAllSpans implementation

.buildkite/pipeline.yml

@@ -11,6 +11,10 @@ steps:
     agents:
       queue: macos-12-arm
     command: './gradlew test'
+    plugins:
+      artifacts#v1.9.0:
+        upload: "bugsnag-android-performance/build/reports/tests/"
+        compressed: bugsnag-android-performance-test-reports.tgz
 
   - label: ':android: Lint test scenarios'
     timeout_in_minutes: 10

bugsnag-android-performance/src/main/kotlin/com/bugsnag/android/performance/internal/SpanTracker.kt

@@ -43,6 +43,23 @@ class SpanTracker {
      * or discard the associated Span objects when the token is leaked. This avoids the Span
      * objects "leaking" within the SpanContext hierarchy, since we assume that once the token
      * is lost there is no remaining way for the user to end() the Span.
+     *
+     * The "bindings" table is structured similar to HashMap or similar classes, but using
+     * SpanBinding in place of a Map.Entry as we have a more complex key structure. For example
+     * a table might look something like:
+     *
+     * [ ] binding -> collision-binding -> collision-binding -> null
+     * [ ] null
+     * [ ] null
+     * [ ] binding -> null
+     * [ ] binding -> binding -> null
+     * [ ] null
+     * [ ] null
+     * [ ] null
+     *
+     * The linked-list represented by the SpanBinding.next property is *only* used to handle table
+     * space collisions (when multiple bindings need to occupy the same slot in the bindings table).
+     * They do *not* represent a full-table linked list (as LinkedHashMap would).
      */
 
     private var bindings: Array<SpanBinding?> = arrayOfNulls(DEFAULT_SPAN_TRACKER_TABLE_SIZE)
@@ -231,15 +248,37 @@ class SpanTracker {
         lock.write {
             sweepStaleEntriesUnderWriteLock()
 
-            // we do a sweep of the end binding table
+            // we do a sweep of the entire binding table
             // this is typically quite small as it isn't common for more than a few
             // root tokens to be active at a time, leading to the table mostly being
             // subTokens of the same roots
-            for (index in bindings.indices) {
-                val binding = bindings[index] ?: continue
-                val (newHead, removed) = binding.removeWhere { it.get() === token }
-                bindings[index] = newHead
-                removed?.markLeaked(endTime)
+            val table = bindings
+            // this loop walks "down" the bindings table
+            for (index in table.indices) {
+                var currentBinding: SpanBinding? = table[index] ?: continue
+
+                var newHead: SpanBinding? = null
+                var previous: SpanBinding? = null
+
+                // this loop walks "sideways" through the linked-list of bindings on this "row"
+                while (currentBinding != null) {
+                    if (currentBinding.get() === token) {
+                        // end the span at an appropriate time
+                        currentBinding.markLeaked(endTime)
+                        // unlink currentBinding from the list
+                        previous?.next = currentBinding.next
+                    } else if (newHead == null) {
+                        newHead = currentBinding
+                        previous = currentBinding
+                    } else {
+                        previous = currentBinding
+                    }
+
+                    // move to the next binding (if there is one)
+                    currentBinding = currentBinding.next
+                }
+
+                table[index] = newHead
             }
         }
     }
@@ -281,9 +320,7 @@ class SpanTracker {
     }
 
     private fun expandBindingsTable(): Array<SpanBinding?> {
-        // WARNING: do not put anything other than 2 here!
-        // The table size *must* remain power-of-two as it grows, otherwise indexForHash will break
-        val newBindingsTable = arrayOfNulls<SpanBinding>(bindings.size * 2)
+        val newBindingsTable = arrayOfNulls<SpanBinding>(expandedTableSize(bindings.size))
         val newTableSize = newBindingsTable.size
 
         for (index in bindings.indices) {
@@ -304,6 +341,10 @@ class SpanTracker {
         return newBindingsTable
     }
 
+    // WARNING: do not put anything other than 2 here!
+    // The table size *must* remain power-of-two as it grows, otherwise indexForHash will break
+    private fun expandedTableSize(currentTableSize: Int) = currentTableSize * 2
+
     private class SpanBinding(
         boundObject: Any,
         referenceQueue: ReferenceQueue<in Any>,
@@ -379,7 +420,7 @@ class SpanTracker {
         }
 
         /**
-         * Remove a SpanBinding from the linked-list where the given predicate matches that
+         * Remove a *single* SpanBinding from the linked-list where the given predicate matches that
          * SpanBinding, returning the new "head" of the linked-list and the removed SpanBinding
          * as a pair.
          */

bugsnag-android-performance/src/test/java/com/bugsnag/android/performance/internal/SpanTrackerTest.kt

@@ -148,7 +148,7 @@ class SpanTrackerTest {
         val tracker = SpanTracker()
         val trackedSpans = populateDenseSpanTracker(tokens, tracker)
 
-        val endedToken = tokens[3]
+        val endedToken = tokens[3] // it doesn't really matter which we choose
         tracker.endAllSpans(endedToken, 1234L)
 
         val rootSpan = trackedSpans.remove(endedToken to null)!!
@@ -185,18 +185,18 @@ class SpanTrackerTest {
     }
 
     private fun populateDenseSpanTracker(
-        tokens: Array<Any>,
+        tokens: Array<out Any>,
         tracker: SpanTracker,
     ): MutableMap<Pair<Any, ViewLoadPhase?>, SpanImpl> {
         val spans = HashMap<Pair<Any, ViewLoadPhase?>, SpanImpl>()
 
-        tokens.forEachIndexed { index, token ->
-            val rootTokenSpan = createSpan("Token$index")
+        tokens.forEach { token ->
+            val rootTokenSpan = createSpan(token.toString())
             assertSame(rootTokenSpan, tracker.associate(token, null, rootTokenSpan))
             spans[token to null] = rootTokenSpan
 
             ViewLoadPhase.values().forEach { phase ->
-                val span = createSpan("Token$index/$phase")
+                val span = createSpan("$token/$phase")
                 val boundSpan = tracker.associate(token, phase, span)
 
                 assertSame(span, boundSpan)