Skip to content

Commit 268a1e0

Browse files
43jayclaude
andcommitted
fix(profiling): Snapshot frame measurement deques before async serialization
ChunkMeasurementCollector#stop was passing its data maps by reference into ProfileMeasurement. The collected data is submitted async, so races with the next ChunkMeasurementCollector#start which reset the referenced maps Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 35b2bcb commit 268a1e0

File tree

2 files changed

+114
-4
lines changed

2 files changed

+114
-4
lines changed

sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import io.sentry.util.SentryRandom;
3333
import java.io.File;
3434
import java.util.ArrayDeque;
35+
import java.util.ArrayList;
3536
import java.util.HashMap;
3637
import java.util.List;
3738
import java.util.Map;
@@ -408,7 +409,8 @@ public int getActiveTraceCount() {
408409
* <p>Performance data is collected by the {@link CompositePerformanceCollector}'s Timer thread
409410
* every 100ms and returned as a list on {@code stop()}.
410411
*/
411-
private static class ChunkMeasurementCollector {
412+
@VisibleForTesting
413+
static class ChunkMeasurementCollector {
412414
private final @NotNull SentryFrameMetricsCollector frameMetricsCollector;
413415
private @Nullable String frameMetricsListenerId = null;
414416
private @Nullable CompositePerformanceCollector performanceCollector = null;
@@ -501,18 +503,20 @@ private void addFrameDataToMeasurements(
501503
measurements.put(
502504
ProfileMeasurement.ID_SLOW_FRAME_RENDERS,
503505
new ProfileMeasurement(
504-
ProfileMeasurement.UNIT_NANOSECONDS, slowFrameRenderMeasurements));
506+
ProfileMeasurement.UNIT_NANOSECONDS, new ArrayList<>(slowFrameRenderMeasurements)));
505507
}
506508
if (!frozenFrameRenderMeasurements.isEmpty()) {
507509
measurements.put(
508510
ProfileMeasurement.ID_FROZEN_FRAME_RENDERS,
509511
new ProfileMeasurement(
510-
ProfileMeasurement.UNIT_NANOSECONDS, frozenFrameRenderMeasurements));
512+
ProfileMeasurement.UNIT_NANOSECONDS,
513+
new ArrayList<>(frozenFrameRenderMeasurements)));
511514
}
512515
if (!screenFrameRateMeasurements.isEmpty()) {
513516
measurements.put(
514517
ProfileMeasurement.ID_SCREEN_FRAME_RATES,
515-
new ProfileMeasurement(ProfileMeasurement.UNIT_HZ, screenFrameRateMeasurements));
518+
new ProfileMeasurement(
519+
ProfileMeasurement.UNIT_HZ, new ArrayList<>(screenFrameRateMeasurements)));
516520
}
517521
}
518522

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package io.sentry.android.core
2+
3+
import io.sentry.CompositePerformanceCollector
4+
import io.sentry.PerformanceCollectionData
5+
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector
6+
import io.sentry.profilemeasurements.ProfileMeasurement
7+
import kotlin.test.Test
8+
import kotlin.test.assertEquals
9+
import org.mockito.kotlin.any
10+
import org.mockito.kotlin.argumentCaptor
11+
import org.mockito.kotlin.mock
12+
import org.mockito.kotlin.times
13+
import org.mockito.kotlin.verify
14+
import org.mockito.kotlin.whenever
15+
16+
class ChunkMeasurementCollectorTest {
17+
18+
/**
19+
* Drives [PerfettoContinuousProfiler.ChunkMeasurementCollector] through two full `start ->
20+
* collect -> stop` cycles to assert that the metrics collected are correct.
21+
*/
22+
@Test
23+
fun `each start-stop cycle returns its own independent measurements`() {
24+
val frameMetricsCollector: SentryFrameMetricsCollector = mock()
25+
val performanceCollector: CompositePerformanceCollector = mock()
26+
val collector = PerfettoContinuousProfiler.ChunkMeasurementCollector(frameMetricsCollector)
27+
val listenerCaptor = argumentCaptor<SentryFrameMetricsCollector.FrameMetricsCollectorListener>()
28+
29+
// Return distinct performance data for each stop() call.
30+
whenever(performanceCollector.stop(any<String>()))
31+
.thenReturn(
32+
// Cycle 1: 2 samples, both with cpu + heap, only first with native.
33+
listOf(
34+
perfData(nanos = 100L, cpu = 10.0, heap = 1_000L, native = 500L),
35+
perfData(nanos = 200L, cpu = 20.0, heap = 2_000L, native = null),
36+
),
37+
// Cycle 2: 3 samples, all with heap, only some with cpu/native.
38+
listOf(
39+
perfData(nanos = 1_000L, cpu = 30.0, heap = 3_000L, native = null),
40+
perfData(nanos = 1_100L, cpu = null, heap = 4_000L, native = 800L),
41+
perfData(nanos = 1_200L, cpu = 50.0, heap = 5_000L, native = 900L),
42+
),
43+
)
44+
45+
// --- Cycle 1 ---
46+
collector.start(performanceCollector, "chunk-1")
47+
verify(frameMetricsCollector).startCollection(listenerCaptor.capture())
48+
// onFrameMetricCollected(frameStart, frameEnd, duration, delay, isSlow, isFrozen, refreshRate)
49+
listenerCaptor.lastValue.apply {
50+
onFrameMetricCollected(0L, 1_000L, 100L, 0L, true, false, 60.0f) // slow
51+
onFrameMetricCollected(0L, 2_000L, 800L, 0L, false, true, 60.0f) // frozen
52+
onFrameMetricCollected(0L, 3_000L, 50L, 0L, false, false, 90.0f) // refresh change
53+
}
54+
val chunk1 = collector.stop()
55+
56+
// --- Cycle 2 ---
57+
collector.start(performanceCollector, "chunk-2")
58+
verify(frameMetricsCollector, times(2)).startCollection(listenerCaptor.capture())
59+
listenerCaptor.lastValue.apply {
60+
onFrameMetricCollected(0L, 10_000L, 150L, 0L, true, false, 60.0f) // slow
61+
onFrameMetricCollected(0L, 11_000L, 200L, 0L, true, false, 60.0f) // slow
62+
onFrameMetricCollected(0L, 12_000L, 900L, 0L, false, true, 60.0f) // frozen
63+
}
64+
val chunk2 = collector.stop()
65+
66+
// Cycle 1: 1 slow, 1 frozen; refresh rate goes 0 -> 60 -> 90 (2 changes recorded);
67+
// 2 cpu samples, 2 heap samples, 1 native sample.
68+
assertChunkCounts(chunk1, slow = 1, frozen = 1, refreshRate = 2, cpu = 2, heap = 2, native = 1)
69+
// Cycle 2: 2 slow, 1 frozen; refresh rate goes 0 -> 60 (1 change recorded);
70+
// 2 cpu samples (one was null), 3 heap samples, 2 native samples.
71+
assertChunkCounts(chunk2, slow = 2, frozen = 1, refreshRate = 1, cpu = 2, heap = 3, native = 2)
72+
}
73+
74+
private fun perfData(nanos: Long, cpu: Double?, heap: Long?, native: Long?) =
75+
PerformanceCollectionData(nanos).apply {
76+
cpuUsagePercentage = cpu
77+
usedHeapMemory = heap
78+
usedNativeMemory = native
79+
}
80+
81+
private fun assertChunkCounts(
82+
measurements: Map<String, ProfileMeasurement>,
83+
slow: Int,
84+
frozen: Int,
85+
refreshRate: Int,
86+
cpu: Int,
87+
heap: Int,
88+
native: Int,
89+
) {
90+
assertEquals(slow, measurements[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]?.values?.size ?: 0)
91+
assertEquals(
92+
frozen,
93+
measurements[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]?.values?.size ?: 0,
94+
)
95+
assertEquals(
96+
refreshRate,
97+
measurements[ProfileMeasurement.ID_SCREEN_FRAME_RATES]?.values?.size ?: 0,
98+
)
99+
assertEquals(cpu, measurements[ProfileMeasurement.ID_CPU_USAGE]?.values?.size ?: 0)
100+
assertEquals(heap, measurements[ProfileMeasurement.ID_MEMORY_FOOTPRINT]?.values?.size ?: 0)
101+
assertEquals(
102+
native,
103+
measurements[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]?.values?.size ?: 0,
104+
)
105+
}
106+
}

0 commit comments

Comments
 (0)