diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a290e2b3e9..381efdf7abf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +### Performance + +- Remove executor prewarm during SDK init ([#5681](https://github.com/getsentry/sentry-java/pull/5681)) + - The single-threaded `SentryExecutorService` queued the prewarm work ahead of the first useful task, so it could only delay init work, never speed it up; the thread and class loading it warmed are paid identically by the first real task submitted right after. + ## 8.47.0 ### Behavioral Changes diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt index 66bda9bce0b..f402af24f80 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt @@ -81,8 +81,6 @@ class AndroidProfilerTest { override fun close(timeoutMillis: Long) {} override fun isClosed() = false - - override fun prewarm() = Unit } val options = diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 0829e4dc796..b37a6bbdee5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -89,8 +89,6 @@ class AndroidTransactionProfilerTest { override fun close(timeoutMillis: Long) {} override fun isClosed() = false - - override fun prewarm() = Unit } val options = diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt index da69b7cf330..79076e9dc8f 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt @@ -35,8 +35,6 @@ class ImmediateExecutorService : ISentryExecutorService { override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false - - override fun prewarm() = Unit } class DeferredExecutorService : ISentryExecutorService { @@ -74,8 +72,6 @@ class DeferredExecutorService : ISentryExecutorService { override fun isClosed(): Boolean = false - override fun prewarm() = Unit - fun hasScheduledRunnables(): Boolean = scheduledRunnables.isNotEmpty() } @@ -90,8 +86,6 @@ class NonOverridableNoOpSentryExecutorService : ISentryExecutorService { override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false - - override fun prewarm() = Unit } fun createSentryClientMock(enabled: Boolean = true) = diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 383ea92b116..bd285878710 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1133,7 +1133,6 @@ public abstract interface class io/sentry/ISentryClient { public abstract interface class io/sentry/ISentryExecutorService { public abstract fun close (J)V public abstract fun isClosed ()Z - public abstract fun prewarm ()V public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; 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 public fun close (J)V public static fun getInstance ()Lio/sentry/ISentryExecutorService; public fun isClosed ()Z - public fun prewarm ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; @@ -3181,7 +3179,6 @@ public final class io/sentry/SentryExecutorService : io/sentry/ISentryExecutorSe public fun (Lio/sentry/SentryOptions;)V public fun close (J)V public fun isClosed ()Z - public fun prewarm ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index ffad05361f6..9bdef8db2b7 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -45,10 +45,4 @@ Future schedule(final @NotNull Runnable runnable, final long delayMillis) * @return If the executorService was previously closed */ boolean isClosed(); - - /** - * Pre-warms the executor service by increasing the initial queue capacity. SHOULD be called - * directly after instantiating this executor service. - */ - void prewarm(); } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index c2ce81f6577..37778c3cc09 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -38,7 +38,4 @@ public void close(long timeoutMillis) {} public boolean isClosed() { return false; } - - @Override - public void prewarm() {} } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 919607e5879..f5397f191ad 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -351,7 +351,6 @@ private static void init(final @NotNull SentryOptions options, final boolean glo // to set a new one if (options.getExecutorService().isClosed()) { options.setExecutorService(new SentryExecutorService(options)); - options.getExecutorService().prewarm(); } // load lazy fields of the options in a separate thread diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index adb50b232e9..8e4731e9b5e 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -16,13 +16,6 @@ @ApiStatus.Internal public final class SentryExecutorService implements ISentryExecutorService { - /** - * ScheduledThreadPoolExecutor grows work queue by 50% each time. With the initial capacity of 16 - * it will have to resize 4 times to reach 40, which is a decent middle-ground for prewarming. - * This will prevent from growing in unexpected areas of the SDK. - */ - private static final int INITIAL_QUEUE_SIZE = 40; - /** * By default, the work queue is unbounded so it can grow as much as the memory allows. We want to * 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 { private final @NotNull ScheduledThreadPoolExecutor executorService; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); - @SuppressWarnings("UnnecessaryLambda") - private final @NotNull Runnable dummyRunnable = () -> {}; - private final @Nullable SentryOptions options; @TestOnly @@ -120,35 +110,6 @@ public boolean isClosed() { } } - @SuppressWarnings({"FutureReturnValueIgnored"}) - @Override - public void prewarm() { - try { - executorService.submit( - () -> { - try { - // schedule a bunch of dummy runnables in the future that will never execute to - // trigger - // queue growth and then purge the queue - for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { - final Future future = - executorService.schedule(dummyRunnable, 365L, TimeUnit.DAYS); - future.cancel(true); - } - executorService.purge(); - } catch (RejectedExecutionException ignored) { - // ignore - } - }); - } catch (RejectedExecutionException e) { - if (options != null) { - options - .getLogger() - .log(SentryLevel.WARNING, "Prewarm task rejected from " + executorService, e); - } - } - } - private static final class SentryExecutorServiceThreadFactory implements ThreadFactory { private int cnt; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 0d038482d07..8c995001016 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -678,7 +678,6 @@ public void activate() { // SentryExecutorService should be initialized before any // SendCachedEventFireAndForgetIntegration executorService = new SentryExecutorService(this); - executorService.prewarm(); } // SpotlightIntegration is loaded via reflection to allow the sentry-spotlight module diff --git a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt index 2ccba650ad9..57dfb578ee9 100644 --- a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt +++ b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt @@ -1,6 +1,5 @@ package io.sentry -import io.sentry.test.getProperty import java.util.concurrent.BlockingQueue import java.util.concurrent.Callable import java.util.concurrent.CancellationException @@ -191,21 +190,6 @@ class SentryExecutorServiceTest { verify(executor).schedule(any(), any(), any()) } - @Test - fun `SentryExecutorService prewarm schedules dummy tasks and clears queue`() { - val executor = ScheduledThreadPoolExecutor(1) - - val sentryExecutor = SentryExecutorService(executor, null) - sentryExecutor.prewarm() - - Thread.sleep(1000) - - // the internal queue/array should be resized 4 times to 54 - assertEquals(54, (executor.queue.getProperty("queue") as Array<*>).size) - // the queue should be empty - assertEquals(0, executor.queue.size) - } - @Test fun `SentryExecutorService schedules any number of job`() { val executor = ScheduledThreadPoolExecutor(1) diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 3712b083de7..4a122f11422 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -778,7 +778,6 @@ class SentryTest { it.cacheDirPath = getTempPath() it.setLogger(logger) it.executorService = SentryExecutorService() - it.executorService.prewarm() it.executorService.close(0) it.isDebug = true }