Skip to content

Commit ea2a517

Browse files
runningcodeclaude
andauthored
perf(core): Lazily allocate AutoClosableReentrantLock (JAVA-588) (#5643)
* 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) <noreply@anthropic.com> * changelog * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4414d9f commit ea2a517

4 files changed

Lines changed: 116 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- 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))
2222
- Start the frame metrics thread lazily on first collection instead of during SDK init ([#5641](https://github.com/getsentry/sentry-java/pull/5641))
2323
- 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))
24+
- 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))
2425

2526
## 8.46.0
2627

sentry/api/sentry.api

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7589,9 +7589,10 @@ public abstract class io/sentry/transport/TransportResult {
75897589
public static fun success ()Lio/sentry/transport/TransportResult;
75907590
}
75917591

7592-
public final class io/sentry/util/AutoClosableReentrantLock : java/util/concurrent/locks/ReentrantLock {
7592+
public final class io/sentry/util/AutoClosableReentrantLock : io/sentry/ISentryLifecycleToken {
75937593
public fun <init> ()V
75947594
public fun acquire ()Lio/sentry/ISentryLifecycleToken;
7595+
public fun close ()V
75957596
}
75967597

75977598
public final class io/sentry/util/CheckInUtils {
Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,70 @@
11
package io.sentry.util;
22

33
import io.sentry.ISentryLifecycleToken;
4+
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
45
import java.util.concurrent.locks.ReentrantLock;
6+
import org.jetbrains.annotations.ApiStatus;
57
import org.jetbrains.annotations.NotNull;
8+
import org.jetbrains.annotations.Nullable;
9+
import org.jetbrains.annotations.TestOnly;
610

7-
public final class AutoClosableReentrantLock extends ReentrantLock {
11+
/**
12+
* Hands out an {@link ISentryLifecycleToken} from {@link #acquire()} for use with
13+
* try-with-resources (replacing {@code synchronized} blocks).
14+
*
15+
* <p>The underlying {@link ReentrantLock} is created lazily on the first {@link #acquire()}. Many
16+
* SDK objects hold a lock but never contend on it (especially during {@code SentryAndroid.init}),
17+
* so the eager allocation of a {@link ReentrantLock} (and its {@code AbstractQueuedSynchronizer})
18+
* was pure GC and main-thread overhead. We keep a {@link ReentrantLock} rather than reverting to
19+
* {@code synchronized} to stay friendly to virtual threads (Loom), see #3715.
20+
*
21+
* <p>{@link #acquire()} returns this instance as the token, so the steady-state acquire/close path
22+
* allocates nothing. Reentrant acquires stay balanced because try-with-resources calls {@link
23+
* #close()} exactly once per acquire.
24+
*/
25+
@ApiStatus.Internal
26+
public final class AutoClosableReentrantLock implements ISentryLifecycleToken {
827

9-
private static final long serialVersionUID = -3283069816958445549L;
28+
private static final @NotNull AtomicReferenceFieldUpdater<
29+
AutoClosableReentrantLock, ReentrantLock>
30+
LOCK_UPDATER =
31+
AtomicReferenceFieldUpdater.newUpdater(
32+
AutoClosableReentrantLock.class, ReentrantLock.class, "lock");
1033

11-
public ISentryLifecycleToken acquire() {
12-
lock();
13-
return new AutoClosableReentrantLockLifecycleToken(this);
14-
}
34+
private volatile @Nullable ReentrantLock lock;
1535

16-
static final class AutoClosableReentrantLockLifecycleToken implements ISentryLifecycleToken {
36+
public @NotNull ISentryLifecycleToken acquire() {
37+
getOrCreateLock().lock();
38+
return this;
39+
}
1740

18-
private final @NotNull ReentrantLock lock;
41+
@Override
42+
public void close() {
43+
Objects.requireNonNull(lock, "close() called before acquire()").unlock();
44+
}
1945

20-
AutoClosableReentrantLockLifecycleToken(final @NotNull ReentrantLock lock) {
21-
this.lock = lock;
46+
private @NotNull ReentrantLock getOrCreateLock() {
47+
final @Nullable ReentrantLock existing = lock;
48+
if (existing != null) {
49+
return existing;
2250
}
23-
24-
@Override
25-
public void close() {
26-
lock.unlock();
51+
final @NotNull ReentrantLock candidate = new ReentrantLock();
52+
if (LOCK_UPDATER.compareAndSet(this, null, candidate)) {
53+
return candidate;
2754
}
55+
// The CAS can only fail because another thread installed its lock first, and the field is
56+
// never reset, so all callers end up contending on that same instance.
57+
return Objects.requireNonNull(lock, "lock must have been set by the winning thread");
58+
}
59+
60+
@TestOnly
61+
boolean isLocked() {
62+
final @Nullable ReentrantLock current = lock;
63+
return current != null && current.isLocked();
64+
}
65+
66+
@TestOnly
67+
boolean isLockAllocated() {
68+
return lock != null;
2869
}
2970
}

sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
package io.sentry.util
22

3+
import java.util.concurrent.CountDownLatch
4+
import java.util.concurrent.TimeUnit
5+
import java.util.concurrent.atomic.AtomicInteger
36
import kotlin.test.Test
7+
import kotlin.test.assertEquals
48
import kotlin.test.assertFalse
9+
import kotlin.test.assertSame
510
import kotlin.test.assertTrue
611

712
class AutoClosableReentrantLockTest {
@@ -11,4 +16,57 @@ class AutoClosableReentrantLockTest {
1116
lock.acquire().use { assertTrue(lock.isLocked) }
1217
assertFalse(lock.isLocked)
1318
}
19+
20+
@Test
21+
fun `acquire returns the lock itself as the token, allocating nothing`() {
22+
val lock = AutoClosableReentrantLock()
23+
lock.acquire().use { token -> assertSame(lock, token) }
24+
}
25+
26+
@Test
27+
fun `does not allocate the underlying lock until first acquire`() {
28+
val lock = AutoClosableReentrantLock()
29+
assertFalse(lock.isLockAllocated)
30+
lock.acquire().use {}
31+
assertTrue(lock.isLockAllocated)
32+
}
33+
34+
@Test
35+
fun `supports reentrant acquire from the same thread`() {
36+
val lock = AutoClosableReentrantLock()
37+
lock.acquire().use {
38+
lock.acquire().use { assertTrue(lock.isLocked) }
39+
assertTrue(lock.isLocked)
40+
}
41+
assertFalse(lock.isLocked)
42+
}
43+
44+
@Test
45+
fun `mutually excludes concurrent threads`() {
46+
val lock = AutoClosableReentrantLock()
47+
val inCriticalSection = AtomicInteger(0)
48+
val maxObserved = AtomicInteger(0)
49+
val start = CountDownLatch(1)
50+
val threadCount = 8
51+
val iterations = 1000
52+
val threads =
53+
(0 until threadCount).map {
54+
Thread {
55+
start.await()
56+
repeat(iterations) {
57+
lock.acquire().use {
58+
val current = inCriticalSection.incrementAndGet()
59+
maxObserved.accumulateAndGet(current, ::maxOf)
60+
inCriticalSection.decrementAndGet()
61+
}
62+
}
63+
}
64+
}
65+
threads.forEach(Thread::start)
66+
start.countDown()
67+
threads.forEach { it.join(TimeUnit.SECONDS.toMillis(10)) }
68+
69+
assertEquals(1, maxObserved.get())
70+
assertFalse(lock.isLocked)
71+
}
1472
}

0 commit comments

Comments
 (0)