Skip to content

Commit f2802aa

Browse files
runningcodeclaude
andauthored
perf: Let HostnameCache worker thread idle out (JAVA-653) (#5817)
* perf: Let HostnameCache worker thread idle out (JAVA-653) HostnameCache used Executors.newSingleThreadExecutor, whose worker thread stays alive for the life of the process even though the cache refreshes at most once every 5 hours. Use a ThreadPoolExecutor with a keep-alive and allowCoreThreadTimeOut(true) so the thread terminates when idle and is recreated on the next refresh. Keeping a dedicated executor (rather than sharing one) preserves the blocking future.get(1s) timeout behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * changelog --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 998c538 commit f2802aa

3 files changed

Lines changed: 60 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- Parse the app start profiling config with only the deserializer it needs instead of building a full `JsonSerializer` and `SentryOptions`, cutting 188 of 221 allocations on the main thread before `Application.onCreate` ([#5867](https://github.com/getsentry/sentry-java/pull/5867))
2222
- Batch and coalesce scope-persistence disk writes to reduce startup cost ([#5791](https://github.com/getsentry/sentry-java/pull/5791))
2323
- Scope mutations are now coalesced (latest value per field) and breadcrumbs are appended in batches behind a single fsync, instead of one synchronous disk write per mutation.
24+
- Reduce the number of SDK threads: the `HostnameCache` worker thread now times out while idle instead of staying alive for the whole process lifetime ([#5817](https://github.com/getsentry/sentry-java/pull/5817))
2425

2526
### Dependencies
2627

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66
import java.util.concurrent.Callable;
77
import java.util.concurrent.ExecutionException;
88
import java.util.concurrent.ExecutorService;
9-
import java.util.concurrent.Executors;
109
import java.util.concurrent.Future;
10+
import java.util.concurrent.LinkedBlockingQueue;
1111
import java.util.concurrent.ThreadFactory;
12+
import java.util.concurrent.ThreadPoolExecutor;
1213
import java.util.concurrent.TimeUnit;
1314
import java.util.concurrent.TimeoutException;
1415
import java.util.concurrent.atomic.AtomicBoolean;
@@ -34,6 +35,9 @@ public final class HostnameCache {
3435
/** Time before the get hostname operation times out (in ms). */
3536
private static final long GET_HOSTNAME_TIMEOUT = TimeUnit.SECONDS.toMillis(1);
3637

38+
/** How long the worker thread may stay idle before it self-terminates. */
39+
private static final long THREAD_KEEP_ALIVE_SECONDS = 30;
40+
3741
private static volatile @Nullable HostnameCache INSTANCE;
3842
private static final @NotNull AutoClosableReentrantLock staticLock =
3943
new AutoClosableReentrantLock();
@@ -52,8 +56,7 @@ public final class HostnameCache {
5256

5357
private final @NotNull Callable<InetAddress> getLocalhost;
5458

55-
private final @NotNull ExecutorService executorService =
56-
Executors.newSingleThreadExecutor(new HostnameCacheThreadFactory());
59+
private final @NotNull ExecutorService executorService;
5760

5861
public static @NotNull HostnameCache getInstance() {
5962
if (INSTANCE == null) {
@@ -87,6 +90,18 @@ private HostnameCache() {
8790
HostnameCache(long cacheDuration, final @NotNull Callable<InetAddress> getLocalhost) {
8891
this.cacheDuration = cacheDuration;
8992
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
93+
// A single thread executor whose worker thread times out while idle, so no thread is kept
94+
// alive between the infrequent cache refreshes.
95+
final @NotNull ThreadPoolExecutor executor =
96+
new ThreadPoolExecutor(
97+
1,
98+
1,
99+
THREAD_KEEP_ALIVE_SECONDS,
100+
TimeUnit.SECONDS,
101+
new LinkedBlockingQueue<>(),
102+
new HostnameCacheThreadFactory());
103+
executor.allowCoreThreadTimeOut(true);
104+
this.executorService = executor;
90105
updateCache();
91106
}
92107

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package io.sentry
2+
3+
import com.google.common.truth.Truth.assertThat
4+
import io.sentry.test.getProperty
5+
import java.net.InetAddress
6+
import java.util.concurrent.ThreadPoolExecutor
7+
import java.util.concurrent.TimeUnit
8+
import kotlin.test.Test
9+
import org.mockito.kotlin.mock
10+
import org.mockito.kotlin.whenever
11+
12+
class HostnameCacheTest {
13+
14+
private fun getSut(): HostnameCache {
15+
val address = mock<InetAddress>()
16+
whenever(address.canonicalHostName).thenReturn("myhost")
17+
return HostnameCache(TimeUnit.HOURS.toMillis(1)) { address }
18+
}
19+
20+
@Test
21+
fun `hostname is resolved and cached`() {
22+
val cache = getSut()
23+
assertThat(cache.hostname).isEqualTo("myhost")
24+
}
25+
26+
@Test
27+
fun `worker thread times out while idle instead of staying alive`() {
28+
val cache = getSut()
29+
val executorService = cache.getProperty<ThreadPoolExecutor>("executorService")
30+
assertThat(executorService.allowsCoreThreadTimeOut()).isTrue()
31+
assertThat(executorService.corePoolSize).isEqualTo(1)
32+
assertThat(executorService.maximumPoolSize).isEqualTo(1)
33+
}
34+
35+
@Test
36+
fun `close shuts the executor down`() {
37+
val cache = getSut()
38+
cache.close()
39+
assertThat(cache.isClosed).isTrue()
40+
}
41+
}

0 commit comments

Comments
 (0)