Skip to content

Commit

Permalink
RUM-6286: replacing joinToString when it possible, refactor a bit
Browse files Browse the repository at this point in the history
  • Loading branch information
satween committed Dec 16, 2024
1 parent e4c4bcc commit ece50e3
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 29 deletions.
2 changes: 2 additions & 0 deletions dd-sdk-android-core/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ class com.datadog.android.core.SdkReference
fun get(): com.datadog.android.api.SdkCore?
fun <T> allowThreadDiskReads(() -> T): T
fun <T> allowThreadDiskWrites(() -> T): T
fun StringBuilder.appendIfNotEmpty(String)
fun StringBuilder.appendIfNotEmpty(Char)
enum com.datadog.android.core.configuration.BackPressureMitigation
- DROP_OLDEST
- IGNORE_NEWEST
Expand Down
5 changes: 5 additions & 0 deletions dd-sdk-android-core/api/dd-sdk-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,11 @@ public final class com/datadog/android/core/StrictModeExtKt {
public static final fun allowThreadDiskWrites (Lkotlin/jvm/functions/Function0;)Ljava/lang/Object;
}

public final class com/datadog/android/core/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/core/configuration/BackPressureMitigation : java/lang/Enum {
public static final field DROP_OLDEST Lcom/datadog/android/core/configuration/BackPressureMitigation;
public static final field IGNORE_NEWEST Lcom/datadog/android/core/configuration/BackPressureMitigation;
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.core

/**
* This utility function helps replace joinToString calls with more efficient StringBuilder's methods calls.
* In case when content should be separated with some separator it's handy to add it in front of new string only
* if buffer already contain some data
* @param str - string that should be added into buffer only if it already contains some data
*/
fun StringBuilder.appendIfNotEmpty(str: String) = apply {
if (isNotEmpty()) append(str)
}

/**
* This utility function helps replace joinToString calls with more efficient StringBuilder's methods calls.
* In case when content should be separated with some separator it's handy to add it in front of new string only
* if buffer already contain some data
* @param char - char that should be added into 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
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,61 @@
package com.datadog.android.core

import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

class StringBuilderExtKtTest {

@Test
fun `M add char W addIfNotEmpty {buffer is not empty}`() {
// Given
val initialData = "something inside buffer already"
val buffer = StringBuilder(initialData)

// When
buffer.appendIfNotEmpty(' ')

// Then

assertThat(buffer.toString()).isEqualTo("$initialData ")
}

@Test
fun `M add str W addIfNotEmpty {buffer is not empty}`() {
// Given
val initialData = "something inside buffer already"
val buffer = StringBuilder(initialData)

// When
buffer.appendIfNotEmpty(" ")

// Then

assertThat(buffer.toString()).isEqualTo("$initialData ")
}

@Test
fun `M not add any char W addIfNotEmpty {buffer is empty}`() {
// Given
val buffer = StringBuilder()

// When
buffer.appendIfNotEmpty(' ')

// Then

assertThat(buffer.toString()).isEqualTo("")
}

@Test
fun `M not add any str W addIfNotEmpty {buffer is empty}`() {
// Given
val buffer = StringBuilder()

// When
buffer.appendIfNotEmpty(" ")

// Then

assertThat(buffer.toString()).isEqualTo("")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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.utils.forge.Configurator
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
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.system.measureNanoTime

private const val ITEMS_TO_JOINT = 10_000
private const val REPETITION_COUNT = 10_000

@Extensions(
ExtendWith(ForgeExtension::class)
)
@ForgeConfiguration(Configurator::class)
internal class PerformanceTest {

private fun List<Long>.mean() = sum().toDouble() / size

@Test
fun `M be faster than joinToString W buildString`(forge: Forge) {
val itemsForJoin = forge.aList(ITEMS_TO_JOINT) { forge.aString() }
val jointToStringResults = mutableListOf<Long>()
val builderResults = mutableListOf<Long>()

var jointToStringResult = ""
var builderResult = ""

for (i in 0..REPETITION_COUNT) {
jointToStringResults.add(
measureNanoTime {
val jointToStringContainer = mutableListOf<String>()
for (item in itemsForJoin) {
jointToStringContainer.add(item)
}
jointToStringResult = jointToStringContainer.joinToString(separator = " ") { it }
}
)

builderResults.add(
measureNanoTime {
builderResult = buildString {
itemsForJoin.forEachIndexed { i, item ->
if (i > 0) append(" ")
append(item)
}
}
}
)
}

assertThat(builderResult).isEqualTo(jointToStringResult) // same result
assertThat(builderResults.mean()).isLessThan(jointToStringResults.mean())
}
}
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 @@ -7,6 +7,7 @@
package com.datadog.android.rum.internal.anr

import com.datadog.android.api.InternalLogger
import com.datadog.android.core.appendIfNotEmpty
import com.datadog.android.core.feature.event.ThreadDump
import java.io.IOException
import java.io.InputStream
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 ece50e3

Please sign in to comment.