perf(core): Lazily allocate AutoClosableReentrantLock (JAVA-588)#5643
Draft
runningcode wants to merge 2 commits into
Draft
perf(core): Lazily allocate AutoClosableReentrantLock (JAVA-588)#5643runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
AutoClosableReentrantLock extended ReentrantLock, so every SDK object holding one allocated a ReentrantLock (and its AbstractQueuedSynchronizer) eagerly in its field initializer. A customer Perfetto trace showed ~81 such allocations on the main thread during SentryAndroid.init, many for locks that are never acquired during init. Hold the ReentrantLock internally and create it lazily on first acquire(), using an AtomicReferenceFieldUpdater CAS so creation stays atomic and Loom-friendly (no synchronized, preserving #3715). Every call site uses acquire() only, so dropping the ReentrantLock superclass touches no caller. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Performance
- Lazily allocate AutoClosableReentrantLock (JAVA-588) ([#5643](https://github.com/getsentry/sentry-java/pull/5643))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 |
|---|---|---|---|
| 18c0bc2 | 306.73 ms | 349.77 ms | 43.03 ms |
| 0eaac1e | 316.82 ms | 357.34 ms | 40.52 ms |
| d15471f | 303.49 ms | 439.08 ms | 135.59 ms |
| fc5ccaf | 276.52 ms | 370.46 ms | 93.93 ms |
| e2dce0b | 308.96 ms | 360.10 ms | 51.14 ms |
| 5b1a06b | 352.27 ms | 413.70 ms | 61.43 ms |
| 37ec571 | 366.04 ms | 424.28 ms | 58.23 ms |
| 9fbb112 | 361.43 ms | 427.57 ms | 66.14 ms |
| bbc35bb | 324.88 ms | 425.73 ms | 100.85 ms |
| ff8eea4 | 313.42 ms | 337.08 ms | 23.66 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 18c0bc2 | 1.58 MiB | 2.13 MiB | 557.33 KiB |
| 0eaac1e | 1.58 MiB | 2.19 MiB | 619.17 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| fc5ccaf | 1.58 MiB | 2.13 MiB | 557.54 KiB |
| e2dce0b | 0 B | 0 B | 0 B |
| 5b1a06b | 0 B | 0 B | 0 B |
| 37ec571 | 0 B | 0 B | 0 B |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| bbc35bb | 1.58 MiB | 2.12 MiB | 553.01 KiB |
| ff8eea4 | 1.58 MiB | 2.28 MiB | 718.64 KiB |
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.
📜 Description
io.sentry.util.AutoClosableReentrantLockextendedjava.util.concurrent.locks.ReentrantLock, and ~57 SDK classes create one eagerly in a field initializer. Constructing the SDK object graph therefore allocated aReentrantLock(plus itsAbstractQueuedSynchronizerSync) per object — even for objects whose lock is never acquired.This change holds the
ReentrantLockinternally and creates it lazily on the firstacquire(), using anAtomicReferenceFieldUpdaterCAS so the lazy creation is atomic and stays Loom-friendly (nosynchronized, preserving the intent of #3715). The lifecycle token captures the resolved lock instance soclose()unlocks the correct one.💡 Motivation and Context
Part of the Reduce SDK init time [Android] effort (JAVA-588). A customer-provided Perfetto trace showed ~81
ReentrantLockallocations on the main thread underSentryAndroid.init, contributing GC pressure and main-thread CPU. With lazy allocation, only locks actually acquired during init allocate; the many never-contended locks no longer allocate at construction.Compatibility note: this is technically a binary-compatibility change —
AutoClosableReentrantLockno longer extendsReentrantLock, so the inheritedReentrantLockpublic methods leave its API surface (reflected insentry.api). A usage audit confirmed every call site uses onlyacquire()(try-with-resources); nothing callslock()/unlock()/tryLock()/isHeldByCurrentThread()directly or types a field/param asReentrantLock, so no caller is affected.A new on-device A/B benchmark (
LockAllocationBenchmarkTest, not run in CI) quantifies the allocation reduction and confirmsacquire()hot-path throughput is unchanged.💚 How did you test it?
Unit tests in
AutoClosableReentrantLockTest(lazy creation, reentrancy, and an 8-thread × 1000-iteration mutual-exclusion stress test); full:sentry:testsuite green.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Biggest remaining main-thread init cost in the same trace is Manifest parsing (~147ms) — tracked separately (SDK-1322 / JAVA-531, compile-time injection).
Fixes JAVA-588