From 4be3e70df364b71dad0ce316ea49a78b2857dc3d Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 25 Jun 2026 16:23:09 +0200 Subject: [PATCH 1/4] perf(core): Lazily allocate AutoClosableReentrantLock (JAVA-588) 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) --- sentry/api/sentry.api | 2 +- .../util/AutoClosableReentrantLock.java | 51 +++++++++++++++++-- .../util/AutoClosableReentrantLockTest.kt | 49 ++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 04c876fdbdb..59b8add2102 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7589,7 +7589,7 @@ public abstract class io/sentry/transport/TransportResult { public static fun success ()Lio/sentry/transport/TransportResult; } -public final class io/sentry/util/AutoClosableReentrantLock : java/util/concurrent/locks/ReentrantLock { +public final class io/sentry/util/AutoClosableReentrantLock { public fun ()V public fun acquire ()Lio/sentry/ISentryLifecycleToken; } diff --git a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java index 2a95a58b5fe..b2f28313b89 100644 --- a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java +++ b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java @@ -1,16 +1,57 @@ package io.sentry.util; import io.sentry.ISentryLifecycleToken; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.locks.ReentrantLock; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; -public final class AutoClosableReentrantLock extends ReentrantLock { +/** + * Hands out an {@link ISentryLifecycleToken} from {@link #acquire()} for use with + * try-with-resources (replacing {@code synchronized} blocks). + * + *

The underlying {@link ReentrantLock} is created lazily on the first {@link #acquire()}. Many + * SDK objects hold a lock but never contend on it (especially during {@code SentryAndroid.init}), + * so the eager allocation of a {@link ReentrantLock} (and its {@code AbstractQueuedSynchronizer}) + * was pure GC and main-thread overhead. We keep a {@link ReentrantLock} rather than reverting to + * {@code synchronized} to stay friendly to virtual threads (Loom), see #3715. + */ +public final class AutoClosableReentrantLock { - private static final long serialVersionUID = -3283069816958445549L; + private static final @NotNull AtomicReferenceFieldUpdater< + AutoClosableReentrantLock, ReentrantLock> + LOCK_UPDATER = + AtomicReferenceFieldUpdater.newUpdater( + AutoClosableReentrantLock.class, ReentrantLock.class, "lock"); - public ISentryLifecycleToken acquire() { - lock(); - return new AutoClosableReentrantLockLifecycleToken(this); + private volatile @Nullable ReentrantLock lock; + + public @NotNull ISentryLifecycleToken acquire() { + final @NotNull ReentrantLock theLock = getOrCreateLock(); + theLock.lock(); + return new AutoClosableReentrantLockLifecycleToken(theLock); + } + + private @NotNull ReentrantLock getOrCreateLock() { + final @Nullable ReentrantLock existing = lock; + if (existing != null) { + return existing; + } + // The loser of the race discards its candidate and uses the winner's lock, so all callers + // contend on the same instance. + final @NotNull ReentrantLock candidate = new ReentrantLock(); + if (LOCK_UPDATER.compareAndSet(this, null, candidate)) { + return candidate; + } + final @Nullable ReentrantLock winner = lock; + return winner != null ? winner : candidate; + } + + @TestOnly + boolean isLocked() { + final @Nullable ReentrantLock current = lock; + return current != null && current.isLocked(); } static final class AutoClosableReentrantLockLifecycleToken implements ISentryLifecycleToken { diff --git a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt index 4a69b9638e7..a301425e41c 100644 --- a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt +++ b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt @@ -1,6 +1,10 @@ package io.sentry.util +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -11,4 +15,49 @@ class AutoClosableReentrantLockTest { lock.acquire().use { assertTrue(lock.isLocked) } assertFalse(lock.isLocked) } + + @Test + fun `does not allocate the underlying lock until first acquire`() { + val lock = AutoClosableReentrantLock() + assertFalse(lock.isLocked) + } + + @Test + fun `supports reentrant acquire from the same thread`() { + val lock = AutoClosableReentrantLock() + lock.acquire().use { + lock.acquire().use { assertTrue(lock.isLocked) } + assertTrue(lock.isLocked) + } + assertFalse(lock.isLocked) + } + + @Test + fun `mutually excludes concurrent threads`() { + val lock = AutoClosableReentrantLock() + val inCriticalSection = AtomicInteger(0) + val maxObserved = AtomicInteger(0) + val start = CountDownLatch(1) + val threadCount = 8 + val iterations = 1000 + val threads = + (0 until threadCount).map { + Thread { + start.await() + repeat(iterations) { + lock.acquire().use { + val current = inCriticalSection.incrementAndGet() + maxObserved.accumulateAndGet(current, ::maxOf) + inCriticalSection.decrementAndGet() + } + } + } + } + threads.forEach(Thread::start) + start.countDown() + threads.forEach { it.join(TimeUnit.SECONDS.toMillis(10)) } + + assertEquals(1, maxObserved.get()) + assertFalse(lock.isLocked) + } } From bd1235926a1e7d4a446b836533044c2052560e6a Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 25 Jun 2026 16:24:12 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31c600cbcf4..95a6a5b5f67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Avoid constructing an exception per view when resolving view ids during view-hierarchy and gesture capture ([#5631](https://github.com/getsentry/sentry-java/pull/5631)) - Start the frame metrics thread lazily on first collection instead of during SDK init ([#5641](https://github.com/getsentry/sentry-java/pull/5641)) - Reduce `SentryId` and `SpanId` allocation overhead by replacing their per-instance `LazyEvaluator` (and its lock) with a lightweight lazily-generated `String`. ([#5645](https://github.com/getsentry/sentry-java/pull/5645)) +- Lazily allocate the `ReentrantLock` backing `AutoClosableReentrantLock` to avoid eager lock allocations for SDK objects that never contend during `SentryAndroid.init` ([#5643](https://github.com/getsentry/sentry-java/pull/5643)) ## 8.46.0 From f7075cb6549111daa7ca04e4e95d685dd6e095a3 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 2 Jul 2026 10:34:11 +0200 Subject: [PATCH 3/4] perf(core): Harden lazy lock init and mark AutoClosableReentrantLock internal (JAVA-588) Replace the unreachable candidate fallback after a failed CAS with an explicit non-null check, so a broken invariant fails loudly instead of handing two threads different locks. Mark the class @ApiStatus.Internal and make the lazy-allocation test assert the lock field directly. Co-Authored-By: Claude Fable 5 --- .../io/sentry/util/AutoClosableReentrantLock.java | 14 ++++++++++---- .../sentry/util/AutoClosableReentrantLockTest.kt | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java index b2f28313b89..e9c267fb734 100644 --- a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java +++ b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java @@ -3,6 +3,7 @@ import io.sentry.ISentryLifecycleToken; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.locks.ReentrantLock; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -17,6 +18,7 @@ * was pure GC and main-thread overhead. We keep a {@link ReentrantLock} rather than reverting to * {@code synchronized} to stay friendly to virtual threads (Loom), see #3715. */ +@ApiStatus.Internal public final class AutoClosableReentrantLock { private static final @NotNull AtomicReferenceFieldUpdater< @@ -38,14 +40,13 @@ public final class AutoClosableReentrantLock { if (existing != null) { return existing; } - // The loser of the race discards its candidate and uses the winner's lock, so all callers - // contend on the same instance. final @NotNull ReentrantLock candidate = new ReentrantLock(); if (LOCK_UPDATER.compareAndSet(this, null, candidate)) { return candidate; } - final @Nullable ReentrantLock winner = lock; - return winner != null ? winner : candidate; + // The CAS can only fail because another thread installed its lock first, and the field is + // never reset, so all callers end up contending on that same instance. + return Objects.requireNonNull(lock, "lock must have been set by the winning thread"); } @TestOnly @@ -54,6 +55,11 @@ boolean isLocked() { return current != null && current.isLocked(); } + @TestOnly + boolean isLockAllocated() { + return lock != null; + } + static final class AutoClosableReentrantLockLifecycleToken implements ISentryLifecycleToken { private final @NotNull ReentrantLock lock; diff --git a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt index a301425e41c..d89c49836e9 100644 --- a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt +++ b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt @@ -19,7 +19,9 @@ class AutoClosableReentrantLockTest { @Test fun `does not allocate the underlying lock until first acquire`() { val lock = AutoClosableReentrantLock() - assertFalse(lock.isLocked) + assertFalse(lock.isLockAllocated) + lock.acquire().use {} + assertTrue(lock.isLockAllocated) } @Test From dee0b4984b7c0e850c1cff610ffd34b5d4b52a0e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 2 Jul 2026 10:45:26 +0200 Subject: [PATCH 4/4] perf(core): Return the lock itself as the lifecycle token (JAVA-588) Every acquire() allocated a fresh lifecycle token, which is per-use garbage on every lock acquisition forever, not just at init. The token was stateless apart from its lock reference, so AutoClosableReentrantLock now implements ISentryLifecycleToken itself and acquire() returns this, making the steady-state acquire/close path allocation-free. Semantics are unchanged: try-with-resources closes once per acquire, so reentrant acquires stay balanced, and unlocking without holding the lock still throws IllegalMonitorStateException. Co-Authored-By: Claude Fable 5 --- sentry/api/sentry.api | 3 +- .../util/AutoClosableReentrantLock.java | 30 ++++++++----------- .../util/AutoClosableReentrantLockTest.kt | 7 +++++ 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 59b8add2102..383ea92b116 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7589,9 +7589,10 @@ public abstract class io/sentry/transport/TransportResult { public static fun success ()Lio/sentry/transport/TransportResult; } -public final class io/sentry/util/AutoClosableReentrantLock { +public final class io/sentry/util/AutoClosableReentrantLock : io/sentry/ISentryLifecycleToken { public fun ()V public fun acquire ()Lio/sentry/ISentryLifecycleToken; + public fun close ()V } public final class io/sentry/util/CheckInUtils { diff --git a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java index e9c267fb734..cf53d860e08 100644 --- a/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java +++ b/sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java @@ -17,9 +17,13 @@ * so the eager allocation of a {@link ReentrantLock} (and its {@code AbstractQueuedSynchronizer}) * was pure GC and main-thread overhead. We keep a {@link ReentrantLock} rather than reverting to * {@code synchronized} to stay friendly to virtual threads (Loom), see #3715. + * + *

{@link #acquire()} returns this instance as the token, so the steady-state acquire/close path + * allocates nothing. Reentrant acquires stay balanced because try-with-resources calls {@link + * #close()} exactly once per acquire. */ @ApiStatus.Internal -public final class AutoClosableReentrantLock { +public final class AutoClosableReentrantLock implements ISentryLifecycleToken { private static final @NotNull AtomicReferenceFieldUpdater< AutoClosableReentrantLock, ReentrantLock> @@ -30,9 +34,13 @@ public final class AutoClosableReentrantLock { private volatile @Nullable ReentrantLock lock; public @NotNull ISentryLifecycleToken acquire() { - final @NotNull ReentrantLock theLock = getOrCreateLock(); - theLock.lock(); - return new AutoClosableReentrantLockLifecycleToken(theLock); + getOrCreateLock().lock(); + return this; + } + + @Override + public void close() { + Objects.requireNonNull(lock, "close() called before acquire()").unlock(); } private @NotNull ReentrantLock getOrCreateLock() { @@ -59,18 +67,4 @@ boolean isLocked() { boolean isLockAllocated() { return lock != null; } - - static final class AutoClosableReentrantLockLifecycleToken implements ISentryLifecycleToken { - - private final @NotNull ReentrantLock lock; - - AutoClosableReentrantLockLifecycleToken(final @NotNull ReentrantLock lock) { - this.lock = lock; - } - - @Override - public void close() { - lock.unlock(); - } - } } diff --git a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt index d89c49836e9..943a2c2bf70 100644 --- a/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt +++ b/sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt @@ -6,6 +6,7 @@ import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertSame import kotlin.test.assertTrue class AutoClosableReentrantLockTest { @@ -16,6 +17,12 @@ class AutoClosableReentrantLockTest { assertFalse(lock.isLocked) } + @Test + fun `acquire returns the lock itself as the token, allocating nothing`() { + val lock = AutoClosableReentrantLock() + lock.acquire().use { token -> assertSame(lock, token) } + } + @Test fun `does not allocate the underlying lock until first acquire`() { val lock = AutoClosableReentrantLock()