Skip to content

Commit e4b7ee0

Browse files
runningcodeclaude
runningcode
and
claude
committed
perf(core): Remove executor prewarm
SentryExecutorService is single-threaded, and prewarm() is submitted ahead of loadLazyFields() during init, so its 40-task schedule/cancel/purge loop cannot reduce first-task latency — it can only delay it. The thread creation and executor class loading it aimed to warm are paid identically by the first real task (loadLazyFields), which is submitted unconditionally right after, so prewarm warms nothing that would not already be warmed. On-device A/B benchmarks on a Galaxy A55 (Android 16) show no measurable first-useful-task speedup from prewarm and ~20us of extra background-thread work. Remove prewarm() from ISentryExecutorService and its implementations and from both init call sites, and add PrewarmBenchmarkTest documenting the measurement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7d8a394 commit e4b7ee0

12 files changed

Lines changed: 178 additions & 80 deletions

File tree

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@ class AndroidProfilerTest {
8181
override fun close(timeoutMillis: Long) {}
8282

8383
override fun isClosed() = false
84-
85-
override fun prewarm() = Unit
8684
}
8785

8886
val options =

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ class AndroidTransactionProfilerTest {
8989
override fun close(timeoutMillis: Long) {}
9090

9191
override fun isClosed() = false
92-
93-
override fun prewarm() = Unit
9492
}
9593

9694
val options =

‎sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/PrewarmBenchmarkTest.kt‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package io.sentry.uitest.android.benchmark
2+
3+
import android.util.Log
4+
import androidx.test.ext.junit.runners.AndroidJUnit4
5+
import io.sentry.SentryExecutorService
6+
import java.util.concurrent.CountDownLatch
7+
import java.util.concurrent.ScheduledThreadPoolExecutor
8+
import java.util.concurrent.TimeUnit
9+
import kotlin.test.Test
10+
import org.junit.runner.RunWith
11+
12+
/**
13+
* On-device A/B benchmark for [SentryExecutorService.prewarm]. Answers two questions asked during
14+
* the JAVA-588 executor cleanup:
15+
* 1. Current prewarm (schedule+cancel 40 dummy tasks, then purge) vs a single `submit(noop)`.
16+
* 2. Whether warming the executor thread / class loading during init helps the first useful task at
17+
* all — given the executor is single-threaded and prewarm is queued ahead of `loadLazyFields`.
18+
*
19+
* Uses `System.nanoTime` with interleaved rounds (same methodology as [InitReflectionBenchmarkTest]
20+
* and [FindTargetBenchmarkTest]). Not run in CI. Each test method runs in a fresh process under the
21+
* orchestrator, which lets [firstTaskClassLoadingCost] observe process-once class loading.
22+
*/
23+
@RunWith(AndroidJUnit4::class)
24+
class PrewarmBenchmarkTest {
25+
26+
private val tag = "PrewarmBench"
27+
28+
// The current prewarm work, reproduced so it can be timed directly. Schedules n dummy runnables
29+
// far in the future, cancels each (they stay queued until purge, forcing the DelayedWorkQueue
30+
// backing array to grow), then purges.
31+
private fun prewarmBody(executor: ScheduledThreadPoolExecutor, n: Int) {
32+
val dummy = Runnable {}
33+
for (i in 0 until n) {
34+
val f = executor.schedule(dummy, 365L, TimeUnit.DAYS)
35+
f.cancel(true)
36+
}
37+
executor.purge()
38+
}
39+
40+
/**
41+
* M1 — raw cost of the prewarm work on the executor thread: the current 40-task loop vs a single
42+
* dummy schedule vs nothing. Run on a warm, dedicated executor so thread-creation noise is
43+
* excluded and only the queue churn is measured.
44+
*/
45+
@Test
46+
fun prewarmBodyCost() {
47+
val executor = ScheduledThreadPoolExecutor(1)
48+
// warm the executor thread + class loading so timings reflect only the loop work
49+
prewarmBody(executor, 40)
50+
51+
fun time(n: Int): Long {
52+
val start = System.nanoTime()
53+
prewarmBody(executor, n)
54+
return System.nanoTime() - start
55+
}
56+
57+
repeat(3) {
58+
time(40)
59+
time(1)
60+
}
61+
val rounds = 8
62+
var loop40 = 0L
63+
var single = 0L
64+
for (round in 0 until rounds) {
65+
if (round % 2 == 0) {
66+
loop40 += time(40)
67+
single += time(1)
68+
} else {
69+
single += time(1)
70+
loop40 += time(40)
71+
}
72+
}
73+
Log.i(
74+
tag,
75+
"M1 prewarm-body: loop40=${loop40 / rounds / 1000}us single=${single / rounds / 1000}us " +
76+
"delta=${(loop40 - single) / rounds / 1000}us",
77+
)
78+
executor.shutdownNow()
79+
}
80+
81+
/**
82+
* M2 — the decision metric. From a fresh [SentryExecutorService] (as init creates), measure the
83+
* time until the first useful task (a stand-in for `loadLazyFields`) actually finishes running,
84+
* for three init strategies:
85+
* - A: current `prewarm()` then submit the task
86+
* - B: `submit(noop)` then submit the task
87+
* - C: no warming, just submit the task
88+
*
89+
* On a single-threaded executor all warming is queued ahead of the task, so this shows whether
90+
* prewarming brings the first useful task sooner (helps) or later (pure overhead).
91+
*/
92+
@Test
93+
fun firstUsefulTaskDelay() {
94+
fun measure(strategy: Int): Long {
95+
val svc = SentryExecutorService()
96+
val done = CountDownLatch(1)
97+
val start = System.nanoTime()
98+
when (strategy) {
99+
0 -> svc.prewarm()
100+
1 -> svc.submit {}
101+
2 -> {}
102+
}
103+
svc.submit { done.countDown() }
104+
done.await(5, TimeUnit.SECONDS)
105+
val elapsed = System.nanoTime() - start
106+
svc.close(0)
107+
return elapsed
108+
}
109+
110+
// discard warmup (first executor in process also pays class loading, biasing whoever runs
111+
// first)
112+
repeat(4) {
113+
measure(0)
114+
measure(1)
115+
measure(2)
116+
}
117+
val rounds = 12
118+
var a = 0L
119+
var b = 0L
120+
var c = 0L
121+
for (round in 0 until rounds) {
122+
// rotate order each round to cancel ordering/thermal bias
123+
when (round % 3) {
124+
0 -> {
125+
a += measure(0)
126+
b += measure(1)
127+
c += measure(2)
128+
}
129+
1 -> {
130+
b += measure(1)
131+
c += measure(2)
132+
a += measure(0)
133+
}
134+
else -> {
135+
c += measure(2)
136+
a += measure(0)
137+
b += measure(1)
138+
}
139+
}
140+
}
141+
Log.i(
142+
tag,
143+
"M2 first-useful-task delay: A(prewarm)=${a / rounds / 1000}us " +
144+
"B(single-submit)=${b / rounds / 1000}us C(none)=${c / rounds / 1000}us",
145+
)
146+
}
147+
148+
/**
149+
* M3 — upper bound on what "warming class loading" could ever save: the extra latency the very
150+
* first [ScheduledThreadPoolExecutor] first-task pays (class loading + JIT of the executor
151+
* internals + first thread creation) versus a subsequent, warm one in the same process. In a real
152+
* app this cost is only paid by Sentry if the app has not already used an STPE.
153+
*/
154+
@Test
155+
fun firstTaskClassLoadingCost() {
156+
fun firstTaskLatency(): Long {
157+
val executor = ScheduledThreadPoolExecutor(1)
158+
val done = CountDownLatch(1)
159+
val start = System.nanoTime()
160+
executor.submit { done.countDown() }
161+
done.await(5, TimeUnit.SECONDS)
162+
val elapsed = System.nanoTime() - start
163+
executor.shutdownNow()
164+
return elapsed
165+
}
166+
167+
val cold = firstTaskLatency() // pays process-once class loading + JIT
168+
var warmTotal = 0L
169+
val warmRounds = 8
170+
repeat(warmRounds) { warmTotal += firstTaskLatency() }
171+
Log.i(
172+
tag,
173+
"M3 first-task latency: cold(first-in-process)=${cold / 1000}us " +
174+
"warm(avg)=${warmTotal / warmRounds / 1000}us " +
175+
"class-loading-upper-bound=${(cold - warmTotal / warmRounds) / 1000}us",
176+
)
177+
}
178+
}

‎sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ class ImmediateExecutorService : ISentryExecutorService {
3535
override fun close(timeoutMillis: Long) {}
3636

3737
override fun isClosed(): Boolean = false
38-
39-
override fun prewarm() = Unit
4038
}
4139

4240
class DeferredExecutorService : ISentryExecutorService {
@@ -74,8 +72,6 @@ class DeferredExecutorService : ISentryExecutorService {
7472

7573
override fun isClosed(): Boolean = false
7674

77-
override fun prewarm() = Unit
78-
7975
fun hasScheduledRunnables(): Boolean = scheduledRunnables.isNotEmpty()
8076
}
8177

@@ -90,8 +86,6 @@ class NonOverridableNoOpSentryExecutorService : ISentryExecutorService {
9086
override fun close(timeoutMillis: Long) {}
9187

9288
override fun isClosed(): Boolean = false
93-
94-
override fun prewarm() = Unit
9589
}
9690

9791
fun createSentryClientMock(enabled: Boolean = true) =

‎sentry/api/sentry.api‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,6 @@ public abstract interface class io/sentry/ISentryClient {
11331133
public abstract interface class io/sentry/ISentryExecutorService {
11341134
public abstract fun close (J)V
11351135
public abstract fun isClosed ()Z
1136-
public abstract fun prewarm ()V
11371136
public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
11381137
public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
11391138
public abstract fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
@@ -1879,7 +1878,6 @@ public final class io/sentry/NoOpSentryExecutorService : io/sentry/ISentryExecut
18791878
public fun close (J)V
18801879
public static fun getInstance ()Lio/sentry/ISentryExecutorService;
18811880
public fun isClosed ()Z
1882-
public fun prewarm ()V
18831881
public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
18841882
public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
18851883
public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
@@ -3181,7 +3179,6 @@ public final class io/sentry/SentryExecutorService : io/sentry/ISentryExecutorSe
31813179
public fun <init> (Lio/sentry/SentryOptions;)V
31823180
public fun close (J)V
31833181
public fun isClosed ()Z
3184-
public fun prewarm ()V
31853182
public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
31863183
public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
31873184
public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;

‎sentry/src/main/java/io/sentry/ISentryExecutorService.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,4 @@ Future<?> schedule(final @NotNull Runnable runnable, final long delayMillis)
4545
* @return If the executorService was previously closed
4646
*/
4747
boolean isClosed();
48-
49-
/**
50-
* Pre-warms the executor service by increasing the initial queue capacity. SHOULD be called
51-
* directly after instantiating this executor service.
52-
*/
53-
void prewarm();
5448
}

‎sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,4 @@ public void close(long timeoutMillis) {}
3838
public boolean isClosed() {
3939
return false;
4040
}
41-
42-
@Override
43-
public void prewarm() {}
4441
}

‎sentry/src/main/java/io/sentry/Sentry.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,6 @@ private static void init(final @NotNull SentryOptions options, final boolean glo
351351
// to set a new one
352352
if (options.getExecutorService().isClosed()) {
353353
options.setExecutorService(new SentryExecutorService(options));
354-
options.getExecutorService().prewarm();
355354
}
356355

357356
// load lazy fields of the options in a separate thread

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

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,6 @@
1616
@ApiStatus.Internal
1717
public final class SentryExecutorService implements ISentryExecutorService {
1818

19-
/**
20-
* ScheduledThreadPoolExecutor grows work queue by 50% each time. With the initial capacity of 16
21-
* it will have to resize 4 times to reach 40, which is a decent middle-ground for prewarming.
22-
* This will prevent from growing in unexpected areas of the SDK.
23-
*/
24-
private static final int INITIAL_QUEUE_SIZE = 40;
25-
2619
/**
2720
* By default, the work queue is unbounded so it can grow as much as the memory allows. We want to
2821
* limit it by 271 which would be x8 times growth from the default initial capacity.
@@ -32,9 +25,6 @@ public final class SentryExecutorService implements ISentryExecutorService {
3225
private final @NotNull ScheduledThreadPoolExecutor executorService;
3326
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
3427

35-
@SuppressWarnings("UnnecessaryLambda")
36-
private final @NotNull Runnable dummyRunnable = () -> {};
37-
3828
private final @Nullable SentryOptions options;
3929

4030
@TestOnly
@@ -120,35 +110,6 @@ public boolean isClosed() {
120110
}
121111
}
122112

123-
@SuppressWarnings({"FutureReturnValueIgnored"})
124-
@Override
125-
public void prewarm() {
126-
try {
127-
executorService.submit(
128-
() -> {
129-
try {
130-
// schedule a bunch of dummy runnables in the future that will never execute to
131-
// trigger
132-
// queue growth and then purge the queue
133-
for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) {
134-
final Future<?> future =
135-
executorService.schedule(dummyRunnable, 365L, TimeUnit.DAYS);
136-
future.cancel(true);
137-
}
138-
executorService.purge();
139-
} catch (RejectedExecutionException ignored) {
140-
// ignore
141-
}
142-
});
143-
} catch (RejectedExecutionException e) {
144-
if (options != null) {
145-
options
146-
.getLogger()
147-
.log(SentryLevel.WARNING, "Prewarm task rejected from " + executorService, e);
148-
}
149-
}
150-
}
151-
152113
private static final class SentryExecutorServiceThreadFactory implements ThreadFactory {
153114
private int cnt;
154115

‎sentry/src/main/java/io/sentry/SentryOptions.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,6 @@ public void activate() {
678678
// SentryExecutorService should be initialized before any
679679
// SendCachedEventFireAndForgetIntegration
680680
executorService = new SentryExecutorService(this);
681-
executorService.prewarm();
682681
}
683682

684683
// SpotlightIntegration is loaded via reflection to allow the sentry-spotlight module

0 commit comments

Comments
 (0)