Skip to content

Commit

Permalink
[QA] Avoid collecting normal frames (#3782)
Browse files Browse the repository at this point in the history
* avoid keeping normal frames in memory when collecting slow/frozen frames
* renamed SpanFrameMetricsCollector.getTotalFrameCount method to getSlowFrozenFrameCount
* confirm no delay in UI test when there are no slow/frozen frames collected
  • Loading branch information
stefanosiano authored Nov 14, 2024
1 parent a183163 commit dab52e2
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

### Fixes

- Avoid collecting normal frames ([#3782](https://github.com/getsentry/sentry-java/pull/3782))
- Ensure android initialization process continues even if options configuration block throws an exception ([#3887](https://github.com/getsentry/sentry-java/pull/3887))
- Do not report parsing ANR error when there are no threads ([#3888](https://github.com/getsentry/sentry-java/pull/3888))
- This should significantly reduce the number of events with message "Sentry Android SDK failed to parse system thread dump..." reported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
@ApiStatus.Internal
final class SentryFrameMetrics {

private int normalFrameCount;
private int slowFrameCount;
private int frozenFrameCount;

Expand All @@ -18,15 +17,11 @@ final class SentryFrameMetrics {
public SentryFrameMetrics() {}

public SentryFrameMetrics(
final int normalFrameCount,
final int slowFrameCount,
final long slowFrameDelayNanos,
final int frozenFrameCount,
final long frozenFrameDelayNanos,
final long totalDurationNanos) {

this.normalFrameCount = normalFrameCount;

this.slowFrameCount = slowFrameCount;
this.slowFrameDelayNanos = slowFrameDelayNanos;

Expand All @@ -47,15 +42,9 @@ public void addFrame(
} else if (isSlow) {
slowFrameDelayNanos += delayNanos;
slowFrameCount += 1;
} else {
normalFrameCount += 1;
}
}

public int getNormalFrameCount() {
return normalFrameCount;
}

public int getSlowFrameCount() {
return slowFrameCount;
}
Expand All @@ -72,17 +61,16 @@ public long getFrozenFrameDelayNanos() {
return frozenFrameDelayNanos;
}

public int getTotalFrameCount() {
return normalFrameCount + slowFrameCount + frozenFrameCount;
/** Returns the sum of the slow and frozen frames. */
public int getSlowFrozenFrameCount() {
return slowFrameCount + frozenFrameCount;
}

public long getTotalDurationNanos() {
return totalDurationNanos;
}

public void clear() {
normalFrameCount = 0;

slowFrameCount = 0;
slowFrameDelayNanos = 0;

Expand All @@ -95,7 +83,6 @@ public void clear() {
@NotNull
public SentryFrameMetrics duplicate() {
return new SentryFrameMetrics(
normalFrameCount,
slowFrameCount,
slowFrameDelayNanos,
frozenFrameCount,
Expand All @@ -110,7 +97,6 @@ public SentryFrameMetrics duplicate() {
@NotNull
public SentryFrameMetrics diffTo(final @NotNull SentryFrameMetrics other) {
return new SentryFrameMetrics(
normalFrameCount - other.normalFrameCount,
slowFrameCount - other.slowFrameCount,
slowFrameDelayNanos - other.slowFrameDelayNanos,
frozenFrameCount - other.frozenFrameCount,
Expand All @@ -123,8 +109,7 @@ public SentryFrameMetrics diffTo(final @NotNull SentryFrameMetrics other) {
* to 0
*/
public boolean containsValidData() {
return normalFrameCount >= 0
&& slowFrameCount >= 0
return slowFrameCount >= 0
&& slowFrameDelayNanos >= 0
&& frozenFrameCount >= 0
&& frozenFrameDelayNanos >= 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ private void captureFrameMetrics(@NotNull final ISpan span) {
}
}

int totalFrameCount = frameMetrics.getTotalFrameCount();
int totalFrameCount = frameMetrics.getSlowFrozenFrameCount();

final long nextScheduledFrameNanos = frameMetricsCollector.getLastKnownFrameStartTimeNanos();
// nextScheduledFrameNanos might be -1 if no frames have been scheduled for drawing yet
Expand Down Expand Up @@ -254,15 +254,17 @@ public void onFrameMetricCollected(
(long) ((double) ONE_SECOND_NANOS / (double) refreshRate);
lastKnownFrameDurationNanos = expectedFrameDurationNanos;

frames.add(
new Frame(
frameStartNanos,
frameEndNanos,
durationNanos,
delayNanos,
isSlow,
isFrozen,
expectedFrameDurationNanos));
if (isSlow || isFrozen) {
frames.add(
new Frame(
frameStartNanos,
frameEndNanos,
durationNanos,
delayNanos,
isSlow,
isFrozen,
expectedFrameDurationNanos));
}
}

private static int interpolateFrameCount(
Expand All @@ -277,7 +279,7 @@ private static int interpolateFrameCount(
final long frameMetricsDurationNanos = frameMetrics.getTotalDurationNanos();
final long nonRenderedDuration = spanDurationNanos - frameMetricsDurationNanos;
if (nonRenderedDuration > 0) {
return (int) (nonRenderedDuration / frameDurationNanos);
return (int) Math.ceil((double) nonRenderedDuration / frameDurationNanos);
}
return 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,6 @@ import kotlin.test.assertFalse
import kotlin.test.assertTrue

class SentryFrameMetricsTest {
@Test
fun addFastFrame() {
val frameMetrics = SentryFrameMetrics()
frameMetrics.addFrame(10, 0, false, false)
assertEquals(1, frameMetrics.normalFrameCount)

frameMetrics.addFrame(10, 0, false, false)
assertEquals(2, frameMetrics.normalFrameCount)
}

@Test
fun addSlowFrame() {
Expand Down Expand Up @@ -43,10 +34,12 @@ class SentryFrameMetricsTest {
@Test
fun totalFrameCount() {
val frameMetrics = SentryFrameMetrics()
// Normal frames are ignored
frameMetrics.addFrame(10, 0, false, false)
// Slow and frozen frames are considered
frameMetrics.addFrame(116, 100, true, false)
frameMetrics.addFrame(1016, 1000, true, true)
assertEquals(3, frameMetrics.totalFrameCount)
assertEquals(2, frameMetrics.slowFrozenFrameCount)
}

@Test
Expand All @@ -57,12 +50,11 @@ class SentryFrameMetricsTest {
frameMetrics.addFrame(1016, 1000, true, true)

val dup = frameMetrics.duplicate()
assertEquals(1, dup.normalFrameCount)
assertEquals(1, dup.slowFrameCount)
assertEquals(100, dup.slowFrameDelayNanos)
assertEquals(1, dup.frozenFrameCount)
assertEquals(1000, dup.frozenFrameDelayNanos)
assertEquals(3, dup.totalFrameCount)
assertEquals(2, dup.slowFrozenFrameCount)
}

@Test
Expand All @@ -89,7 +81,7 @@ class SentryFrameMetricsTest {
assertEquals(1, diff.frozenFrameCount)
assertEquals(1000, diff.frozenFrameDelayNanos)

assertEquals(2, diff.totalFrameCount)
assertEquals(2, diff.slowFrozenFrameCount)
}

@Test
Expand All @@ -102,12 +94,11 @@ class SentryFrameMetricsTest {

frameMetrics.clear()

assertEquals(0, frameMetrics.normalFrameCount)
assertEquals(0, frameMetrics.slowFrameCount)
assertEquals(0, frameMetrics.slowFrameDelayNanos)
assertEquals(0, frameMetrics.frozenFrameCount)
assertEquals(0, frameMetrics.frozenFrameDelayNanos)
assertEquals(0, frameMetrics.totalFrameCount)
assertEquals(0, frameMetrics.slowFrozenFrameCount)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class SpanFrameMetricsCollectorTest {
sut.onFrameMetricCollected(0, 10, 10, 0, false, false, 60.0f)
sut.onFrameMetricCollected(16, 48, 32, 16, true, false, 60.0f)
sut.onFrameMetricCollected(60, 92, 32, 16, true, false, 60.0f)
sut.onFrameMetricCollected(100, 800, 800, 784, true, true, 60.0f)
sut.onFrameMetricCollected(100, 800, 700, 784, true, true, 60.0f)

// then a second span starts
fixture.timeNanos = 800
Expand Down Expand Up @@ -337,10 +337,11 @@ class SpanFrameMetricsCollectorTest {
fixture.timeNanos = TimeUnit.SECONDS.toNanos(2)
sut.onSpanFinished(span)

// then still 60 frames should be reported (1 second at 60fps)
verify(span).setData("frames.total", 60)
// then still 61 frames should be reported (1 second at 60fps with approximation)
verify(span).setData("frames.total", 61)
verify(span).setData("frames.slow", 0)
verify(span).setData("frames.frozen", 0)
verify(span).setData("frames.delay", 0.0)
}

@Test
Expand All @@ -364,9 +365,9 @@ class SpanFrameMetricsCollectorTest {
sut.onSpanFinished(span)

// then
// still 60 fps should be reported for 1 seconds
// still 61 fps should be reported for 1 seconds (with approximation)
// and one frame with frame delay should be reported (1s - 16ms)
verify(span).setData("frames.total", 61)
verify(span).setData("frames.total", 62)
verify(span).setData("frames.slow", 0)
verify(span).setData("frames.frozen", 1)
verify(span).setData(eq("frames.delay"), AdditionalMatchers.eq(0.983333334, 0.01))
Expand Down

0 comments on commit dab52e2

Please sign in to comment.