Use Random through ThreadLocal<Random> (#3835)

* Use Random as a ThreadLocal<>

* changelog

* code review changes

Author: adinauer

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))

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(

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 <init> ()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;

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();

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;
   }
 }

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.
+ *
+ * <p>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.
+   *
+   * <p>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<Random> {
+
+    @Override
+    protected Random initialValue() {
+      return new Random();
+    }
+  }
+}

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)
+    }
+}