Skip to content

Commit 21a827f

Browse files
sentry-junior[bot]romtsnclaude
authored
perf(replay): Use hash set for mask view class lookups (#6410)
* perf(replay): Use hash set for mask view class lookups maskViewClasses and unmaskViewClasses were backed by a CopyOnWriteArraySet, whose contains() is a linear scan. The view hierarchy traversal checks both sets for every superclass of every view on the main thread, so capture cost grew with the size of the configured class lists. Back both sets with a ConcurrentHashMap key set for O(1) lookups while keeping thread-safe, non-throwing iteration. Co-Authored-By: Roman Zavarnitsyn <roman.zavarnitsyn@sentry.io> * changelog Co-Authored-By: Roman Zavarnitsyn <roman.zavarnitsyn@sentry.io> * fix(replay): Ignore null mask view class names ConcurrentHashMap-backed sets reject null keys, while the previous CopyOnWriteArraySet accepted them. Hybrid SDKs (e.g. React Native via ReadableArray.getString) can pass null despite the @NotNull annotation, which would now throw during SDK init. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Roman Zavarnitsyn <roman.zavarnitsyn@sentry.io> Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 054686d commit 21a827f

4 files changed

Lines changed: 41 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
- Added `Environment` to the `Scope`, allowing you to overwrite the `Environment` set on the options ([#5772](https://github.com/getsentry/sentry-java/pull/5772))
88
- The scope `Environment` is also applied to the trace envelope header (DSC), logs, metrics and profiles, so all telemetry of a trace reports the same environment
99

10+
### Improvements
11+
12+
- Speed up Session Replay and screenshot masking for large `maskViewClasses`/`unmaskViewClasses` lists by using constant-time class lookups ([#6410](https://github.com/getsentry/sentry-java/pull/6410))
13+
1014
### Fixes
1115

1216
- Keep UI profiling running when `startProfiler` is called before a requested stop takes effect at the end of the current chunk ([#6291](https://github.com/getsentry/sentry-java/pull/6291))

‎sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import java.util.concurrent.ScheduledExecutorService
4646
import java.util.concurrent.TimeUnit.MILLISECONDS
4747
import java.util.concurrent.TimeUnit.SECONDS
4848
import kotlin.test.Test
49-
import kotlin.test.assertContentEquals
5049
import kotlin.test.assertEquals
5150
import kotlin.test.assertFalse
5251
import kotlin.test.assertNull
@@ -696,8 +695,8 @@ class SessionCaptureStrategyTest {
696695
assertEquals(true, optionsEvent[0].optionsPayload["maskAllText"])
697696
assertEquals(false, optionsEvent[0].optionsPayload["maskAllImages"])
698697
assertEquals("high", optionsEvent[0].optionsPayload["quality"])
699-
assertContentEquals(
700-
listOf(
698+
assertThat(optionsEvent[0].optionsPayload["maskedViewClasses"] as Collection<*>)
699+
.containsExactly(
701700
"android.widget.TextView",
702701
"android.webkit.WebView",
703702
"android.widget.VideoView",
@@ -706,13 +705,9 @@ class SessionCaptureStrategyTest {
706705
"com.google.android.exoplayer2.ui.PlayerView",
707706
"com.google.android.exoplayer2.ui.StyledPlayerView",
708707
"my.custom.View",
709-
),
710-
optionsEvent[0].optionsPayload["maskedViewClasses"] as Collection<*>,
711-
)
712-
assertContentEquals(
713-
listOf("android.widget.ImageView"),
714-
optionsEvent[0].optionsPayload["unmaskedViewClasses"] as Collection<*>,
715-
)
708+
)
709+
assertThat(optionsEvent[0].optionsPayload["unmaskedViewClasses"] as Collection<*>)
710+
.containsExactly("android.widget.ImageView")
716711
},
717712
)
718713
}

‎sentry/src/main/java/io/sentry/SentryMaskingOptions.java‎

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

3+
import java.util.Collections;
34
import java.util.Set;
4-
import java.util.concurrent.CopyOnWriteArraySet;
5+
import java.util.concurrent.ConcurrentHashMap;
56
import org.jetbrains.annotations.ApiStatus;
67
import org.jetbrains.annotations.NotNull;
78
import org.jetbrains.annotations.Nullable;
@@ -32,7 +33,7 @@ public abstract class SentryMaskingOptions {
3233
*
3334
* <p>Default is empty.
3435
*/
35-
protected Set<String> maskViewClasses = new CopyOnWriteArraySet<>();
36+
protected Set<String> maskViewClasses = Collections.newSetFromMap(new ConcurrentHashMap<>());
3637

3738
/**
3839
* Ignore all views with the specified class names from masking. The class name is the fully
@@ -44,7 +45,7 @@ public abstract class SentryMaskingOptions {
4445
*
4546
* <p>Default is empty.
4647
*/
47-
protected Set<String> unmaskViewClasses = new CopyOnWriteArraySet<>();
48+
protected Set<String> unmaskViewClasses = Collections.newSetFromMap(new ConcurrentHashMap<>());
4849

4950
/** The class name of the view container that masks all of its children. */
5051
protected @Nullable String maskViewContainerClass = null;
@@ -92,6 +93,10 @@ public Set<String> getMaskViewClasses() {
9293
}
9394

9495
public void addMaskViewClass(final @NotNull String className) {
96+
// Hybrid SDKs may pass null despite the annotation, and the backing set rejects null keys
97+
if (className == null) {
98+
return;
99+
}
95100
this.maskViewClasses.add(className);
96101
this.unmaskViewClasses.remove(className);
97102
}
@@ -102,6 +107,10 @@ public Set<String> getUnmaskViewClasses() {
102107
}
103108

104109
public void addUnmaskViewClass(final @NotNull String className) {
110+
// Hybrid SDKs may pass null despite the annotation, and the backing set rejects null keys
111+
if (className == null) {
112+
return;
113+
}
105114
this.unmaskViewClasses.add(className);
106115
this.maskViewClasses.remove(className);
107116
}

‎sentry/src/test/java/io/sentry/SentryMaskingOptionsTest.kt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,26 @@ class SentryMaskingOptionsTest {
4040
assertTrue(options.unmaskViewClasses.contains("com.example.MyView"))
4141
}
4242

43+
@Test
44+
fun `addMaskViewClass ignores null`() {
45+
val options = TestMaskingOptions()
46+
SentryMaskingOptions::class
47+
.java
48+
.getMethod("addMaskViewClass", String::class.java)
49+
.invoke(options, null)
50+
assertTrue(options.maskViewClasses.isEmpty())
51+
}
52+
53+
@Test
54+
fun `addUnmaskViewClass ignores null`() {
55+
val options = TestMaskingOptions()
56+
SentryMaskingOptions::class
57+
.java
58+
.getMethod("addUnmaskViewClass", String::class.java)
59+
.invoke(options, null)
60+
assertTrue(options.unmaskViewClasses.isEmpty())
61+
}
62+
4363
@Test
4464
fun `setMaskAllText true adds TextView to maskViewClasses`() {
4565
val options = TestMaskingOptions()

0 commit comments

Comments
 (0)