Skip to content

Commit 85a3a57

Browse files
runningcodeclaude
andcommitted
perf: Avoid per-transaction Timer thread in SentryTracer (JAVA-570)
Transactions with an idle or deadline timeout each created a java.util.Timer, which spawns a thread synchronously on the calling thread (often the main thread on Android). At scale (screen loads, HTTP spans) this was the dominant source of SDK thread churn. Schedule the idle/deadline timeouts on the shared SentryExecutorService instead, so no thread is created per transaction. On finish only the scheduled futures are cancelled; the shared executor is never shut down here since it is used SDK-wide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 58b65f0 commit 85a3a57

2 files changed

Lines changed: 52 additions & 61 deletions

File tree

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

Lines changed: 29 additions & 40 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+
// Shared executor used to schedule the timeout tasks. Null once the tracer is finished, at which
43+
// point no more timeouts may be scheduled. It is never shut down here since it is shared
44+
// SDK-wide.
45+
private volatile @Nullable ISentryExecutorService timerExecutorService = 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+
timerExecutorService = 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+
if (timerExecutorService != null) {
113115
final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();
114116

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

126121
try {
127-
timer.schedule(idleTimeoutTask, idleTimeout);
122+
idleTimeoutFuture =
123+
timerExecutorService.schedule(this::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 (timerExecutorService != null) {
269265
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
270-
if (timer != null) {
266+
if (timerExecutorService != null) {
271267
cancelIdleTimer();
272268
cancelDeadlineTimer();
273-
timer.cancel();
274-
timer = null;
269+
timerExecutorService = null;
275270
}
276271
}
277272
}
@@ -295,10 +290,10 @@ public void finish(
295290

296291
private void cancelIdleTimer() {
297292
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
298-
if (idleTimeoutTask != null) {
299-
idleTimeoutTask.cancel();
293+
if (idleTimeoutFuture != null) {
294+
idleTimeoutFuture.cancel(false);
300295
isIdleFinishTimerRunning.set(false);
301-
idleTimeoutTask = null;
296+
idleTimeoutFuture = null;
302297
}
303298
}
304299
}
@@ -307,18 +302,12 @@ private void scheduleDeadlineTimeout() {
307302
final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();
308303
if (deadlineTimeOut != null) {
309304
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
310-
if (timer != null) {
305+
if (timerExecutorService != null) {
311306
cancelDeadlineTimer();
312307
isDeadlineTimerRunning.set(true);
313-
deadlineTimeoutTask =
314-
new TimerTask() {
315-
@Override
316-
public void run() {
317-
onDeadlineTimeoutReached();
318-
}
319-
};
320308
try {
321-
timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
309+
deadlineTimeoutFuture =
310+
timerExecutorService.schedule(this::onDeadlineTimeoutReached, deadlineTimeOut);
322311
} catch (Throwable e) {
323312
scopes
324313
.getOptions()
@@ -335,10 +324,10 @@ public void run() {
335324

336325
private void cancelDeadlineTimer() {
337326
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
338-
if (deadlineTimeoutTask != null) {
339-
deadlineTimeoutTask.cancel();
327+
if (deadlineTimeoutFuture != null) {
328+
deadlineTimeoutFuture.cancel(false);
340329
isDeadlineTimerRunning.set(false);
341-
deadlineTimeoutTask = null;
330+
deadlineTimeoutFuture = null;
342331
}
343332
}
344333
}
@@ -973,20 +962,20 @@ Span getRoot() {
973962

974963
@TestOnly
975964
@Nullable
976-
TimerTask getIdleTimeoutTask() {
977-
return idleTimeoutTask;
965+
Future<?> getIdleTimeoutFuture() {
966+
return idleTimeoutFuture;
978967
}
979968

980969
@TestOnly
981970
@Nullable
982-
TimerTask getDeadlineTimeoutTask() {
983-
return deadlineTimeoutTask;
971+
Future<?> getDeadlineTimeoutFuture() {
972+
return deadlineTimeoutFuture;
984973
}
985974

986975
@TestOnly
987976
@Nullable
988-
Timer getTimer() {
989-
return timer;
977+
ISentryExecutorService getTimerExecutorService() {
978+
return timerExecutorService;
990979
}
991980

992981
@TestOnly

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

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import kotlin.test.assertEquals
1313
import kotlin.test.assertFalse
1414
import kotlin.test.assertNotEquals
1515
import kotlin.test.assertNotNull
16+
import kotlin.test.assertNotSame
1617
import kotlin.test.assertNull
1718
import kotlin.test.assertSame
1819
import kotlin.test.assertTrue
@@ -913,15 +914,15 @@ class SentryTracerTest {
913914
@Test
914915
fun `when initialized without deadlineTimeout, does not schedule finish timer`() {
915916
val transaction = fixture.getSut()
916-
assertNull(transaction.deadlineTimeoutTask)
917+
assertNull(transaction.deadlineTimeoutFuture)
917918
}
918919

919920
@Test
920921
fun `when initialized with deadlineTimeout, schedules finish timer`() {
921922
val transaction = fixture.getSut(deadlineTimeout = 50)
922923

923924
assertTrue(transaction.isDeadlineTimerRunning.get())
924-
assertNotNull(transaction.deadlineTimeoutTask)
925+
assertNotNull(transaction.deadlineTimeoutFuture)
925926
}
926927

927928
@Test
@@ -949,7 +950,7 @@ class SentryTracerTest {
949950
transaction.finish(SpanStatus.OK)
950951

951952
assertEquals(transaction.isDeadlineTimerRunning.get(), false)
952-
assertNull(transaction.deadlineTimeoutTask)
953+
assertNull(transaction.deadlineTimeoutFuture)
953954
assertEquals(transaction.isFinished, true)
954955
assertEquals(SpanStatus.OK, transaction.status)
955956
assertEquals(SpanStatus.OK, span.status)
@@ -958,26 +959,26 @@ class SentryTracerTest {
958959
@Test
959960
fun `when initialized with idleTimeout it has no influence on deadline timeout`() {
960961
val transaction = fixture.getSut(idleTimeout = 3000, deadlineTimeout = 20)
961-
val deadlineTimeoutTask = transaction.deadlineTimeoutTask
962+
val deadlineTimeoutFuture = transaction.deadlineTimeoutFuture
962963

963964
val span = transaction.startChild("op")
964965
// when the span finishes, it re-schedules the idle task
965966
span.finish()
966967

967968
// but the deadline timeout task should not be re-scheduled
968-
assertEquals(deadlineTimeoutTask, transaction.deadlineTimeoutTask)
969+
assertSame(deadlineTimeoutFuture, transaction.deadlineTimeoutFuture)
969970
}
970971

971972
@Test
972973
fun `when initialized without idleTimeout, does not schedule finish timer`() {
973974
val transaction = fixture.getSut()
974-
assertNull(transaction.idleTimeoutTask)
975+
assertNull(transaction.idleTimeoutFuture)
975976
}
976977

977978
@Test
978979
fun `when initialized with idleTimeout, schedules finish timer`() {
979980
val transaction = fixture.getSut(idleTimeout = 50)
980-
assertNotNull(transaction.idleTimeoutTask)
981+
assertNotNull(transaction.idleTimeoutFuture)
981982
}
982983

983984
@Test
@@ -1008,22 +1009,23 @@ class SentryTracerTest {
10081009

10091010
transaction.startChild("op")
10101011

1011-
assertNull(transaction.idleTimeoutTask)
1012+
assertNull(transaction.idleTimeoutFuture)
10121013
}
10131014

10141015
@Test
10151016
fun `when a child is finished and the transaction is idle, resets the timer`() {
10161017
val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 3000)
10171018

1018-
val initialTime = transaction.idleTimeoutTask!!.scheduledExecutionTime()
1019+
val initialFuture = transaction.idleTimeoutFuture
10191020

10201021
val span = transaction.startChild("op")
1021-
Thread.sleep(1)
10221022
span.finish()
10231023

1024-
val timerAfterFinishingChild = transaction.idleTimeoutTask!!.scheduledExecutionTime()
1024+
// finishing the child re-schedules the idle timeout, replacing the pending future
1025+
val futureAfterFinishingChild = transaction.idleTimeoutFuture
10251026

1026-
assertTrue { timerAfterFinishingChild > initialTime }
1027+
assertNotNull(futureAfterFinishingChild)
1028+
assertNotSame(initialFuture, futureAfterFinishingChild)
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
@@ -1080,7 +1082,7 @@ class SentryTracerTest {
10801082
trimEnd = true,
10811083
samplingDecision = TracesSamplingDecision(true),
10821084
)
1083-
assertNotNull(transaction.timer)
1085+
assertNotNull(transaction.timerExecutorService)
10841086
}
10851087

10861088
@Test
@@ -1092,7 +1094,7 @@ class SentryTracerTest {
10921094
trimEnd = true,
10931095
samplingDecision = TracesSamplingDecision(true),
10941096
)
1095-
assertNull(transaction.timer)
1097+
assertNull(transaction.timerExecutorService)
10961098
}
10971099

10981100
@Test
@@ -1104,9 +1106,9 @@ class SentryTracerTest {
11041106
trimEnd = true,
11051107
samplingDecision = TracesSamplingDecision(true),
11061108
)
1107-
assertNotNull(transaction.timer)
1109+
assertNotNull(transaction.timerExecutorService)
11081110
transaction.finish(SpanStatus.OK)
1109-
assertNull(transaction.timer)
1111+
assertNull(transaction.timerExecutorService)
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 timer executor is shut down, 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 timer executor is shut down, 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)