From 510e2900b3b34684d68cbb7c1fd7a58664c3178c Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 25 Jun 2026 16:23:09 +0200 Subject: [PATCH 1/2] 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) --- .../benchmark/LockAllocationBenchmarkTest.kt | 84 +++++++++++++++++++ sentry/api/sentry.api | 2 +- .../util/AutoClosableReentrantLock.java | 51 +++++++++-- .../util/AutoClosableReentrantLockTest.kt | 49 +++++++++++ 4 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/LockAllocationBenchmarkTest.kt diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/LockAllocationBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/LockAllocationBenchmarkTest.kt new file mode 100644 index 00000000000..112c23f93ed --- /dev/null +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/LockAllocationBenchmarkTest.kt @@ -0,0 +1,84 @@ +package io.sentry.uitest.android.benchmark + +import android.content.Context +import android.os.Debug +import android.util.Log +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.util.AutoClosableReentrantLock +import java.io.File +import java.util.concurrent.locks.ReentrantLock +import kotlin.test.Test +import org.junit.runner.RunWith + +/** + * On-device A/B benchmark for JAVA-588 (lazy lock allocation in [AutoClosableReentrantLock]). + * + * Before the change `AutoClosableReentrantLock extends ReentrantLock`, so constructing each + * lock-bearing SDK object allocated a `ReentrantLock` (+ its `AbstractQueuedSynchronizer`) eagerly, + * even when the lock was never acquired during `SentryAndroid.init`. After the change the + * underlying `ReentrantLock` is created lazily on first `acquire()`. + * + * Each phase is wrapped in [Debug.startMethodTracing] so the produced ART `.trace` files can be + * loaded into Perfetto `trace_processor` to count `ReentrantLock.` / + * `AbstractQueuedSynchronizer` slices. btrace app tracing is broken on Pixel 3 / Android 12, which + * is why this uses ART method tracing. + * + * Not run in CI; it exists to quantify the SDK-init lock-allocation change on a real device. + */ +@RunWith(AndroidJUnit4::class) +class LockAllocationBenchmarkTest { + + private val tag = "LockAllocBench" + private val count = 10_000 + private val dir: File + get() = ApplicationProvider.getApplicationContext().getExternalFilesDir(null)!! + + /** + * Construction without ever acquiring: this is the common init case. OLD eagerly allocates a + * ReentrantLock per object; NEW allocates none. + */ + @Test + fun construct_withoutAcquire() { + val old = File(dir, "lockalloc_old") + Debug.startMethodTracing(old.absolutePath) + val oldSink = arrayOfNulls(count) + for (i in 0 until count) { + oldSink[i] = ReentrantLock() // what `extends ReentrantLock` allocated at construction + } + Debug.stopMethodTracing() + + val new = File(dir, "lockalloc_new") + Debug.startMethodTracing(new.absolutePath) + val newSink = arrayOfNulls(count) + for (i in 0 until count) { + newSink[i] = AutoClosableReentrantLock() // lazy: no ReentrantLock yet + } + Debug.stopMethodTracing() + + Log.i(tag, "CONSTRUCT count=$count old=eager-ReentrantLock new=lazy (see lockalloc_*.trace)") + // Keep the arrays reachable so the allocations are not optimized away. + Log.i(tag, "sinks ${oldSink.size}/${newSink.size}") + } + + /** acquire()/close throughput must be unchanged on the hot path (lock already created). */ + @Test + fun acquireThroughput() { + val iterations = 200_000 + val rounds = 6 + val lock = AutoClosableReentrantLock() + + fun time(): Long { + val start = System.nanoTime() + repeat(iterations) { lock.acquire().close() } + return System.nanoTime() - start + } + + repeat(2) { time() } // warmup + var total = 0L + for (round in 0 until rounds) { + total += time() + } + Log.i(tag, "ACQUIRE ${total / (rounds.toLong() * iterations)}ns/op") + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e9083350349..f6406402447 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 6ee6062c89f520ffdaf5cc3d37e6320bbbe8fbcc Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 25 Jun 2026 16:24:12 +0200 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2049c2b2543..a732ab8b6be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Improvements + +- 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.45.0 ### Features