diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b37b59967..154e00f3da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Fixes - Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076)) +- Improve frame delay collection performance by using sentry-java `getFramesDelay` API ([#6074](https://github.com/getsentry/sentry-react-native/pull/6074)) ### Dependencies diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryFrameDelayCollector.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryFrameDelayCollector.java deleted file mode 100644 index a3295ed4b4..0000000000 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryFrameDelayCollector.java +++ /dev/null @@ -1,128 +0,0 @@ -package io.sentry.react; - -import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import org.jetbrains.annotations.Nullable; - -/** - * Collects per-frame delay data from {@link SentryFrameMetricsCollector} and provides a method to - * query the accumulated delay within a given time range. - * - *

This is a temporary solution until sentry-java exposes a queryable API for frames delay - * (similar to sentry-cocoa's getFramesDelaySPI). - */ -public class RNSentryFrameDelayCollector - implements SentryFrameMetricsCollector.FrameMetricsCollectorListener { - - private static final long MAX_FRAME_AGE_NANOS = 5L * 60 * 1_000_000_000L; // 5 minutes - - private final List frames = new CopyOnWriteArrayList<>(); - - private @Nullable String listenerId; - private @Nullable SentryFrameMetricsCollector collector; - - /** - * Starts collecting frame delay data from the given collector. - * - * @return true if collection was started successfully - */ - public boolean start(@Nullable SentryFrameMetricsCollector frameMetricsCollector) { - if (frameMetricsCollector == null) { - return false; - } - stop(); - this.collector = frameMetricsCollector; - this.listenerId = frameMetricsCollector.startCollection(this); - return this.listenerId != null; - } - - /** Stops collecting frame delay data. */ - public void stop() { - if (collector != null && listenerId != null) { - collector.stopCollection(listenerId); - listenerId = null; - collector = null; - } - frames.clear(); - } - - @Override - public void onFrameMetricCollected( - long frameStartNanos, - long frameEndNanos, - long durationNanos, - long delayNanos, - boolean isSlow, - boolean isFrozen, - float refreshRate) { - if (delayNanos <= 0) { - return; - } - frames.add(new FrameRecord(frameStartNanos, frameEndNanos, delayNanos)); - pruneOldFrames(frameEndNanos); - } - - /** - * Returns the total frames delay in seconds for the given time range. - * - *

Handles partial overlap: if a frame's delay period partially falls within the query range, - * only the overlapping portion is counted. - * - * @param startNanos start of the query range in system nanos (e.g., System.nanoTime()) - * @param endNanos end of the query range in system nanos - * @return delay in seconds, or -1 if no data is available - */ - public double getFramesDelay(long startNanos, long endNanos) { - if (startNanos >= endNanos) { - return -1; - } - - long totalDelayNanos = 0; - - for (FrameRecord frame : frames) { - if (frame.endNanos <= startNanos) { - continue; - } - if (frame.startNanos >= endNanos) { - break; - } - - // The delay portion of a frame is at the end of the frame duration. - // delayStart = frameEnd - delay, delayEnd = frameEnd - long delayStart = frame.endNanos - frame.delayNanos; - long delayEnd = frame.endNanos; - - // Intersect the delay interval with the query range - long overlapStart = Math.max(delayStart, startNanos); - long overlapEnd = Math.min(delayEnd, endNanos); - - if (overlapEnd > overlapStart) { - totalDelayNanos += (overlapEnd - overlapStart); - } - } - - return totalDelayNanos / 1e9; - } - - private void pruneOldFrames(long currentNanos) { - long cutoff = currentNanos - MAX_FRAME_AGE_NANOS; - // Remove from the front one-by-one. CopyOnWriteArrayList.remove(0) is O(n) per call, - // but old frames are pruned incrementally so typically only 0-1 entries are removed. - while (!frames.isEmpty() && frames.get(0).endNanos < cutoff) { - frames.remove(0); - } - } - - private static class FrameRecord { - final long startNanos; - final long endNanos; - final long delayNanos; - - FrameRecord(long startNanos, long endNanos, long delayNanos) { - this.startNanos = startNanos; - this.endNanos = endNanos; - this.delayNanos = delayNanos; - } - } -} diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 4136eb5d3b..f2f492bd8d 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -45,6 +45,7 @@ import io.sentry.android.core.InternalSentrySdk; import io.sentry.android.core.SentryAndroidDateProvider; import io.sentry.android.core.SentryAndroidOptions; +import io.sentry.android.core.SentryFramesDelayResult; import io.sentry.android.core.SentryShakeDetector; import io.sentry.android.core.ViewHierarchyEventProcessor; import io.sentry.android.core.internal.debugmeta.AssetsDebugMetaLoader; @@ -98,7 +99,8 @@ public class RNSentryModuleImpl { private final ReactApplicationContext reactApplicationContext; private final PackageInfo packageInfo; private FrameMetricsAggregator frameMetricsAggregator = null; - private final RNSentryFrameDelayCollector frameDelayCollector = new RNSentryFrameDelayCollector(); + @VisibleForTesting @Nullable SentryFrameMetricsCollector frameMetricsCollector = null; + private @Nullable String frameMetricsListenerId = null; private boolean androidXAvailable; @VisibleForTesting static long lastStartTimestampMs = -1; @@ -413,9 +415,14 @@ public void fetchNativeFramesDelay( long startNanos = nowNanos - (long) (startOffsetSeconds * 1e9); long endNanos = nowNanos - (long) (endOffsetSeconds * 1e9); - double delaySeconds = frameDelayCollector.getFramesDelay(startNanos, endNanos); - if (delaySeconds >= 0) { - promise.resolve(delaySeconds); + if (frameMetricsCollector == null) { + promise.resolve(null); + return; + } + + SentryFramesDelayResult result = frameMetricsCollector.getFramesDelay(startNanos, endNanos); + if (result != null && result.getDelaySeconds() >= 0) { + promise.resolve(result.getDelaySeconds()); } else { promise.resolve(null); } @@ -747,12 +754,28 @@ public void enableNativeFramesTracking() { if (options instanceof SentryAndroidOptions) { final SentryFrameMetricsCollector collector = ((SentryAndroidOptions) options).getFrameMetricsCollector(); - if (frameDelayCollector.start(collector)) { - logger.log(SentryLevel.INFO, "RNSentryFrameDelayCollector installed."); + if (collector != null) { + // Register a no-op listener to ensure frame metrics collection is active. + // This is needed so that getFramesDelay() has data to query. + stopFrameMetricsCollection(); + String listenerId = + collector.startCollection( + (startNanos, + endNanos, + durationNanos, + delayNanos, + isSlow, + isFrozen, + refreshRate) -> {}); + if (listenerId != null) { + frameMetricsCollector = collector; + frameMetricsListenerId = listenerId; + logger.log(SentryLevel.INFO, "SentryFrameMetricsCollector listener installed."); + } } } } catch (Throwable ignored) { // NOPMD - We don't want to crash in any case - logger.log(SentryLevel.WARNING, "Error starting RNSentryFrameDelayCollector."); + logger.log(SentryLevel.WARNING, "Error starting frame metrics collection."); } } @@ -761,7 +784,15 @@ public void disableNativeFramesTracking() { frameMetricsAggregator.stop(); frameMetricsAggregator = null; } - frameDelayCollector.stop(); + stopFrameMetricsCollection(); + } + + private void stopFrameMetricsCollection() { + if (frameMetricsCollector != null && frameMetricsListenerId != null) { + frameMetricsCollector.stopCollection(frameMetricsListenerId); + } + frameMetricsCollector = null; + frameMetricsListenerId = null; } public void getNewScreenTimeToDisplay(Promise promise) { diff --git a/packages/core/android/src/test/java/io/sentry/react/RNSentryFramesDelayTest.java b/packages/core/android/src/test/java/io/sentry/react/RNSentryFramesDelayTest.java new file mode 100644 index 0000000000..976dee2774 --- /dev/null +++ b/packages/core/android/src/test/java/io/sentry/react/RNSentryFramesDelayTest.java @@ -0,0 +1,101 @@ +package io.sentry.react; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; +import com.facebook.react.bridge.Promise; +import com.facebook.react.bridge.ReactApplicationContext; +import io.sentry.android.core.SentryFramesDelayResult; +import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; +import org.junit.Before; +import org.junit.Test; + +public class RNSentryFramesDelayTest { + + private RNSentryModuleImpl module; + private Promise promise; + + @Before + public void setUp() throws Exception { + ReactApplicationContext reactContext = mock(ReactApplicationContext.class); + PackageManager packageManager = mock(PackageManager.class); + when(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(new PackageInfo()); + when(reactContext.getPackageManager()).thenReturn(packageManager); + when(reactContext.getPackageName()).thenReturn("com.test.app"); + module = new RNSentryModuleImpl(reactContext); + promise = mock(Promise.class); + } + + @Test + public void resolvesNullWhenCollectorIsNull() { + module.frameMetricsCollector = null; + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now - 1.0, now, promise); + verify(promise).resolve(isNull()); + } + + @Test + public void resolvesDelayFromCollector() { + SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class); + when(collector.getFramesDelay(anyLong(), anyLong())) + .thenReturn(new SentryFramesDelayResult(0.123, 2)); + module.frameMetricsCollector = collector; + + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now - 1.0, now, promise); + verify(promise).resolve(eq(0.123)); + } + + @Test + public void resolvesNullWhenDelayIsNegative() { + SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class); + when(collector.getFramesDelay(anyLong(), anyLong())) + .thenReturn(new SentryFramesDelayResult(-1, 0)); + module.frameMetricsCollector = collector; + + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now - 1.0, now, promise); + verify(promise).resolve(isNull()); + } + + @Test + public void resolvesNullWhenResultIsNull() { + SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class); + when(collector.getFramesDelay(anyLong(), anyLong())).thenReturn(null); + module.frameMetricsCollector = collector; + + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now - 1.0, now, promise); + verify(promise).resolve(isNull()); + } + + @Test + public void resolvesNullWhenStartIsInFuture() { + SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class); + module.frameMetricsCollector = collector; + + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now + 100.0, now + 200.0, promise); + verify(promise).resolve(isNull()); + } + + @Test + public void resolvesZeroDelayWhenNoSlowFrames() { + SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class); + when(collector.getFramesDelay(anyLong(), anyLong())) + .thenReturn(new SentryFramesDelayResult(0.0, 0)); + module.frameMetricsCollector = collector; + + double now = System.currentTimeMillis() / 1e3; + module.fetchNativeFramesDelay(now - 1.0, now, promise); + verify(promise).resolve(eq(0.0)); + } +}