Skip to content

Commit bead45a

Browse files
runningcodeclaude
andcommitted
perf: Replace Timer threads with shared timer executor (JAVA-653)
RateLimiter and LifecycleWatcher each spun up a dedicated java.util.Timer thread that stayed alive forever once created. Schedule their tasks on the shared timer executor instead, whose single worker thread is reused across all timeouts and self-terminates when idle. This reduces the SDK's steady-state thread count and memory footprint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ae3e4c7 commit bead45a

4 files changed

Lines changed: 82 additions & 74 deletions

File tree

sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
import io.sentry.transport.CurrentDateProvider;
99
import io.sentry.transport.ICurrentDateProvider;
1010
import io.sentry.util.AutoClosableReentrantLock;
11-
import io.sentry.util.LazyEvaluator;
12-
import java.util.Timer;
13-
import java.util.TimerTask;
11+
import java.util.concurrent.Future;
1412
import java.util.concurrent.atomic.AtomicLong;
1513
import org.jetbrains.annotations.NotNull;
1614
import org.jetbrains.annotations.Nullable;
@@ -22,9 +20,8 @@ final class LifecycleWatcher implements AppState.AppStateListener {
2220

2321
private final long sessionIntervalMillis;
2422

25-
private @Nullable TimerTask timerTask;
26-
private final @NotNull LazyEvaluator<Timer> timer = new LazyEvaluator<>(() -> new Timer(true));
27-
private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock();
23+
private @Nullable Future<?> endSessionFuture;
24+
private final @NotNull AutoClosableReentrantLock endSessionLock = new AutoClosableReentrantLock();
2825
private final @NotNull IScopes scopes;
2926
private final boolean enableSessionTracking;
3027
private final boolean enableAppLifecycleBreadcrumbs;
@@ -104,29 +101,40 @@ public void onBackground() {
104101
}
105102

106103
private void scheduleEndSession() {
107-
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
104+
try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
108105
cancelTask();
109-
timerTask =
110-
new TimerTask() {
111-
@Override
112-
public void run() {
113-
if (enableSessionTracking) {
114-
scopes.endSession();
115-
}
116-
scopes.getOptions().getReplayController().stop();
117-
scopes.getOptions().getContinuousProfiler().close(false);
106+
final @NotNull Runnable endSession =
107+
() -> {
108+
if (enableSessionTracking) {
109+
scopes.endSession();
118110
}
111+
scopes.getOptions().getReplayController().stop();
112+
scopes.getOptions().getContinuousProfiler().close(false);
119113
};
120114

121-
timer.getValue().schedule(timerTask, sessionIntervalMillis);
115+
try {
116+
endSessionFuture =
117+
scopes
118+
.getOptions()
119+
.getTimerExecutorService()
120+
.schedule(endSession, sessionIntervalMillis);
121+
} catch (Throwable e) {
122+
scopes
123+
.getOptions()
124+
.getLogger()
125+
.log(SentryLevel.WARNING, "Failed to schedule end of session. Ending it now.", e);
126+
// if we cannot re-check after the session interval, end the session right away instead of
127+
// leaving it open forever
128+
endSession.run();
129+
}
122130
}
123131
}
124132

125133
private void cancelTask() {
126-
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
127-
if (timerTask != null) {
128-
timerTask.cancel();
129-
timerTask = null;
134+
try (final @NotNull ISentryLifecycleToken ignored = endSessionLock.acquire()) {
135+
if (endSessionFuture != null) {
136+
endSessionFuture.cancel(false);
137+
endSessionFuture = null;
130138
}
131139
}
132140
}
@@ -144,13 +152,7 @@ private void addAppBreadcrumb(final @NotNull String state) {
144152

145153
@TestOnly
146154
@Nullable
147-
TimerTask getTimerTask() {
148-
return timerTask;
149-
}
150-
151-
@TestOnly
152-
@NotNull
153-
Timer getTimer() {
154-
return timer.getValue();
155+
Future<?> getEndSessionFuture() {
156+
return endSessionFuture;
155157
}
156158
}

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import io.sentry.IScope
77
import io.sentry.IScopes
88
import io.sentry.ReplayController
99
import io.sentry.ScopeCallback
10+
import io.sentry.SentryExecutorService
1011
import io.sentry.SentryLevel
1112
import io.sentry.SentryOptions
1213
import io.sentry.Session
@@ -32,7 +33,8 @@ class LifecycleWatcherTest {
3233
private class Fixture {
3334
val scopes = mock<IScopes>()
3435
val dateProvider = mock<ICurrentDateProvider>()
35-
val options = SentryOptions()
36+
// a real executor so scheduled end-session tasks actually run
37+
val options = SentryOptions().apply { setTimerExecutorService(SentryExecutorService(this)) }
3638
val replayController = mock<ReplayController>()
3739
val continuousProfiler = mock<IContinuousProfiler>()
3840

@@ -115,10 +117,10 @@ class LifecycleWatcherTest {
115117
watcher.onForeground()
116118

117119
watcher.onBackground()
118-
assertNotNull(watcher.timerTask)
120+
assertNotNull(watcher.endSessionFuture)
119121

120122
watcher.onForeground()
121-
assertNull(watcher.timerTask)
123+
assertNull(watcher.endSessionFuture)
122124

123125
verify(fixture.scopes, never()).endSession()
124126
verify(fixture.replayController, never()).stop()
@@ -186,13 +188,6 @@ class LifecycleWatcherTest {
186188
verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>())
187189
}
188190

189-
@Test
190-
fun `timer is created if session tracking is enabled`() {
191-
val watcher =
192-
fixture.getSUT(enableAutoSessionTracking = true, enableAppLifecycleBreadcrumbs = false)
193-
assertNotNull(watcher.timer)
194-
}
195-
196191
@Test
197192
fun `if the scopes has already a fresh session running, don't start new one`() {
198193
val watcher =

sentry/src/main/java/io/sentry/transport/RateLimiter.java

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@
2323
import java.util.Arrays;
2424
import java.util.Collections;
2525
import java.util.Date;
26+
import java.util.Iterator;
2627
import java.util.List;
2728
import java.util.Map;
28-
import java.util.Timer;
29-
import java.util.TimerTask;
3029
import java.util.concurrent.ConcurrentHashMap;
3130
import java.util.concurrent.CopyOnWriteArrayList;
31+
import java.util.concurrent.Future;
3232
import org.jetbrains.annotations.NotNull;
3333
import org.jetbrains.annotations.Nullable;
3434

@@ -42,8 +42,9 @@ public final class RateLimiter implements Closeable {
4242
private final @NotNull Map<DataCategory, @NotNull Date> sentryRetryAfterLimit =
4343
new ConcurrentHashMap<>();
4444
private final @NotNull List<IRateLimitObserver> rateLimitObservers = new CopyOnWriteArrayList<>();
45-
private @Nullable Timer timer = null;
46-
private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock();
45+
private final @NotNull List<Future<?>> notifyObserversFutures = new ArrayList<>();
46+
private final @NotNull AutoClosableReentrantLock notifyFuturesLock =
47+
new AutoClosableReentrantLock();
4748

4849
public RateLimiter(
4950
final @NotNull ICurrentDateProvider currentDateProvider,
@@ -278,11 +279,11 @@ public void updateRetryAfterLimits(
278279
continue;
279280
}
280281

281-
applyRetryAfterOnlyIfLonger(dataCategory, date);
282+
applyRetryAfterOnlyIfLonger(dataCategory, date, retryAfterMillis);
282283
}
283284
} else {
284285
// if categories are empty, we should apply to "all" categories.
285-
applyRetryAfterOnlyIfLonger(DataCategory.All, date);
286+
applyRetryAfterOnlyIfLonger(DataCategory.All, date, retryAfterMillis);
286287
}
287288
}
288289
}
@@ -291,7 +292,7 @@ public void updateRetryAfterLimits(
291292
final long retryAfterMillis = parseRetryAfterOrDefault(retryAfterHeader);
292293
// we dont care if Date is UTC as we just add the relative seconds
293294
final Date date = new Date(currentDateProvider.getCurrentTimeMillis() + retryAfterMillis);
294-
applyRetryAfterOnlyIfLonger(DataCategory.All, date);
295+
applyRetryAfterOnlyIfLonger(DataCategory.All, date, retryAfterMillis);
295296
}
296297
}
297298

@@ -300,10 +301,11 @@ public void updateRetryAfterLimits(
300301
*
301302
* @param dataCategory the DataCategory
302303
* @param date the Date to be applied
304+
* @param delayMillis the millis until the rate limit is lifted
303305
*/
304306
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
305307
private void applyRetryAfterOnlyIfLonger(
306-
final @NotNull DataCategory dataCategory, final @NotNull Date date) {
308+
final @NotNull DataCategory dataCategory, final @NotNull Date date, final long delayMillis) {
307309
final Date oldDate = sentryRetryAfterLimit.get(dataCategory);
308310

309311
// only overwrite its previous date if the limit is even longer
@@ -312,19 +314,25 @@ private void applyRetryAfterOnlyIfLonger(
312314

313315
notifyRateLimitObservers();
314316

315-
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
316-
if (timer == null) {
317-
timer = new Timer(true);
317+
// notify observers again once the rate limit is lifted, using the shared timer executor
318+
// instead of a dedicated Timer thread
319+
try (final @NotNull ISentryLifecycleToken ignored = notifyFuturesLock.acquire()) {
320+
final @NotNull Iterator<Future<?>> iterator = notifyObserversFutures.iterator();
321+
while (iterator.hasNext()) {
322+
if (iterator.next().isDone()) {
323+
iterator.remove();
324+
}
325+
}
326+
try {
327+
notifyObserversFutures.add(
328+
options
329+
.getTimerExecutorService()
330+
.schedule(() -> notifyRateLimitObservers(), delayMillis));
331+
} catch (Throwable e) {
332+
options
333+
.getLogger()
334+
.log(SentryLevel.WARNING, "Failed to schedule rate limit lifted notification.", e);
318335
}
319-
320-
timer.schedule(
321-
new TimerTask() {
322-
@Override
323-
public void run() {
324-
notifyRateLimitObservers();
325-
}
326-
},
327-
date);
328336
}
329337
}
330338
}
@@ -364,11 +372,11 @@ public void removeRateLimitObserver(@NotNull final IRateLimitObserver observer)
364372

365373
@Override
366374
public void close() throws IOException {
367-
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
368-
if (timer != null) {
369-
timer.cancel();
370-
timer = null;
375+
try (final @NotNull ISentryLifecycleToken ignored = notifyFuturesLock.acquire()) {
376+
for (Future<?> future : notifyObserversFutures) {
377+
future.cancel(false);
371378
}
379+
notifyObserversFutures.clear();
372380
}
373381
rateLimitObservers.clear();
374382
}

sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import io.sentry.SentryEnvelope
1818
import io.sentry.SentryEnvelopeHeader
1919
import io.sentry.SentryEnvelopeItem
2020
import io.sentry.SentryEvent
21+
import io.sentry.SentryExecutorService
2122
import io.sentry.SentryLogEvent
2223
import io.sentry.SentryLogEvents
2324
import io.sentry.SentryLogLevel
@@ -37,11 +38,10 @@ import io.sentry.protocol.SentryId
3738
import io.sentry.protocol.SentryTransaction
3839
import io.sentry.protocol.User
3940
import io.sentry.test.getProperty
40-
import io.sentry.test.injectForField
4141
import io.sentry.util.HintUtils
4242
import java.io.File
43-
import java.util.Timer
4443
import java.util.UUID
44+
import java.util.concurrent.Future
4545
import java.util.concurrent.atomic.AtomicBoolean
4646
import kotlin.test.Test
4747
import kotlin.test.assertEquals
@@ -66,6 +66,8 @@ class RateLimiterTest {
6666

6767
fun getSUT(): RateLimiter {
6868
val options = SentryOptions().apply { setLogger(NoOpLogger.getInstance()) }
69+
// a real executor so scheduled rate-limit-lifted notifications actually run
70+
options.setTimerExecutorService(SentryExecutorService(options))
6971

7072
SentryOptionsManipulator.setClientReportRecorder(options, clientReportRecorder)
7173

@@ -654,7 +656,7 @@ class RateLimiterTest {
654656
}
655657

656658
@Test
657-
fun `apply rate limits schedules a timer to notify observers of lifted limits`() {
659+
fun `apply rate limits schedules a task to notify observers of lifted limits`() {
658660
val rateLimiter = fixture.getSUT()
659661
whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0, 1, 2001)
660662

@@ -667,18 +669,19 @@ class RateLimiterTest {
667669
}
668670

669671
@Test
670-
fun `close cancels the timer`() {
672+
fun `close cancels pending notify tasks`() {
671673
val rateLimiter = fixture.getSUT()
672-
val timer = mock<Timer>()
673-
rateLimiter.injectForField("timer", timer)
674+
rateLimiter.updateRetryAfterLimits("60:replay:key", null, 1)
675+
676+
val futures = rateLimiter.getProperty<List<Future<*>>>("notifyObserversFutures")
677+
assertEquals(1, futures.size)
678+
val future = futures.first()
674679

675680
// When the rate limiter is closed
676681
rateLimiter.close()
677682

678-
// Then the timer is cancelled
679-
verify(timer).cancel()
680-
681-
// And is removed by the rateLimiter
682-
assertNull(rateLimiter.getProperty("timer"))
683+
// Then the pending notify task is cancelled and dropped
684+
assertTrue(future.isCancelled)
685+
assertTrue(rateLimiter.getProperty<List<Future<*>>>("notifyObserversFutures").isEmpty())
683686
}
684687
}

0 commit comments

Comments
 (0)