firebase / firebase/firebase-android-sdk

Performance Gradle plugin instruments synthetic bridge methods, duplicating @AddTrace traces

Open
#8,579 1 comment 1 reaction 0 assignees View on GitHub
api: performance
Dominant language
Java
Stars
2.6k
Forks
710
Avg merge
2d 23h
Merged PRs (30d)
34

Description

### [READ] Step 1: Are you in the right place?

Yes: this is a bug in firebase-perf-gradle, the Firebase Performance Gradle plugin hosted in this repository.

### [REQUIRED] Step 2: Describe your environment

- Android Studio version: **N/A (reproduced from the command line)**
- Firebase Component: **Performance Monitoring (Gradle plugin bytecode instrumentation)**
- Component version: **Gradle plugin 2.0.2 / com.google.firebase:firebase-perf:22.0.6**

### [REQUIRED] Step 3: Describe the problem

The plugin instruments **every** method carrying `@AddTrace`, including compiler-generated `ACC_BRIDGE` / `ACC_SYNTHETIC` methods. When a Kotlin method that implements a generic interface is
annotated, the erasure bridge carries a copy of the annotation, so the plugin emits `FirebasePerformance.startTrace(...)` / `Trace.stop()` in both the real method and the bridge.

Since a call through the interface goes through the bridge, which then calls the real method, **a single call reports the trace twice**, the bridge trace wrapping the real one. Consequences: trace counts are doubled in the Firebase console (durations stay correct).

Kotlin copies method annotations onto erasure bridges as of language version 2.4 ([KT-82655](https://youtrack.jetbrains.com/issue/KT-82655), requested by [KT-38983](https://youtrack.jetbrains.com/issue/KT-38983)), which is why this surfaces now. Copying the annotation is intended Kotlin behaviour, the bug is that the instrumentation does not skip bridges.

Real-world impact: our app annotates the `create` method of about fifteen `androidx.startup.Initializer` implementations, and every one of those startup traces is now reported twice.

#### Steps to reproduce:

Minimal reproduction project attached [firebase-perf-bridge-repro.zip](https://github.com/user-attachments/files/31959135/firebase-perf-bridge-repro.zip)

```bash
./gradlew :app:assembleDebug
javap -v -p -c app/build/intermediates/classes/debug/transformDebugClassesWithAsm/dirs/com/example/bridgerepro/UnitFactory.class
```

**Actual result**, both `create()V` and its synthetic bridge `create()Ljava/lang/Object;` are instrumented:

```
public void create();
flags: (0x0001) ACC_PUBLIC
2: invokestatic // FirebasePerformance.startTrace:(String)Trace
11: invokevirtual // Trace.stop:()V
com.google.firebase.perf.metrics.AddTrace(name="unit_factory_create")

public java.lang.Object create();
flags: (0x1041) ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
2: invokestatic // FirebasePerformance.startTrace:(String)Trace
14: invokevirtual // Trace.stop:()V
com.google.firebase.perf.metrics.AddTrace(name="unit_factory_create")
```

The reproduction is intentionally build-time only (no `Activity`, no `google-services.json`), since the duplication is already unambiguous in the bytecode. At runtime, a call through `Factory` enters the bridge, which starts a trace and then calls the real method, which starts a second trace under the same name.

**Expected result**, only `create()V` is instrumented and the synthetic bridge is left untouched.

**Control**, uncommenting `freeCompilerArgs.add("-language-version=2.3")` in `app/build.gradle.kts` makes the duplication disappear: with language version 2.3 the annotation is not copied onto the bridge, so only one method is instrumented. This isolates the plugin's handling of bridges as the cause.

The reproduction also contains a `Factory` implementation, showing the behaviour is not specific to `Unit`.

#### Relevant Code:

The whole reproduction is one file:

```kotlin
interface Factory {
fun create(): T
}

class UnitFactory : Factory {

@AddTrace(name = "unit_factory_create")
override fun create() {
Thread.sleep(1)
}
}
```

On the plugin side, `InstrumentationVisitor.visitMethod` already receives `access`, but only forwards it to `FirebasePerfMethodVisitor`, nothing filters on it ([`InstrumentationVisitor.java`](https://github.com/firebase/firebase-android-sdk/blob/main/firebase-perf-gradle/src/main/java/com/google/firebase/perf/plugin/instrumentation/InstrumentationVisitor.java)):

```java
final MethodVisitor rootMethodVisitor =
classVisitor.visitMethod(access, methodName, methodDesc, signature, exceptions);
return new FirebasePerfMethodVisitor(
classInfo.type.getDescriptor(), api, rootMethodVisitor, access, methodName, methodDesc, instrConfig);
```

`FirebasePerfMethodVisitor.visitAnnotation` then dispatches purely on the annotation descriptor, and [`FirebaseTimerAnnotationProcessor.onMethodEnter`](https://github.com/firebase/firebase-android-sdk/blob/main/firebase-perf-gradle/src/main/java/com/google/firebase/perf/plugin/instrumentation/annotation/FirebaseTimerAnnotationProcessor.java#L42) only reads the annotation's `enabled` attribute, so nothing rejects a bridge.

Bridge methods contain no user code, so returning the uninstrumented visitor for them would fix it:

```java
if ((access & Opcodes.ACC_BRIDGE) != 0 || (access & Opcodes.ACC_SYNTHETIC) != 0) {
return rootMethodVisitor;
}
```

Filtering on `ACC_BRIDGE` alone is enough for this case; also skipping `ACC_SYNTHETIC` is safe hardening, since `@AddTrace` cannot be applied to compiler-generated code in the first place.

Contributor guide

Open the contributing guide

Research direction

Start in firebase-perf-gradle/src/main/java/com/google/firebase/perf/plugin/instrumentation/InstrumentationVisitor.java and trace how access reaches FirebasePerfMethodVisitor.visitAnnotation and FirebaseTimerAnnotationProcessor.onMethodEnter. Build the attached reproduction with ./gradlew :app:assembleDebug, then inspect UnitFactory.class with javap; done means only create()V is instrumented while the bridge remains untouched.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, java, kotlin
Domain
build-system
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.