Skip to content

Commit d5d5aea

Browse files
runningcodeclaude
andcommitted
perf(core): Reuse shared executor for transaction timeouts (SDK-1347)
SentryTracer created a dedicated java.util.Timer (a new thread) per transaction that had an idle or deadline timeout. For apps with many transactions (screen loads, HTTP spans) this churned threads and added CPU overhead. Schedule the idle/deadline timeouts on the SDK's shared SentryExecutorService (a single ScheduledThreadPoolExecutor) instead, so no per-transaction thread is created. The executor is set with removeOnCancelPolicy(true) so timeouts that are cancelled early (the common case) are evicted from the queue immediately rather than lingering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2ebf90a commit d5d5aea

4 files changed

Lines changed: 77 additions & 68 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import io.sentry.Scopes
2323
import io.sentry.Sentry
2424
import io.sentry.SentryDate
2525
import io.sentry.SentryDateProvider
26+
import io.sentry.SentryExecutorService
2627
import io.sentry.SentryNanotimeDate
2728
import io.sentry.SentryTraceHeader
2829
import io.sentry.SentryTracer
@@ -652,6 +653,8 @@ class ActivityLifecycleIntegrationTest {
652653
it.tracesSampleRate = 1.0
653654
it.isEnableTimeToFullDisplayTracing = true
654655
it.idleTimeout = 100
656+
// idle/deadline timeouts run on the shared executor; use a real one so they fire
657+
it.executorService = SentryExecutorService(it)
655658
}
656659
)
657660
sut.register(fixture.scopes, fixture.options)
@@ -1883,6 +1886,10 @@ class ActivityLifecycleIntegrationTest {
18831886
fixture.options.tracesSampleRate = 1.0
18841887
fixture.options.isEnableTimeToFullDisplayTracing = true
18851888
fixture.options.executorService = deferredExecutorService
1889+
// Isolate the ttfd auto-close: without disabling the transaction's own idle/deadline timeouts
1890+
// (also scheduled on this executor now), runAll() would finish the transaction first.
1891+
fixture.options.idleTimeout = null
1892+
fixture.options.deadlineTimeout = 0
18861893
sut.register(fixture.scopes, fixture.options)
18871894
sut.onActivityCreated(activity, fixture.bundle)
18881895
sut.onActivityResumed(activity)

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,20 @@ public final class SentryExecutorService implements ISentryExecutorService {
4646
}
4747

4848
public SentryExecutorService(final @Nullable SentryOptions options) {
49-
this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), options);
49+
this(newScheduledExecutor(), options);
5050
}
5151

5252
public SentryExecutorService() {
53-
this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), null);
53+
this(newScheduledExecutor(), null);
54+
}
55+
56+
private static @NotNull ScheduledThreadPoolExecutor newScheduledExecutor() {
57+
final @NotNull ScheduledThreadPoolExecutor executor =
58+
new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory());
59+
// Cancelled scheduled tasks (e.g. transaction idle/deadline timeouts that finished early) are
60+
// removed from the work queue right away instead of lingering until their scheduled time.
61+
executor.setRemoveOnCancelPolicy(true);
62+
return executor;
5463
}
5564

5665
private boolean isQueueAvailable() {

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

Lines changed: 32 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@
1212
import java.util.List;
1313
import java.util.ListIterator;
1414
import java.util.Map;
15-
import java.util.Timer;
16-
import java.util.TimerTask;
1715
import java.util.concurrent.CopyOnWriteArrayList;
16+
import java.util.concurrent.Future;
1817
import java.util.concurrent.atomic.AtomicBoolean;
1918
import java.util.concurrent.atomic.AtomicReference;
2019
import org.jetbrains.annotations.ApiStatus;
@@ -37,10 +36,13 @@ public final class SentryTracer implements ITransaction {
3736
*/
3837
private @NotNull FinishStatus finishStatus = FinishStatus.NOT_FINISHED;
3938

40-
private volatile @Nullable TimerTask idleTimeoutTask;
41-
private volatile @Nullable TimerTask deadlineTimeoutTask;
39+
private volatile @Nullable Future<?> idleTimeoutFuture;
40+
private volatile @Nullable Future<?> deadlineTimeoutFuture;
4241

43-
private volatile @Nullable Timer timer = null;
42+
// Idle/deadline timeouts are scheduled on the shared executor service rather than a
43+
// per-transaction Timer thread. Set at construction when this transaction has timeouts, and
44+
// cleared on finish so no further tasks are scheduled.
45+
private volatile @Nullable ISentryExecutorService timeoutScheduler = null;
4446
private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock();
4547
private final @NotNull AutoClosableReentrantLock tracerLock = new AutoClosableReentrantLock();
4648

@@ -99,7 +101,7 @@ public SentryTracer(
99101

100102
if (transactionOptions.getIdleTimeout() != null
101103
|| transactionOptions.getDeadlineTimeout() != null) {
102-
timer = new Timer(true);
104+
timeoutScheduler = scopes.getOptions().getExecutorService();
103105

104106
scheduleDeadlineTimeout();
105107
scheduleFinish();
@@ -109,22 +111,16 @@ public SentryTracer(
109111
@Override
110112
public void scheduleFinish() {
111113
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
112-
if (timer != null) {
114+
final @Nullable ISentryExecutorService scheduler = timeoutScheduler;
115+
if (scheduler != null) {
113116
final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();
114117

115118
if (idleTimeout != null) {
116119
cancelIdleTimer();
117120
isIdleFinishTimerRunning.set(true);
118-
idleTimeoutTask =
119-
new TimerTask() {
120-
@Override
121-
public void run() {
122-
onIdleTimeoutReached();
123-
}
124-
};
125121

126122
try {
127-
timer.schedule(idleTimeoutTask, idleTimeout);
123+
idleTimeoutFuture = scheduler.schedule(() -> onIdleTimeoutReached(), idleTimeout);
128124
} catch (Throwable e) {
129125
scopes
130126
.getOptions()
@@ -265,13 +261,12 @@ public void finish(
265261
});
266262
final SentryTransaction transaction = new SentryTransaction(this);
267263

268-
if (timer != null) {
264+
if (timeoutScheduler != null) {
269265
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
270-
if (timer != null) {
266+
if (timeoutScheduler != null) {
271267
cancelIdleTimer();
272268
cancelDeadlineTimer();
273-
timer.cancel();
274-
timer = null;
269+
timeoutScheduler = null;
275270
}
276271
}
277272
}
@@ -295,10 +290,11 @@ public void finish(
295290

296291
private void cancelIdleTimer() {
297292
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
298-
if (idleTimeoutTask != null) {
299-
idleTimeoutTask.cancel();
293+
final @Nullable Future<?> future = idleTimeoutFuture;
294+
if (future != null) {
295+
future.cancel(false);
300296
isIdleFinishTimerRunning.set(false);
301-
idleTimeoutTask = null;
297+
idleTimeoutFuture = null;
302298
}
303299
}
304300
}
@@ -307,18 +303,13 @@ private void scheduleDeadlineTimeout() {
307303
final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();
308304
if (deadlineTimeOut != null) {
309305
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
310-
if (timer != null) {
306+
final @Nullable ISentryExecutorService scheduler = timeoutScheduler;
307+
if (scheduler != null) {
311308
cancelDeadlineTimer();
312309
isDeadlineTimerRunning.set(true);
313-
deadlineTimeoutTask =
314-
new TimerTask() {
315-
@Override
316-
public void run() {
317-
onDeadlineTimeoutReached();
318-
}
319-
};
320310
try {
321-
timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
311+
deadlineTimeoutFuture =
312+
scheduler.schedule(() -> onDeadlineTimeoutReached(), deadlineTimeOut);
322313
} catch (Throwable e) {
323314
scopes
324315
.getOptions()
@@ -335,10 +326,11 @@ public void run() {
335326

336327
private void cancelDeadlineTimer() {
337328
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
338-
if (deadlineTimeoutTask != null) {
339-
deadlineTimeoutTask.cancel();
329+
final @Nullable Future<?> future = deadlineTimeoutFuture;
330+
if (future != null) {
331+
future.cancel(false);
340332
isDeadlineTimerRunning.set(false);
341-
deadlineTimeoutTask = null;
333+
deadlineTimeoutFuture = null;
342334
}
343335
}
344336
}
@@ -973,20 +965,19 @@ Span getRoot() {
973965

974966
@TestOnly
975967
@Nullable
976-
TimerTask getIdleTimeoutTask() {
977-
return idleTimeoutTask;
968+
Future<?> getIdleTimeoutFuture() {
969+
return idleTimeoutFuture;
978970
}
979971

980972
@TestOnly
981973
@Nullable
982-
TimerTask getDeadlineTimeoutTask() {
983-
return deadlineTimeoutTask;
974+
Future<?> getDeadlineTimeoutFuture() {
975+
return deadlineTimeoutFuture;
984976
}
985977

986978
@TestOnly
987-
@Nullable
988-
Timer getTimer() {
989-
return timer;
979+
boolean isTimeoutSchedulerActive() {
980+
return timeoutScheduler != null;
990981
}
991982

992983
@TestOnly

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

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ class SentryTracerTest {
3737
options.dsn = "https://key@sentry.io/proj"
3838
options.environment = "environment"
3939
options.release = "release@3.0.0"
40+
// Transaction idle/deadline timeouts are scheduled on the shared executor service (in
41+
// production it is activated during Sentry.init); tests need a real one so timeouts fire.
42+
options.executorService = SentryExecutorService(options)
4043
scopes = spy(createTestScopes(options))
4144
compositePerformanceCollector = spy(DefaultCompositePerformanceCollector(options))
4245
}
@@ -913,15 +916,15 @@ class SentryTracerTest {
913916
@Test
914917
fun `when initialized without deadlineTimeout, does not schedule finish timer`() {
915918
val transaction = fixture.getSut()
916-
assertNull(transaction.deadlineTimeoutTask)
919+
assertNull(transaction.deadlineTimeoutFuture)
917920
}
918921

919922
@Test
920923
fun `when initialized with deadlineTimeout, schedules finish timer`() {
921924
val transaction = fixture.getSut(deadlineTimeout = 50)
922925

923926
assertTrue(transaction.isDeadlineTimerRunning.get())
924-
assertNotNull(transaction.deadlineTimeoutTask)
927+
assertNotNull(transaction.deadlineTimeoutFuture)
925928
}
926929

927930
@Test
@@ -949,7 +952,7 @@ class SentryTracerTest {
949952
transaction.finish(SpanStatus.OK)
950953

951954
assertEquals(transaction.isDeadlineTimerRunning.get(), false)
952-
assertNull(transaction.deadlineTimeoutTask)
955+
assertNull(transaction.deadlineTimeoutFuture)
953956
assertEquals(transaction.isFinished, true)
954957
assertEquals(SpanStatus.OK, transaction.status)
955958
assertEquals(SpanStatus.OK, span.status)
@@ -958,26 +961,26 @@ class SentryTracerTest {
958961
@Test
959962
fun `when initialized with idleTimeout it has no influence on deadline timeout`() {
960963
val transaction = fixture.getSut(idleTimeout = 3000, deadlineTimeout = 20)
961-
val deadlineTimeoutTask = transaction.deadlineTimeoutTask
964+
val deadlineTimeoutFuture = transaction.deadlineTimeoutFuture
962965

963966
val span = transaction.startChild("op")
964967
// when the span finishes, it re-schedules the idle task
965968
span.finish()
966969

967970
// but the deadline timeout task should not be re-scheduled
968-
assertEquals(deadlineTimeoutTask, transaction.deadlineTimeoutTask)
971+
assertEquals(deadlineTimeoutFuture, transaction.deadlineTimeoutFuture)
969972
}
970973

971974
@Test
972975
fun `when initialized without idleTimeout, does not schedule finish timer`() {
973976
val transaction = fixture.getSut()
974-
assertNull(transaction.idleTimeoutTask)
977+
assertNull(transaction.idleTimeoutFuture)
975978
}
976979

977980
@Test
978981
fun `when initialized with idleTimeout, schedules finish timer`() {
979982
val transaction = fixture.getSut(idleTimeout = 50)
980-
assertNotNull(transaction.idleTimeoutTask)
983+
assertNotNull(transaction.idleTimeoutFuture)
981984
}
982985

983986
@Test
@@ -1008,22 +1011,21 @@ class SentryTracerTest {
10081011

10091012
transaction.startChild("op")
10101013

1011-
assertNull(transaction.idleTimeoutTask)
1014+
assertNull(transaction.idleTimeoutFuture)
10121015
}
10131016

10141017
@Test
10151018
fun `when a child is finished and the transaction is idle, resets the timer`() {
10161019
val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 3000)
10171020

1018-
val initialTime = transaction.idleTimeoutTask!!.scheduledExecutionTime()
1021+
val initialFuture = assertNotNull(transaction.idleTimeoutFuture)
10191022

10201023
val span = transaction.startChild("op")
1021-
Thread.sleep(1)
10221024
span.finish()
10231025

1024-
val timerAfterFinishingChild = transaction.idleTimeoutTask!!.scheduledExecutionTime()
1025-
1026-
assertTrue { timerAfterFinishingChild > initialTime }
1026+
// finishing the child cancels and re-schedules the idle timeout, yielding a new future
1027+
val rescheduledFuture = assertNotNull(transaction.idleTimeoutFuture)
1028+
assertNotEquals(initialFuture, rescheduledFuture)
10271029
}
10281030

10291031
@Test
@@ -1035,7 +1037,7 @@ class SentryTracerTest {
10351037
Thread.sleep(1)
10361038
span.finish()
10371039

1038-
assertNull(transaction.idleTimeoutTask)
1040+
assertNull(transaction.idleTimeoutFuture)
10391041
}
10401042

10411043
@Test
@@ -1072,41 +1074,41 @@ class SentryTracerTest {
10721074
}
10731075

10741076
@Test
1075-
fun `timer is created if idle timeout is set`() {
1077+
fun `timeout scheduling is active if idle timeout is set`() {
10761078
val transaction =
10771079
fixture.getSut(
10781080
waitForChildren = true,
10791081
idleTimeout = 50,
10801082
trimEnd = true,
10811083
samplingDecision = TracesSamplingDecision(true),
10821084
)
1083-
assertNotNull(transaction.timer)
1085+
assertTrue(transaction.isTimeoutSchedulerActive)
10841086
}
10851087

10861088
@Test
1087-
fun `timer is not created if idle timeout is not set`() {
1089+
fun `timeout scheduling is inactive if idle timeout is not set`() {
10881090
val transaction =
10891091
fixture.getSut(
10901092
waitForChildren = true,
10911093
idleTimeout = null,
10921094
trimEnd = true,
10931095
samplingDecision = TracesSamplingDecision(true),
10941096
)
1095-
assertNull(transaction.timer)
1097+
assertFalse(transaction.isTimeoutSchedulerActive)
10961098
}
10971099

10981100
@Test
1099-
fun `timer is cancelled on finish`() {
1101+
fun `timeout scheduling is stopped on finish`() {
11001102
val transaction =
11011103
fixture.getSut(
11021104
waitForChildren = true,
11031105
idleTimeout = 50,
11041106
trimEnd = true,
11051107
samplingDecision = TracesSamplingDecision(true),
11061108
)
1107-
assertNotNull(transaction.timer)
1109+
assertTrue(transaction.isTimeoutSchedulerActive)
11081110
transaction.finish(SpanStatus.OK)
1109-
assertNull(transaction.timer)
1111+
assertFalse(transaction.isTimeoutSchedulerActive)
11101112
}
11111113

11121114
@Test
@@ -1539,18 +1541,18 @@ class SentryTracerTest {
15391541
}
15401542

15411543
@Test
1542-
fun `when timer is cancelled, schedule finish does not crash`() {
1544+
fun `when executor is closed, schedule finish does not crash`() {
15431545
val tracer = fixture.getSut(idleTimeout = 50, deadlineTimeout = 100)
1544-
tracer.timer!!.cancel()
1546+
fixture.options.executorService.close(0)
15451547
tracer.scheduleFinish()
15461548
}
15471549

15481550
@Test
1549-
fun `when timer is cancelled, schedule finish finishes the transaction immediately`() {
1551+
fun `when executor is closed, schedule finish finishes the transaction immediately`() {
15501552
val tracer = fixture.getSut(idleTimeout = 50)
15511553
tracer.startChild("load").finish()
15521554

1553-
tracer.timer!!.cancel()
1555+
fixture.options.executorService.close(0)
15541556
tracer.scheduleFinish()
15551557

15561558
assertTrue(tracer.isFinished)

0 commit comments

Comments
 (0)