feat(core): Replace SentryExecutorService with pre-allocated queue#5717
Draft
romtsn wants to merge 3 commits into
Draft
feat(core): Replace SentryExecutorService with pre-allocated queue#5717romtsn wants to merge 3 commits into
romtsn wants to merge 3 commits into
Conversation
…plementation ScheduledThreadPoolExecutor's internal DelayedWorkQueue is a heap that resizes dynamically (50% growth from initial capacity 16). prewarm() was introduced to pre-grow that array during init, but doing so on the main thread is itself the worst possible time to trigger allocations — and the queue resize cost is only ~8µs anyway. This replaces the whole approach: a custom executor backed by a PriorityQueue pre-allocated to INITIAL_QUEUE_CAPACITY=64 at construction time. The backing array never resizes during normal SDK operation. A single daemon worker thread uses Object.wait/notifyAll for precise wakeup on scheduled tasks. prewarm() becomes a documented no-op. Key properties: - No array resize at runtime: queue pre-allocated at construction - Precise scheduling: worker sleeps until next task triggerTime, wakes immediately when an earlier task is enqueued - MAX_QUEUE_SIZE (271) and purge-on-overflow semantics preserved - ScheduledTask<T> extends FutureTask<T> for free Future<T> contract - Drops @testonly ScheduledThreadPoolExecutor constructor (nothing to inject) Refs #5681
Old tests verified delegation to a mocked ScheduledThreadPoolExecutor. New tests verify actual executor behavior: task execution, scheduling, close semantics, queue limit enforcement, cancelled-task purging, and trigger-time ordering.
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Replace SentryExecutorService with pre-allocated queue ([#5717](https://github.com/getsentry/sentry-java/pull/5717))If none of the above apply, you can opt out of this check by adding |
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| abfcc92 | 309.54 ms | 380.32 ms | 70.78 ms |
| 4e3e79d | 328.10 ms | 395.64 ms | 67.54 ms |
| 8c1fb22 | 316.62 ms | 352.78 ms | 36.16 ms |
| b67bb28 | 307.59 ms | 341.24 ms | 33.65 ms |
| d15471f | 361.89 ms | 378.07 ms | 16.18 ms |
| 22f4345 | 313.52 ms | 364.96 ms | 51.44 ms |
| 6727e14 | 337.22 ms | 373.94 ms | 36.71 ms |
| fc5ccaf | 322.49 ms | 405.25 ms | 82.76 ms |
| 22f4345 | 314.79 ms | 375.02 ms | 60.23 ms |
| 2195398 | 319.02 ms | 342.38 ms | 23.36 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| abfcc92 | 1.58 MiB | 2.13 MiB | 557.31 KiB |
| 4e3e79d | 0 B | 0 B | 0 B |
| 8c1fb22 | 0 B | 0 B | 0 B |
| b67bb28 | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| 6727e14 | 1.58 MiB | 2.28 MiB | 718.64 KiB |
| fc5ccaf | 1.58 MiB | 2.13 MiB | 557.54 KiB |
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| 2195398 | 0 B | 0 B | 0 B |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ScheduledThreadPoolExecutor's internalDelayedWorkQueueis a min-heap that grows dynamically (50% per resize, starting from 16).prewarm()was introduced to pre-grow that array duringSentry.init— but init runs on the main thread, which is exactly when ANR risk is highest. The resize cost it avoids is only ~8µs; the prewarm itself adds ~100µs and is the more expensive operation.More fundamentally,
ScheduledThreadPoolExecutor's queue isn't replaceable, so there was no clean way to fix this without a custom implementation.Solution
Replace
SentryExecutorServicewith a custom implementation backed by:PriorityQueue<ScheduledTask<?>>pre-allocated toINITIAL_QUEUE_CAPACITY = 64slots at construction — the backing array never resizes during normal SDK operationObject.wait(millis)/notifyAll()— sleeps precisely until the next task is due, wakes immediately when an earlier task is enqueuedScheduledTask<T> extends FutureTask<T>— gets the fullFuture<T>contract for freeprewarm()is now a documented no-op. TheMAX_QUEUE_SIZE = 271hard limit and purge-on-overflow behaviour are preserved unchanged.What changed
ScheduledThreadPoolExecutor.DelayedWorkQueue(dynamic, unreplaceable)PriorityQueuepre-allocated to 64 slotsprewarm()@TestOnlyconstructorScheduledThreadPoolExecutorNotes
./gradlew spotlessApply apiDumpmust be run locally before merging — sandbox can't run the JVMprewarm()is kept in the interface as@Deprecatedfor API compatibility; can be removed fromISentryExecutorServicein a follow-upcc @romtsn
--
View Junior Session in Sentry