Skip to content

Commit 105fd6d

Browse files
committed
fix(compose): address PR review feedback on LocalSentryScopes root span caching
Key scopes weakly in RootSpans so custom IScopes instances (and their cached parent spans) can be garbage collected once nothing else holds a reference, instead of being pinned for the lifetime of the root Composition. Also stop permanently caching a null parent span when no transaction is bound yet, so a later transaction on the same scopes is still picked up. Addresses review feedback from sentry-review-bot and cursor bugbot on #5838.
1 parent 5a43f30 commit 105fd6d

2 files changed

Lines changed: 17 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22

33
## Unreleased
44

5-
### Improvements
6-
7-
- Skip building Android manifest metadata debug log messages when debug logging is disabled, reducing allocations during SDK init ([#5790](https://github.com/getsentry/sentry-java/pull/5790))
8-
## Unreleased
9-
105
### Features
116

127
- Add `LocalSentryScopes` to `sentry-compose`, allowing `SentryTraced` to trace against a custom `IScopes` instance instead of always defaulting to `Sentry.getCurrentScopes()` ([#5838](https://github.com/getsentry/sentry-java/pull/5838))

‎sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import io.sentry.ISpan
1515
import io.sentry.Sentry
1616
import io.sentry.SpanOptions
1717
import io.sentry.compose.SentryModifier.sentryTag
18+
import java.util.WeakHashMap
1819

1920
private const val OP_PARENT_COMPOSITION = "ui.compose.composition"
2021
private const val OP_COMPOSE = "ui.compose"
@@ -67,10 +68,22 @@ private fun createRenderingParentSpan(scopes: IScopes): ImmutableHolder<ISpan?>
6768
)
6869

6970
private class RootSpans {
70-
val compositionSpans = HashMap<IScopes, ImmutableHolder<ISpan?>>()
71-
val renderingSpans = HashMap<IScopes, ImmutableHolder<ISpan?>>()
71+
// Weakly keyed so scopes instances that are no longer referenced elsewhere (e.g. a short-lived
72+
// custom scopes provided via LocalSentryScopes for a finished screen/session) can be garbage
73+
// collected instead of being pinned here for the lifetime of the root Composition.
74+
val compositionSpans = WeakHashMap<IScopes, ImmutableHolder<ISpan?>>()
75+
val renderingSpans = WeakHashMap<IScopes, ImmutableHolder<ISpan?>>()
7276
}
7377

78+
private fun getOrCreateParentSpan(
79+
map: MutableMap<IScopes, ImmutableHolder<ISpan?>>,
80+
scopes: IScopes,
81+
create: (IScopes) -> ImmutableHolder<ISpan?>,
82+
): ImmutableHolder<ISpan?> =
83+
// Only cache the holder once it actually contains a span; a null result (no transaction bound
84+
// to the scopes yet) is recomputed on the next call so a later transaction is still picked up.
85+
map[scopes] ?: create(scopes).also { if (it.item != null) map[scopes] = it }
86+
7487
// Cached once per Composition and shared by every SentryTraced call within it, mirroring the
7588
// old eagerly-computed `compositionLocalOf { ... }` default (which Compose resolves once and
7689
// reuses for every `.current` read that has no ancestor Provider, sibling or not). Keyed per
@@ -88,9 +101,9 @@ public fun SentryTraced(
88101
val scopes = LocalSentryScopes.current
89102
val rootSpans = LocalRootSpans.current
90103
val parentCompositionSpan =
91-
rootSpans.compositionSpans.getOrPut(scopes) { createCompositionParentSpan(scopes) }
104+
getOrCreateParentSpan(rootSpans.compositionSpans, scopes, ::createCompositionParentSpan)
92105
val parentRenderingSpan =
93-
rootSpans.renderingSpans.getOrPut(scopes) { createRenderingParentSpan(scopes) }
106+
getOrCreateParentSpan(rootSpans.renderingSpans, scopes, ::createRenderingParentSpan)
94107

95108
val compositionSpan =
96109
parentCompositionSpan.item?.startChild(OP_COMPOSE, tag)?.apply {

0 commit comments

Comments
 (0)