RUM-6286: replacing joinToString when it possible, refactor a bit

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/data/upload/CurlInterceptor.kt

@@ -67,25 +67,21 @@ internal class CurlInterceptor(
                 printBody = printBody
             )
 
-        fun toCommand(): String {
-            val parts = mutableListOf<String>()
-            parts.add("curl")
-            parts.add(FORMAT_METHOD.format(Locale.US, method.uppercase(Locale.US)))
-
+        fun toCommand(): String = buildString {
+            append("curl").append(' ')
+            append(FORMAT_METHOD.format(Locale.US, method.uppercase(Locale.US))).append(' ')
             headers.forEach { (key, values) ->
                 values.forEach { value ->
-                    parts.add(FORMAT_HEADER.format(Locale.US, key, value))
+                    append(FORMAT_HEADER.format(Locale.US, key, value)).append(' ')
                 }
             }
 
             if (contentType != null && !headers.containsKey(CONTENT_TYPE)) {
-                parts.add(FORMAT_HEADER.format(Locale.US, CONTENT_TYPE, contentType))
+                append(FORMAT_HEADER.format(Locale.US, CONTENT_TYPE, contentType)).append(' ')
             }
 
-            requestBody?.let { parts.addAll(it.toParts()) }
-            parts.add(FORMAT_URL.format(Locale.US, url))
-
-            return parts.joinToString(" ")
+            requestBody?.toParts()?.forEach { append(it).append(' ') }
+            append(FORMAT_URL.format(Locale.US, url))
         }
 
         private fun RequestBody.toParts(): List<String> {

dd-sdk-android-core/src/main/kotlin/com/datadog/android/error/internal/DatadogExceptionHandler.kt

@@ -133,12 +133,11 @@ internal class DatadogExceptionHandler(
         ) + safeGetAllStacktraces()
             .filterKeys { it != crashedThread }
             .filterValues { it.isNotEmpty() }
-            .map {
-                val thread = it.key
+            .map { (thread, stackTrace) ->
                 ThreadDump(
                     name = thread.name,
                     state = thread.state.asString(),
-                    stack = it.value.loggableStackTrace(),
+                    stack = stackTrace.loggableStackTrace(),
                     crashed = false
                 )
             }

dd-sdk-android-core/src/test/kotlin/com/datadog/android/utils/JointToStringVsStringBuilderPerformanceTest.kt

@@ -0,0 +1,108 @@
+/*
+ * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
+ * This product includes software developed at Datadog (https://www.datadoghq.com/).
+ * Copyright 2016-Present Datadog, Inc.
+ */
+package com.datadog.android.utils
+
+import com.datadog.android.internal.utils.appendIfNotEmpty
+import fr.xgouchet.elmyr.Forge
+import fr.xgouchet.elmyr.junit5.ForgeExtension
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.junit.jupiter.api.extension.Extensions
+import kotlin.math.pow
+import kotlin.math.round
+import kotlin.math.sqrt
+import kotlin.system.measureNanoTime
+
+@Extensions(
+    ExtendWith(ForgeExtension::class)
+)
+internal class JointToStringVsStringBuilderPerformanceTest {
+
+    @Test
+    fun `M be faster than joinToString W buildString`(forge: Forge) {
+        val itemsForJoin = forge.aList(ITEMS_TO_JOIN) { forge.aString() }
+        val joinToStringExecutionTime = mutableListOf<Long>()
+        val buildStringExecutionTime = mutableListOf<Long>()
+
+        var jointToStringResult: String
+        var builderResult: String
+
+        repeat(REPETITION_COUNT) {
+            joinToStringExecutionTime.add(
+                measureNanoTime {
+                    val jointToStringContainer = mutableListOf<String>()
+                    for (item in itemsForJoin) {
+                        jointToStringContainer.add(item)
+                    }
+                    jointToStringResult = jointToStringContainer.joinToString(separator = " ") { it }
+                }
+            )
+
+            buildStringExecutionTime.add(
+                measureNanoTime {
+                    builderResult = buildString {
+                        itemsForJoin.forEach { item -> appendIfNotEmpty(' ').append(item) }
+                    }
+                }
+            )
+
+            assertThat(builderResult).isEqualTo(jointToStringResult) // same result
+        }
+
+        val statisticsReport = (
+            "buildString:\n" +
+                " mean = ${buildStringExecutionTime.mean}\n" +
+                " std = ${buildStringExecutionTime.std}\n" +
+                " cv = ${"%.2f".format(buildStringExecutionTime.cv)}%\n" +
+                " p50 = ${buildStringExecutionTime.percentile(50)}\n" +
+                " p90 = ${buildStringExecutionTime.percentile(90)}\n" +
+                " p95 = ${buildStringExecutionTime.percentile(95)}\n" +
+                " p99 = ${buildStringExecutionTime.percentile(99)}\n" +
+                "\n" +
+                "joinToString:\n" +
+                " mean = ${joinToStringExecutionTime.mean}\n" +
+                " std = ${joinToStringExecutionTime.std}\n" +
+                " cv = ${"%.2f".format(joinToStringExecutionTime.cv)}%\n" +
+                " p50 = ${joinToStringExecutionTime.percentile(50)},\n" +
+                " p90 = ${joinToStringExecutionTime.percentile(90)},\n" +
+                " p95 = ${joinToStringExecutionTime.percentile(95)},\n" +
+                " p99 = ${joinToStringExecutionTime.percentile(99)}\n"
+            )
+
+        assertThat(
+            buildStringExecutionTime.percentile(90)
+        ).withFailMessage(
+            statisticsReport
+        ).isLessThan(
+            joinToStringExecutionTime.percentile(90)
+        )
+    }
+
+    companion object {
+        private const val ITEMS_TO_JOIN = 10_000
+        private const val REPETITION_COUNT = 10_000
+
+        private val List<Long>.mean
+            get() = (sum().toDouble() / size)
+
+        private val List<Long>.std: Double
+            get() {
+                val m = mean
+                return sqrt(
+                    sumOf { (it - m).pow(2.0) } / size
+                )
+            }
+
+        private val List<Long>.cv: Double
+            get() = std / mean * 100.0
+
+        private fun List<Long>.percentile(k: Int): Long {
+            val p = (k / 100.0) * (size + 1)
+            return sorted()[round(p).toInt()]
+        }
+    }
+}

dd-sdk-android-internal/api/apiSurface

@@ -38,6 +38,8 @@ object com.datadog.android.internal.utils.ImageViewUtils
   fun resolveContentRectWithScaling(android.widget.ImageView, android.graphics.drawable.Drawable, android.widget.ImageView.ScaleType? = null): android.graphics.Rect
 fun Int.densityNormalized(Float): Int
 fun Long.densityNormalized(Float): Long
+fun StringBuilder.appendIfNotEmpty(String)
+fun StringBuilder.appendIfNotEmpty(Char)
 fun Throwable.loggableStackTrace(): String
 annotation com.datadog.tools.annotation.NoOpImplementation
   constructor(Boolean = false)

dd-sdk-android-internal/api/dd-sdk-android-internal.api

@@ -130,6 +130,11 @@ public final class com/datadog/android/internal/utils/LongExtKt {
 	public static final fun densityNormalized (JF)J
 }
 
+public final class com/datadog/android/internal/utils/StringBuilderExtKt {
+	public static final fun appendIfNotEmpty (Ljava/lang/StringBuilder;C)Ljava/lang/StringBuilder;
+	public static final fun appendIfNotEmpty (Ljava/lang/StringBuilder;Ljava/lang/String;)Ljava/lang/StringBuilder;
+}
+
 public final class com/datadog/android/internal/utils/ThrowableExtKt {
 	public static final fun loggableStackTrace (Ljava/lang/Throwable;)Ljava/lang/String;
 }

dd-sdk-android-internal/src/main/java/com/datadog/android/internal/utils/StringBuilderExt.kt

@@ -0,0 +1,27 @@
+/*
+ * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
+ * This product includes software developed at Datadog (https://www.datadoghq.com/).
+ * Copyright 2016-Present Datadog, Inc.
+ */
+
+package com.datadog.android.internal.utils
+
+/**
+ * This utility function helps to replace [joinToString] calls with more efficient [StringBuilder] methods calls.
+ * In case when content should be separated with some separator, it's handy to add it in front of a new string only
+ * if buffer already contains some data.
+ * @param str string that should be added to the buffer only if it already contains some data.
+ */
+fun StringBuilder.appendIfNotEmpty(str: String) = apply {
+    if (isNotEmpty()) append(str)
+}
+
+/**
+ * This utility function helps to replace [joinToString] calls with more efficient [StringBuilder] methods calls.
+ * In case when content should be separated with some separator, it's handy to add it in front of a new string only
+ * if buffer already contains some data.
+ * @param char char that should be added to the buffer only if it already contains some data.
+ */
+fun StringBuilder.appendIfNotEmpty(char: Char) = apply {
+    if (isNotEmpty()) append(char)
+}

dd-sdk-android-internal/src/test/java/com/datadog/internal/utils/StringBuilderExtKtTest.kt

@@ -0,0 +1,73 @@
+/*
+ * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
+ * This product includes software developed at Datadog (https://www.datadoghq.com/).
+ * Copyright 2016-Present Datadog, Inc.
+ */
+
+package com.datadog.internal.utils
+
+import com.datadog.android.internal.utils.appendIfNotEmpty
+import fr.xgouchet.elmyr.annotation.StringForgery
+import fr.xgouchet.elmyr.junit5.ForgeExtension
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.junit.jupiter.api.extension.Extensions
+
+@Extensions(
+    ExtendWith(ForgeExtension::class)
+)
+internal class StringBuilderExtKtTest {
+
+    @Test
+    fun `M add char W appendIfNotEmpty {buffer is not empty}`(
+        @StringForgery(regex = ".+") initialContent: String
+    ) {
+        // Given
+        val buffer = StringBuilder(initialContent)
+
+        // When
+        buffer.appendIfNotEmpty(' ')
+
+        // Then
+        assertThat(buffer.toString()).isEqualTo("$initialContent ")
+    }
+
+    @Test
+    fun `M add str W appendIfNotEmpty {buffer is not empty}`(
+        @StringForgery(regex = ".+") initialContent: String
+    ) {
+        // Given
+        val buffer = StringBuilder(initialContent)
+
+        // When
+        buffer.appendIfNotEmpty(" ")
+
+        // Then
+        assertThat(buffer.toString()).isEqualTo("$initialContent ")
+    }
+
+    @Test
+    fun `M not add any char W appendIfNotEmpty {buffer is empty}`() {
+        // Given
+        val buffer = StringBuilder()
+
+        // When
+        buffer.appendIfNotEmpty(' ')
+
+        // Then
+        assertThat(buffer.toString()).isEqualTo("")
+    }
+
+    @Test
+    fun `M not add any str W appendIfNotEmpty {buffer is empty}`() {
+        // Given
+        val buffer = StringBuilder()
+
+        // When
+        buffer.appendIfNotEmpty(" ")
+
+        // Then
+        assertThat(buffer.toString()).isEqualTo("")
+    }
+}

detekt_custom.yml

@@ -780,6 +780,7 @@ datadog:
       - "java.lang.StringBuilder.append(kotlin.Char)"
       - "java.lang.StringBuilder.append(kotlin.CharArray?)"
       - "java.lang.StringBuilder.append(kotlin.String?)"
+      - "java.lang.StringBuilder.clear()"
       - "java.lang.StringBuilder.constructor()"
       - "java.math.BigInteger.toHexString()"
       - "java.math.BigInteger.toLong()"

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/DatadogLateCrashReporter.kt

@@ -9,6 +9,7 @@ package com.datadog.android.rum.internal
 import android.app.ApplicationExitInfo
 import android.os.Build
 import androidx.annotation.RequiresApi
+import androidx.annotation.WorkerThread
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.context.DatadogContext
 import com.datadog.android.api.feature.Feature
@@ -97,6 +98,7 @@ internal class DatadogLateCrashReporter(
         }
     }
 
+    @WorkerThread
     @RequiresApi(Build.VERSION_CODES.R)
     override fun handleAnrCrash(
         anrExitInfo: ApplicationExitInfo,

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/LateCrashReporter.kt

@@ -9,13 +9,15 @@ package com.datadog.android.rum.internal
 import android.app.ApplicationExitInfo
 import android.os.Build
 import androidx.annotation.RequiresApi
+import androidx.annotation.WorkerThread
 import com.datadog.android.api.storage.DataWriter
 import com.google.gson.JsonObject
 
 internal interface LateCrashReporter {
 
     fun handleNdkCrashEvent(event: Map<*, *>, rumWriter: DataWriter<Any>)
 
+    @WorkerThread
     @RequiresApi(Build.VERSION_CODES.R)
     fun handleAnrCrash(
         anrExitInfo: ApplicationExitInfo,

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/anr/AndroidTraceParser.kt

@@ -8,6 +8,7 @@ package com.datadog.android.rum.internal.anr
 
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.core.feature.event.ThreadDump
+import com.datadog.android.internal.utils.appendIfNotEmpty
 import java.io.IOException
 import java.io.InputStream
 import java.util.Locale
@@ -33,7 +34,7 @@ internal class AndroidTraceParser(
         val threadDumps = mutableListOf<ThreadDump>()
 
         var isInThreadStackBlock = false
-        val currentThreadStack = mutableListOf<String>()
+        val currentThreadStack = StringBuilder()
         var currentThreadName: String? = null
         var currentThreadState: String? = null
 
@@ -45,7 +46,7 @@ internal class AndroidTraceParser(
                     threadDumps += ThreadDump(
                         name = currentThreadName,
                         state = convertThreadState(currentThreadState.orEmpty()),
-                        stack = currentThreadStack.joinToString("\n"),
+                        stack = currentThreadStack.toString(),
                         crashed = currentThreadName == "main"
                     )
                 }
@@ -73,7 +74,7 @@ internal class AndroidTraceParser(
                 // - locked <0x0dd89f49> (a okhttp3.internal.concurrent.TaskRunner)
                 // we want to skip them for now
                 // also we want to skip any non-stack lines in the thread info block
-                currentThreadStack += line
+                currentThreadStack.appendIfNotEmpty('\n').append(line)
             }
         }
 

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/net/RumRequestFactory.kt

@@ -108,24 +108,20 @@ internal class RumRequestFactory(
         env: String,
         variant: String,
         executionContext: RequestExecutionContext
-    ): String {
-        val elements = mutableListOf(
-            "${RumAttributes.SERVICE_NAME}:$serviceName",
-            "${RumAttributes.APPLICATION_VERSION}:$version",
-            "${RumAttributes.SDK_VERSION}:$sdkVersion",
-            "${RumAttributes.ENV}:$env"
-        )
+    ) = buildString {
+        append("${RumAttributes.SERVICE_NAME}:$serviceName").append(",")
+            .append("${RumAttributes.APPLICATION_VERSION}:$version").append(",")
+            .append("${RumAttributes.SDK_VERSION}:$sdkVersion").append(",")
+            .append("${RumAttributes.ENV}:$env")
 
         if (variant.isNotEmpty()) {
-            elements.add("${RumAttributes.VARIANT}:$variant")
+            append(",").append("${RumAttributes.VARIANT}:$variant")
         }
         if (executionContext.previousResponseCode != null) {
             // we had a previous failure
-            elements.add("${RETRY_COUNT_KEY}:${executionContext.attemptNumber}")
-            elements.add("${LAST_FAILURE_STATUS_KEY}:${executionContext.previousResponseCode}")
+            append(",").append("${RETRY_COUNT_KEY}:${executionContext.attemptNumber}")
+            append(",").append("${LAST_FAILURE_STATUS_KEY}:${executionContext.previousResponseCode}")
         }
-
-        return elements.joinToString(",")
     }
 
     @Suppress("TooGenericExceptionCaught")