Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ fun Project.detektCustomConfig() {
args(
"--config",
"${rootDir.absolutePath}/detekt_custom_general.yml," +
"${rootDir.absolutePath}/detekt_expensive_methods.yml," +
"${rootDir.absolutePath}/detekt_custom_safe_calls.yml," +
"${rootDir.absolutePath}/detekt_custom_unsafe_calls.yml"
)
Expand Down
2 changes: 2 additions & 0 deletions dd-sdk-android-internal/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class com.datadog.android.internal.lifecycle.ProcessLifecycleMonitor : android.a
fun onResumed()
fun onStopped()
fun onPaused()
annotation com.datadog.android.internal.lint.HotMethod
constructor(String, Array<String>)
enum com.datadog.android.internal.network.GraphQLHeaders
constructor(String)
- DD_GRAPHQL_NAME_HEADER
Expand Down
6 changes: 5 additions & 1 deletion dd-sdk-android-internal/api/dd-sdk-android-internal.api
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ public abstract interface class com/datadog/android/internal/lifecycle/ProcessLi
public abstract fun onStopped ()V
}

public abstract interface annotation class com/datadog/android/internal/lint/HotMethod : java/lang/annotation/Annotation {
public abstract fun exclude ()[Ljava/lang/String;
public abstract fun message ()Ljava/lang/String;
}

public final class com/datadog/android/internal/network/GraphQLHeaders : java/lang/Enum {
public static final field DD_GRAPHQL_NAME_HEADER Lcom/datadog/android/internal/network/GraphQLHeaders;
public static final field DD_GRAPHQL_PAYLOAD_HEADER Lcom/datadog/android/internal/network/GraphQLHeaders;
Expand Down Expand Up @@ -644,4 +649,3 @@ public abstract interface annotation class com/datadog/tools/annotation/NoOpImpl
public abstract fun customName ()Ljava/lang/String;
public abstract fun publicNoOpImplementation ()Z
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.lint

/**
* Marks a method as a hot path — one that can be invoked on every frame or touch event.
*
* Implementations must avoid heap allocations, blocking operations, and any work
* that introduces GC pressure. The IDE/Lint will highlight annotated methods as a
* reminder to reviewers.
*
* @property message A short description of why this method is hot (e.g. "called per frame by JankStats").
* @property exclude Check names or specific function/constructor names to skip for this site only.
* Built-in category strings: `"constructor"`, `"anonymous-object"`, `"lambda"`,
* `"string-template"`, `"factory"`, `"collection-ops"`.
* You may also pass a bare method name (e.g. `"forEach"`) or a qualified name
* (e.g. `"kotlin.collections.List.forEach"`) to exclude just that call.
* Prefer a narrow per-name exclusion over a broad category exclusion.
*/
@Retention(AnnotationRetention.SOURCE)
@Target(AnnotationTarget.FUNCTION)
annotation class HotMethod(val message: String, val exclude: Array<String> = []) {
companion object {
const val CHECK_CONSTRUCTOR = "constructor"
const val CHECK_ANONYMOUS_OBJECT = "anonymous-object"
const val CHECK_LAMBDA = "lambda"
const val CHECK_STRING_TEMPLATE = "string-template"
const val CHECK_FACTORY = "factory"
const val CHECK_COLLECTION_OPS = "collection-ops"
}
}
1 change: 1 addition & 0 deletions detekt_custom_safe_calls.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ datadog:
# endregion
# region Android Graphics
- "android.graphics.Bitmap.recycle()"
- "android.graphics.Point.set(kotlin.Int, kotlin.Int)"
- "android.graphics.Canvas.drawColor(kotlin.Int)"
- "android.graphics.Canvas.drawColor(kotlin.Int, android.graphics.PorterDuff.Mode)"
- "android.graphics.Canvas.drawLine(kotlin.Float, kotlin.Float, kotlin.Float, kotlin.Float, android.graphics.Paint)"
Expand Down
201 changes: 201 additions & 0 deletions detekt_expensive_methods.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
build:
maxIssues: 0

config:
validation: true
warningsAsErrors: true

comments:
active: true
AbsentOrWrongFileLicense:
active: true
licenseTemplateFile: 'license.template'
licenseTemplateIsRegex: false
excludes:
# Gradle build outputs and generated artifacts.
- '**/build/**'
# Auto-generated Kotlin model fixtures produced by the buildSrc JSON-to-Kotlin code generator.
- '**/buildSrc/src/test/kotlin/com/example/model/**'
# Fixture files for the NoOpFactory annotation processor tests, read as text data rather than compiled.
- '**/tools/noopfactory/src/test/resources/**'
# Vendored test files ported from the upstream dd-trace-java library.
- '**/features/dd-sdk-android-trace-internal/src/test/kotlin/com/datadog/trace/**'

complexity:
active: false

coroutines:
active: false

empty-blocks:
active: false

exceptions:
active: false

naming:
active: false

potential-bugs:
active: false

style:
active: false

datadog:
active: true
excludes:
- '**/build/**'
- '**/test/**'
- '**/testDebug/**'
- '**/testRelease/**'
- '**/androidTest/**'
- '**/testFixtures/**'
- '**/buildSrc/**'
- '**/*.kts'
- '**/instrumented/**'
- '**/reliability/**'
- '**/sample/**'
- '**/tools/**'
HotMethodIllegalCall:
active: true
# Lambda literals passed to functions in `allowedInlineFunctions` are inlined by the compiler
# and do not allocate — they are excluded from the lambda-allocation check.
# Add any project-specific `inline` functions here.
allowedInlineFunctions:
Comment thread
satween marked this conversation as resolved.
# Kotlin scope functions
- 'let'
- 'also'
- 'apply'
- 'run'
- 'with'
- 'takeIf'
- 'takeUnless'
# Standard utilities
- 'synchronized'
- 'use'
- 'repeat'
- 'measureNanoTime'
- 'measureTimeMillis'
# Kotlin stdlib collection operations — ALL are inline, lambdas are not allocated
- 'filter'
- 'filterNot'
- 'filterIsInstance'
- 'filterNotNull'
- 'map'
- 'mapNotNull'
- 'mapIndexed'
- 'flatMap'
- 'flatten'
- 'forEach'
- 'forEachIndexed'
- 'any'
- 'all'
- 'none'
- 'count'
- 'find'
- 'findLast'
- 'first'
- 'firstOrNull'
- 'last'
- 'lastOrNull'
- 'single'
- 'singleOrNull'
- 'reduce'
- 'fold'
- 'foldIndexed'
- 'sumOf'
- 'maxByOrNull'
- 'minByOrNull'
- 'maxOrNull'
- 'minOrNull'
- 'groupBy'
- 'associate'
- 'associateBy'
- 'partition'
- 'sortedBy'
- 'sortedWith'
- 'sorted'
- 'toList'
- 'toMutableList'
- 'toSet'
- 'toMutableSet'
- 'toMap'
# Builder functions — inline, so the lambda body is inlined (the builder itself still allocates)
- 'buildString'
- 'buildList'
- 'buildSet'
- 'buildMap'
# Both lists use `ContainerFQN.method` format (or `SimpleClass.method` as a suffix match).
# For top-level factory functions the container is the package (e.g. kotlin.collections).
# emptyList/emptySet/emptyMap are intentionally excluded — they return cached singletons.
forbiddenFactoryFunctions:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add array factories to forbidden factories

The factory list omits lower-case array factories like arrayOf, arrayOfNulls, and primitive *ArrayOf calls, so @HotMethod fun f() { val xs = arrayOf(...) } allocates a new array without being caught by either the PascalCase constructor heuristic or this config. Add the kotlin.* array factory entries if the rule is intended to block heap allocations in hot paths.

Useful? React with 👍 / 👎.

- 'kotlin.collections.listOf'
- 'kotlin.collections.mutableListOf'
- 'kotlin.collections.arrayListOf'
- 'kotlin.collections.setOf'
- 'kotlin.collections.mutableSetOf'
- 'kotlin.collections.hashSetOf'
- 'kotlin.collections.linkedSetOf'
- 'kotlin.collections.mapOf'
- 'kotlin.collections.mutableMapOf'
- 'kotlin.collections.hashMapOf'
- 'kotlin.collections.linkedMapOf'
- 'kotlin.collections.buildList'
- 'kotlin.collections.buildSet'
- 'kotlin.collections.buildMap'
- 'kotlin.text.buildString'
forbiddenCalls:
# Linear search
- 'kotlin.collections.List.find'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cover mutable collection receivers

With the current List-only entries, a hot method using a MutableList receiver (for example listeners.forEach { ... } or items.map { ... }) is not matched because the resolver uses the receiver's static type (kotlin.collections.MutableList), which does not equal or suffix-match kotlin.collections.List.*. This leaves common mutable hot-path collections outside the new guardrail; add the mutable/collection receiver variants or match supertypes.

Useful? React with 👍 / 👎.

- 'kotlin.collections.List.findLast'
- 'kotlin.collections.List.first'
- 'kotlin.collections.List.firstOrNull'
- 'kotlin.collections.List.last'
- 'kotlin.collections.List.lastOrNull'
- 'kotlin.collections.List.single'
- 'kotlin.collections.List.singleOrNull'
- 'kotlin.collections.List.indexOf'
- 'kotlin.collections.List.indexOfFirst'
- 'kotlin.collections.List.indexOfLast'
- 'kotlin.collections.List.lastIndexOf'
- 'kotlin.collections.List.any'
- 'kotlin.collections.List.all'
- 'kotlin.collections.List.none'
- 'kotlin.collections.List.count'
# Full iteration — allocates an iterator each call
- 'kotlin.collections.List.forEach'
- 'kotlin.collections.List.forEachIndexed'
- 'kotlin.collections.List.reduce'
- 'kotlin.collections.List.fold'
- 'kotlin.collections.List.foldIndexed'
- 'kotlin.collections.List.sumOf'
- 'kotlin.collections.List.maxOrNull'
- 'kotlin.collections.List.minOrNull'
- 'kotlin.collections.List.maxByOrNull'
- 'kotlin.collections.List.minByOrNull'
# Materialising — allocates a new collection each call
- 'kotlin.collections.List.filter'
- 'kotlin.collections.List.filterNot'
- 'kotlin.collections.List.filterIsInstance'
- 'kotlin.collections.List.filterNotNull'
- 'kotlin.collections.List.map'
- 'kotlin.collections.List.mapNotNull'
- 'kotlin.collections.List.mapIndexed'
- 'kotlin.collections.List.flatMap'
- 'kotlin.collections.List.flatten'
- 'kotlin.collections.List.toList'
- 'kotlin.collections.List.toMutableList'
- 'kotlin.collections.List.toSet'
- 'kotlin.collections.List.toMutableSet'
- 'kotlin.collections.List.toMap'
- 'kotlin.collections.List.groupBy'
- 'kotlin.collections.List.associate'
- 'kotlin.collections.List.associateBy'
- 'kotlin.collections.List.partition'
# Sorting — O(N log N)
- 'kotlin.collections.List.sorted'
- 'kotlin.collections.List.sortedBy'
- 'kotlin.collections.List.sortedWith'
datadog-test-pyramid:
active: false
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package com.datadog.android.rum.internal.instrumentation.gestures
import android.content.Context
import android.view.MotionEvent
import androidx.core.view.GestureDetectorCompat
import com.datadog.android.internal.lint.HotMethod

internal class GesturesDetectorWrapper(
private val gestureListener: GesturesListener,
Expand All @@ -26,6 +27,7 @@ internal class GesturesDetectorWrapper(
GestureDetectorCompat(context, gestureListener)
)

@HotMethod(message = "called on every touch event forwarded from WindowCallbackWrapper")
fun onTouchEvent(event: MotionEvent) {
if (defaultGesturesDetector.onTouchEvent(event)) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import android.view.MotionEvent
import android.view.Window
import com.datadog.android.api.InternalLogger
import com.datadog.android.api.SdkCore
import com.datadog.android.internal.lint.HotMethod
import com.datadog.android.internal.utils.FixedWindowCallback
import com.datadog.android.rum.GlobalRumMonitor
import com.datadog.android.rum.RumActionType
Expand Down Expand Up @@ -40,6 +41,7 @@ internal class WindowCallbackWrapper(

// region Window.Callback

@HotMethod(message = "dispatched on every touch event by the system; avoid allocations inside")
override fun dispatchTouchEvent(event: MotionEvent?): Boolean {
if (event != null) {
// we copy it and delegate it to the gesture detector for analysis
Expand All @@ -50,8 +52,8 @@ internal class WindowCallbackWrapper(
} catch (e: Exception) {
internalLogger.log(
InternalLogger.Level.ERROR,
listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
{ "Error processing MotionEvent" },
ERROR_LOG_TARGETS,
MESSAGE_ERROR_PROCESSING_MOTION_EVENT,
e
)
} finally {
Expand All @@ -60,8 +62,8 @@ internal class WindowCallbackWrapper(
} else {
internalLogger.log(
InternalLogger.Level.ERROR,
listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY),
{ "Received null MotionEvent" }
ERROR_LOG_TARGETS,
MESSAGE_NULL_MOTION_EVENT
Comment thread
ambushwork marked this conversation as resolved.
)
}

Expand Down Expand Up @@ -175,5 +177,15 @@ internal class WindowCallbackWrapper(
const val BACK_DEFAULT_TARGET_NAME: String = "back"

const val EVENT_CONSUMED: Boolean = true

// Pre-allocated to avoid listOf() allocation on the touch hot path (error paths only,
// but still inside @HotMethod body so the allocation is avoided here).
internal val ERROR_LOG_TARGETS = listOf(
InternalLogger.Target.MAINTAINER,
InternalLogger.Target.TELEMETRY
)

private val MESSAGE_ERROR_PROCESSING_MOTION_EVENT: () -> String = { "Error processing MotionEvent" }
private val MESSAGE_NULL_MOTION_EVENT: () -> String = { "Received null MotionEvent" }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package com.datadog.android.rum.internal.metric.slowframes

import android.os.Build
import androidx.metrics.performance.FrameData
import com.datadog.android.internal.lint.HotMethod
import com.datadog.android.internal.time.TimeProvider
import com.datadog.android.rum.configuration.SlowFramesConfiguration
import com.datadog.android.rum.internal.domain.FrameMetricsData
Expand Down Expand Up @@ -64,6 +65,7 @@ internal class DefaultSlowFramesListener(
}

// Called from the background thread
@HotMethod(message = "invoked on every JankStats frame to track slow frames; synchronized inside")
override fun onFrame(volatileFrameData: FrameData) {
val viewId = currentViewId
// currentViewStartedTimestampNs can be set by RUM thread in onViewCreated after we read currentViewId,
Expand Down Expand Up @@ -109,6 +111,10 @@ internal class DefaultSlowFramesListener(
// No previous slow frame record or amount of time since the last update
// is significant enough to consider it idle - adding a new slow frame record.
if (frameDurationNs > 0) {
// SlowFrameRecord is intentionally allocated here: a new record is only
// created when a genuinely new slow-frame segment starts (not every frame),
// so the allocation rate is bounded by actual jank events, not frame rate.
@Suppress("HotMethodIllegalCall")
report.slowFramesRecords += SlowFrameRecord(
frameStartedTimestampNs,
frameDurationNs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package com.datadog.android.rum.internal.vitals
import android.os.Build
import androidx.annotation.RequiresApi
import androidx.metrics.performance.FrameData
import com.datadog.android.internal.lint.HotMethod
import com.datadog.android.internal.system.BuildSdkVersionProvider
import com.datadog.android.rum.internal.domain.FrameMetricsData
import java.util.concurrent.TimeUnit
Expand All @@ -21,6 +22,7 @@ internal class FPSVitalListener(
private var frameDeadline = EXPECTED_60_FPS_FRAME_DURATION_NS
private var displayRefreshRate: Double = SIXTY_FPS

@HotMethod(message = "invoked on every JankStats frame to compute FPS vital")
override fun onFrame(volatileFrameData: FrameData) {
val durationNs = volatileFrameData.frameDurationUiNanos
if (durationNs > 0.0) {
Expand Down
Loading
Loading