perf(core): Reuse shared executor for transaction timeouts (SDK-1347)

CHANGELOG.md

@@ -13,6 +13,7 @@
 
 ### Performance
 
+- Avoid creating a per-transaction `Timer` thread by scheduling transaction idle/deadline timeouts on the shared executor service ([#5646](https://github.com/getsentry/sentry-java/pull/5646))
 - Reduce writer buffer size from 8192 to 512 ([#5544](https://github.com/getsentry/sentry-java/pull/5544))
 - Remove redundant event map copies ([#5536](https://github.com/getsentry/sentry-java/pull/5536))
 - Optimize combined scope by adding an early return if only one scope has data ([#5541](https://github.com/getsentry/sentry-java/pull/5541))

sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt

@@ -23,6 +23,7 @@ import io.sentry.Scopes
 import io.sentry.Sentry
 import io.sentry.SentryDate
 import io.sentry.SentryDateProvider
+import io.sentry.SentryExecutorService
 import io.sentry.SentryNanotimeDate
 import io.sentry.SentryTraceHeader
 import io.sentry.SentryTracer
@@ -652,6 +653,8 @@ class ActivityLifecycleIntegrationTest {
           it.tracesSampleRate = 1.0
           it.isEnableTimeToFullDisplayTracing = true
           it.idleTimeout = 100
+          // idle/deadline timeouts run on the shared executor; use a real one so they fire
+          it.executorService = SentryExecutorService(it)
         }
       )
     sut.register(fixture.scopes, fixture.options)
@@ -1883,6 +1886,10 @@ class ActivityLifecycleIntegrationTest {
     fixture.options.tracesSampleRate = 1.0
     fixture.options.isEnableTimeToFullDisplayTracing = true
     fixture.options.executorService = deferredExecutorService
+    // Isolate the ttfd auto-close: without disabling the transaction's own idle/deadline timeouts
+    // (also scheduled on this executor now), runAll() would finish the transaction first.
+    fixture.options.idleTimeout = null
+    fixture.options.deadlineTimeout = 0
     sut.register(fixture.scopes, fixture.options)
     sut.onActivityCreated(activity, fixture.bundle)
     sut.onActivityResumed(activity)

sentry/src/main/java/io/sentry/SentryExecutorService.java

@@ -46,11 +46,20 @@ public final class SentryExecutorService implements ISentryExecutorService {
   }
 
   public SentryExecutorService(final @Nullable SentryOptions options) {
-    this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), options);
+    this(newScheduledExecutor(), options);
   }
 
   public SentryExecutorService() {
-    this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), null);
+    this(newScheduledExecutor(), null);
+  }
+
+  private static @NotNull ScheduledThreadPoolExecutor newScheduledExecutor() {
+    final @NotNull ScheduledThreadPoolExecutor executor =
+        new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory());
+    // Cancelled scheduled tasks (e.g. transaction idle/deadline timeouts that finished early) are
+    // removed from the work queue right away instead of lingering until their scheduled time.
+    executor.setRemoveOnCancelPolicy(true);
+    return executor;
   }
 
   private boolean isQueueAvailable() {

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -12,9 +12,8 @@
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import org.jetbrains.annotations.ApiStatus;
@@ -37,10 +36,13 @@ public final class SentryTracer implements ITransaction {
    */
   private @NotNull FinishStatus finishStatus = FinishStatus.NOT_FINISHED;
 
-  private volatile @Nullable TimerTask idleTimeoutTask;
-  private volatile @Nullable TimerTask deadlineTimeoutTask;
+  private volatile @Nullable Future<?> idleTimeoutFuture;
+  private volatile @Nullable Future<?> deadlineTimeoutFuture;
 
-  private volatile @Nullable Timer timer = null;
+  // Idle/deadline timeouts are scheduled on the shared executor service rather than a
+  // per-transaction Timer thread. Set at construction when this transaction has timeouts, and
+  // cleared on finish so no further tasks are scheduled.
+  private volatile @Nullable ISentryExecutorService timeoutScheduler = null;
   private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock();
   private final @NotNull AutoClosableReentrantLock tracerLock = new AutoClosableReentrantLock();
 
@@ -99,7 +101,7 @@ public SentryTracer(
 
     if (transactionOptions.getIdleTimeout() != null
         || transactionOptions.getDeadlineTimeout() != null) {
-      timer = new Timer(true);
+      timeoutScheduler = scopes.getOptions().getExecutorService();
 
       scheduleDeadlineTimeout();
       scheduleFinish();
@@ -109,22 +111,16 @@ public SentryTracer(
   @Override
   public void scheduleFinish() {
     try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
-      if (timer != null) {
+      final @Nullable ISentryExecutorService scheduler = timeoutScheduler;
+      if (scheduler != null) {
         final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();
 
         if (idleTimeout != null) {
           cancelIdleTimer();
           isIdleFinishTimerRunning.set(true);
-          idleTimeoutTask =
-              new TimerTask() {
-                @Override
-                public void run() {
-                  onIdleTimeoutReached();
-                }
-              };
 
           try {
-            timer.schedule(idleTimeoutTask, idleTimeout);
+            idleTimeoutFuture = scheduler.schedule(() -> onIdleTimeoutReached(), idleTimeout);
           } catch (Throwable e) {
             scopes
                 .getOptions()
@@ -265,13 +261,12 @@ public void finish(
           });
       final SentryTransaction transaction = new SentryTransaction(this);
 
-      if (timer != null) {
+      if (timeoutScheduler != null) {
         try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
-          if (timer != null) {
+          if (timeoutScheduler != null) {
             cancelIdleTimer();
             cancelDeadlineTimer();
-            timer.cancel();
-            timer = null;
+            timeoutScheduler = null;
           }
         }
       }
@@ -295,10 +290,11 @@ public void finish(
 
   private void cancelIdleTimer() {
     try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
-      if (idleTimeoutTask != null) {
-        idleTimeoutTask.cancel();
+      final @Nullable Future<?> future = idleTimeoutFuture;
+      if (future != null) {
+        future.cancel(false);
         isIdleFinishTimerRunning.set(false);
-        idleTimeoutTask = null;
+        idleTimeoutFuture = null;
       }
     }
   }
@@ -307,18 +303,13 @@ private void scheduleDeadlineTimeout() {
     final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();
     if (deadlineTimeOut != null) {
       try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
-        if (timer != null) {
+        final @Nullable ISentryExecutorService scheduler = timeoutScheduler;
+        if (scheduler != null) {
           cancelDeadlineTimer();
           isDeadlineTimerRunning.set(true);
-          deadlineTimeoutTask =
-              new TimerTask() {
-                @Override
-                public void run() {
-                  onDeadlineTimeoutReached();
-                }
-              };
           try {
-            timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
+            deadlineTimeoutFuture =
+                scheduler.schedule(() -> onDeadlineTimeoutReached(), deadlineTimeOut);
           } catch (Throwable e) {
             scopes
                 .getOptions()
@@ -335,10 +326,11 @@ public void run() {
 
   private void cancelDeadlineTimer() {
     try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
-      if (deadlineTimeoutTask != null) {
-        deadlineTimeoutTask.cancel();
+      final @Nullable Future<?> future = deadlineTimeoutFuture;
+      if (future != null) {
+        future.cancel(false);
         isDeadlineTimerRunning.set(false);
-        deadlineTimeoutTask = null;
+        deadlineTimeoutFuture = null;
       }
     }
   }
@@ -973,20 +965,19 @@ Span getRoot() {
 
   @TestOnly
   @Nullable
-  TimerTask getIdleTimeoutTask() {
-    return idleTimeoutTask;
+  Future<?> getIdleTimeoutFuture() {
+    return idleTimeoutFuture;
   }
 
   @TestOnly
   @Nullable
-  TimerTask getDeadlineTimeoutTask() {
-    return deadlineTimeoutTask;
+  Future<?> getDeadlineTimeoutFuture() {
+    return deadlineTimeoutFuture;
   }
 
   @TestOnly
-  @Nullable
-  Timer getTimer() {
-    return timer;
+  boolean isTimeoutSchedulerActive() {
+    return timeoutScheduler != null;
   }
 
   @TestOnly

sentry/src/test/java/io/sentry/SentryTracerTest.kt

@@ -37,6 +37,9 @@ class SentryTracerTest {
       options.dsn = "https://key@sentry.io/proj"
       options.environment = "environment"
       options.release = "release@3.0.0"
+      // Transaction idle/deadline timeouts are scheduled on the shared executor service (in
+      // production it is activated during Sentry.init); tests need a real one so timeouts fire.
+      options.executorService = SentryExecutorService(options)
       scopes = spy(createTestScopes(options))
       compositePerformanceCollector = spy(DefaultCompositePerformanceCollector(options))
     }
@@ -913,15 +916,15 @@ class SentryTracerTest {
   @Test
   fun `when initialized without deadlineTimeout, does not schedule finish timer`() {
     val transaction = fixture.getSut()
-    assertNull(transaction.deadlineTimeoutTask)
+    assertNull(transaction.deadlineTimeoutFuture)
   }
 
   @Test
   fun `when initialized with deadlineTimeout, schedules finish timer`() {
     val transaction = fixture.getSut(deadlineTimeout = 50)
 
     assertTrue(transaction.isDeadlineTimerRunning.get())
-    assertNotNull(transaction.deadlineTimeoutTask)
+    assertNotNull(transaction.deadlineTimeoutFuture)
   }
 
   @Test
@@ -949,7 +952,7 @@ class SentryTracerTest {
     transaction.finish(SpanStatus.OK)
 
     assertEquals(transaction.isDeadlineTimerRunning.get(), false)
-    assertNull(transaction.deadlineTimeoutTask)
+    assertNull(transaction.deadlineTimeoutFuture)
     assertEquals(transaction.isFinished, true)
     assertEquals(SpanStatus.OK, transaction.status)
     assertEquals(SpanStatus.OK, span.status)
@@ -958,26 +961,26 @@ class SentryTracerTest {
   @Test
   fun `when initialized with idleTimeout it has no influence on deadline timeout`() {
     val transaction = fixture.getSut(idleTimeout = 3000, deadlineTimeout = 20)
-    val deadlineTimeoutTask = transaction.deadlineTimeoutTask
+    val deadlineTimeoutFuture = transaction.deadlineTimeoutFuture
 
     val span = transaction.startChild("op")
     // when the span finishes, it re-schedules the idle task
     span.finish()
 
     // but the deadline timeout task should not be re-scheduled
-    assertEquals(deadlineTimeoutTask, transaction.deadlineTimeoutTask)
+    assertEquals(deadlineTimeoutFuture, transaction.deadlineTimeoutFuture)
   }
 
   @Test
   fun `when initialized without idleTimeout, does not schedule finish timer`() {
     val transaction = fixture.getSut()
-    assertNull(transaction.idleTimeoutTask)
+    assertNull(transaction.idleTimeoutFuture)
   }
 
   @Test
   fun `when initialized with idleTimeout, schedules finish timer`() {
     val transaction = fixture.getSut(idleTimeout = 50)
-    assertNotNull(transaction.idleTimeoutTask)
+    assertNotNull(transaction.idleTimeoutFuture)
   }
 
   @Test
@@ -1008,22 +1011,21 @@ class SentryTracerTest {
 
     transaction.startChild("op")
 
-    assertNull(transaction.idleTimeoutTask)
+    assertNull(transaction.idleTimeoutFuture)
   }
 
   @Test
   fun `when a child is finished and the transaction is idle, resets the timer`() {
     val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 3000)
 
-    val initialTime = transaction.idleTimeoutTask!!.scheduledExecutionTime()
+    val initialFuture = assertNotNull(transaction.idleTimeoutFuture)
 
     val span = transaction.startChild("op")
-    Thread.sleep(1)
     span.finish()
 
-    val timerAfterFinishingChild = transaction.idleTimeoutTask!!.scheduledExecutionTime()
-
-    assertTrue { timerAfterFinishingChild > initialTime }
+    // finishing the child cancels and re-schedules the idle timeout, yielding a new future
+    val rescheduledFuture = assertNotNull(transaction.idleTimeoutFuture)
+    assertNotEquals(initialFuture, rescheduledFuture)
   }
 
   @Test
@@ -1035,7 +1037,7 @@ class SentryTracerTest {
     Thread.sleep(1)
     span.finish()
 
-    assertNull(transaction.idleTimeoutTask)
+    assertNull(transaction.idleTimeoutFuture)
   }
 
   @Test
@@ -1072,41 +1074,41 @@ class SentryTracerTest {
   }
 
   @Test
-  fun `timer is created if idle timeout is set`() {
+  fun `timeout scheduling is active if idle timeout is set`() {
     val transaction =
       fixture.getSut(
         waitForChildren = true,
         idleTimeout = 50,
         trimEnd = true,
         samplingDecision = TracesSamplingDecision(true),
       )
-    assertNotNull(transaction.timer)
+    assertTrue(transaction.isTimeoutSchedulerActive)
   }
 
   @Test
-  fun `timer is not created if idle timeout is not set`() {
+  fun `timeout scheduling is inactive if idle timeout is not set`() {
     val transaction =
       fixture.getSut(
         waitForChildren = true,
         idleTimeout = null,
         trimEnd = true,
         samplingDecision = TracesSamplingDecision(true),
       )
-    assertNull(transaction.timer)
+    assertFalse(transaction.isTimeoutSchedulerActive)
   }
 
   @Test
-  fun `timer is cancelled on finish`() {
+  fun `timeout scheduling is stopped on finish`() {
     val transaction =
       fixture.getSut(
         waitForChildren = true,
         idleTimeout = 50,
         trimEnd = true,
         samplingDecision = TracesSamplingDecision(true),
       )
-    assertNotNull(transaction.timer)
+    assertTrue(transaction.isTimeoutSchedulerActive)
     transaction.finish(SpanStatus.OK)
-    assertNull(transaction.timer)
+    assertFalse(transaction.isTimeoutSchedulerActive)
   }
 
   @Test
@@ -1539,18 +1541,18 @@ class SentryTracerTest {
   }
 
   @Test
-  fun `when timer is cancelled, schedule finish does not crash`() {
+  fun `when executor is closed, schedule finish does not crash`() {
     val tracer = fixture.getSut(idleTimeout = 50, deadlineTimeout = 100)
-    tracer.timer!!.cancel()
+    fixture.options.executorService.close(0)
     tracer.scheduleFinish()
   }
 
   @Test
-  fun `when timer is cancelled, schedule finish finishes the transaction immediately`() {
+  fun `when executor is closed, schedule finish finishes the transaction immediately`() {
     val tracer = fixture.getSut(idleTimeout = 50)
     tracer.startChild("load").finish()
 
-    tracer.timer!!.cancel()
+    fixture.options.executorService.close(0)
     tracer.scheduleFinish()
 
     assertTrue(tracer.isFinished)