diff --git a/CHANGELOG.md b/CHANGELOG.md index 813e9e65bc..cd68c3158e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Use a separate `Random` instance per thread to improve SDK performance ([#3835](https://github.com/getsentry/sentry-java/pull/3835)) + ### Fixes - Accept manifest integer values when requiring floating values ([#3823](https://github.com/getsentry/sentry-java/pull/3823)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java index 0399b634fb..e914029c30 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java @@ -54,7 +54,7 @@ import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.HintUtils; -import io.sentry.util.Random; +import io.sentry.util.SentryRandom; import java.io.File; import java.util.ArrayList; import java.util.Arrays; @@ -83,24 +83,13 @@ public final class AnrV2EventProcessor implements BackfillingEventProcessor { private final @NotNull SentryExceptionFactory sentryExceptionFactory; - private final @Nullable Random random; - public AnrV2EventProcessor( final @NotNull Context context, final @NotNull SentryAndroidOptions options, final @NotNull BuildInfoProvider buildInfoProvider) { - this(context, options, buildInfoProvider, null); - } - - AnrV2EventProcessor( - final @NotNull Context context, - final @NotNull SentryAndroidOptions options, - final @NotNull BuildInfoProvider buildInfoProvider, - final @Nullable Random random) { this.context = ContextUtils.getApplicationContext(context); this.options = options; this.buildInfoProvider = buildInfoProvider; - this.random = random; final SentryStackTraceFactory sentryStackTraceFactory = new SentryStackTraceFactory(this.options); @@ -180,9 +169,8 @@ private boolean sampleReplay(final @NotNull SentryEvent event) { try { // we have to sample here with the old sample rate, because it may change between app launches - final @NotNull Random random = this.random != null ? this.random : new Random(); final double replayErrorSampleRateDouble = Double.parseDouble(replayErrorSampleRate); - if (replayErrorSampleRateDouble < random.nextDouble()) { + if (replayErrorSampleRateDouble < SentryRandom.current().nextDouble()) { options .getLogger() .log( diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index ed06b29b35..8e266bcdba 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5829,6 +5829,11 @@ public final class io/sentry/util/SampleRateUtils { public static fun isValidTracesSampleRate (Ljava/lang/Double;Z)Z } +public final class io/sentry/util/SentryRandom { + public fun ()V + public static fun current ()Lio/sentry/util/Random; +} + public final class io/sentry/util/StringUtils { public static fun byteCountToString (J)Ljava/lang/String; public static fun calculateStringHash (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 27529d100b..b053230ce5 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -18,6 +18,7 @@ import io.sentry.util.HintUtils; import io.sentry.util.Objects; import io.sentry.util.Random; +import io.sentry.util.SentryRandom; import io.sentry.util.TracingUtils; import java.io.Closeable; import java.io.IOException; @@ -40,7 +41,6 @@ public final class SentryClient implements ISentryClient, IMetricsClient { private final @NotNull SentryOptions options; private final @NotNull ITransport transport; - private final @Nullable Random random; private final @NotNull SortBreadcrumbsByDate sortBreadcrumbsByDate = new SortBreadcrumbsByDate(); private final @NotNull IMetricsAggregator metricsAggregator; @@ -66,8 +66,6 @@ public boolean isEnabled() { options.isEnableMetrics() ? new MetricsAggregator(options, this) : NoopMetricsAggregator.getInstance(); - - this.random = options.getSampleRate() == null ? null : new Random(); } private boolean shouldApplyScopeData( @@ -1183,6 +1181,7 @@ public boolean isHealthy() { } private boolean sample() { + final @Nullable Random random = options.getSampleRate() == null ? null : SentryRandom.current(); // https://docs.sentry.io/development/sdk-dev/features/#event-sampling if (options.getSampleRate() != null && random != null) { final double sampling = options.getSampleRate(); diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 5e5b808333..ef04cae369 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -2,6 +2,7 @@ import io.sentry.util.Objects; import io.sentry.util.Random; +import io.sentry.util.SentryRandom; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -10,14 +11,14 @@ final class TracesSampler { private static final @NotNull Double DEFAULT_TRACES_SAMPLE_RATE = 1.0; private final @NotNull SentryOptions options; - private final @NotNull Random random; + private final @Nullable Random random; public TracesSampler(final @NotNull SentryOptions options) { - this(Objects.requireNonNull(options, "options are required"), new Random()); + this(Objects.requireNonNull(options, "options are required"), null); } @TestOnly - TracesSampler(final @NotNull SentryOptions options, final @NotNull Random random) { + TracesSampler(final @NotNull SentryOptions options, final @Nullable Random random) { this.options = options; this.random = random; } @@ -90,6 +91,13 @@ TracesSamplingDecision sample(final @NotNull SamplingContext samplingContext) { } private boolean sample(final @NotNull Double aDouble) { - return !(aDouble < random.nextDouble()); + return !(aDouble < getRandom().nextDouble()); + } + + private Random getRandom() { + if (random == null) { + return SentryRandom.current(); + } + return random; } } diff --git a/sentry/src/main/java/io/sentry/util/SentryRandom.java b/sentry/src/main/java/io/sentry/util/SentryRandom.java new file mode 100644 index 0000000000..f6e1d0a974 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/SentryRandom.java @@ -0,0 +1,37 @@ +package io.sentry.util; + +import org.jetbrains.annotations.NotNull; + +/** + * This SentryRandom is a compromise used for improving performance of the SDK. + * + *

We did some testing where using Random from multiple threads degrades performance + * significantly. We opted for this approach as it wasn't easily possible to vendor + * ThreadLocalRandom since it's using advanced features that can cause java.lang.IllegalAccessError. + */ +public final class SentryRandom { + + private static final @NotNull SentryRandomThreadLocal instance = new SentryRandomThreadLocal(); + + /** + * Returns the current threads instance of {@link Random}. An instance of {@link Random} will be + * created the first time this is invoked on each thread. + * + *

NOTE: Avoid holding a reference to the returned {@link Random} instance as sharing a + * reference across threads (while being thread-safe) will likely degrade performance + * significantly. + * + * @return random + */ + public static @NotNull Random current() { + return instance.get(); + } + + private static class SentryRandomThreadLocal extends ThreadLocal { + + @Override + protected Random initialValue() { + return new Random(); + } + } +} diff --git a/sentry/src/test/java/io/sentry/util/SentryRandomTest.kt b/sentry/src/test/java/io/sentry/util/SentryRandomTest.kt new file mode 100644 index 0000000000..c812c6cbdc --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/SentryRandomTest.kt @@ -0,0 +1,45 @@ +package io.sentry.util + +import kotlin.test.Test +import kotlin.test.assertNotSame +import kotlin.test.assertSame + +class SentryRandomTest { + + @Test + fun `thread local creates a new instance per thread but keeps re-using it for the same thread`() { + val mainThreadRandom1 = SentryRandom.current() + val mainThreadRandom2 = SentryRandom.current() + assertSame(mainThreadRandom1, mainThreadRandom2) + + var thread1Random1: Random? = null + var thread1Random2: Random? = null + + val thread1 = Thread() { + thread1Random1 = SentryRandom.current() + thread1Random2 = SentryRandom.current() + } + + var thread2Random1: Random? = null + var thread2Random2: Random? = null + + val thread2 = Thread() { + thread2Random1 = SentryRandom.current() + thread2Random2 = SentryRandom.current() + } + + thread1.start() + thread2.start() + thread1.join() + thread2.join() + + assertSame(thread1Random1, thread1Random2) + assertNotSame(mainThreadRandom1, thread1Random1) + + assertSame(thread2Random1, thread2Random2) + assertNotSame(mainThreadRandom1, thread2Random1) + + val mainThreadRandom3 = SentryRandom.current() + assertSame(mainThreadRandom1, mainThreadRandom3) + } +}