Skip to content

Commit

Permalink
Merge pull request #2456 from DataDog/tvaleev/rum-6286/join-to-string…
Browse files Browse the repository at this point in the history
…-optimizations

RUM-6286: replacing `joinToString` when it possible, refactor a bit
  • Loading branch information
satween authored Dec 18, 2024
2 parents f9bc2b5 + b2a6eae commit df15ef6
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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()]
}
}
}
2 changes: 2 additions & 0 deletions dd-sdk-android-internal/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -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)
5 changes: 5 additions & 0 deletions dd-sdk-android-internal/api/dd-sdk-android-internal.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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("")
}
}
1 change: 1 addition & 0 deletions detekt_custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -97,6 +98,7 @@ internal class DatadogLateCrashReporter(
}
}

@WorkerThread
@RequiresApi(Build.VERSION_CODES.R)
override fun handleAnrCrash(
anrExitInfo: ApplicationExitInfo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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"
)
}
Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit df15ef6

Please sign in to comment.