Merge pull request #5598 from getsentry/perf/sdk-overhead-reduction-breadcrumb-lazy-data

perf(core): [SDK Overhead Reduction 7] Lazily allocate Breadcrumb data

Author: adinauer

CHANGELOG.md

@@ -4,9 +4,13 @@
 
 ### Internal
 
+<<<<<<< perf/sdk-overhead-reduction-breadcrumb-lazy-data
+- Reduce breadcrumb allocation overhead by creating the `Breadcrumb` data map only when data is added. ([#5598](https://github.com/getsentry/sentry-java/pull/5598))
+=======
 - Reduce JSON serialization overhead by lowering the initial `JsonWriter` nesting stack size while preserving on-demand growth. ([#5591](https://github.com/getsentry/sentry-java/pull/5591))
 - Reduce timestamp helper overhead by replacing unnecessary `Calendar` usage in `DateUtils` with direct `Date` creation. ([#5589](https://github.com/getsentry/sentry-java/pull/5589))
 - Reduce Android startup overhead by using the default timezone directly on older devices or when no timezone info is available in the locale. ([#5587](https://github.com/getsentry/sentry-java/pull/5587))
+>>>>>>> perf/sdk-overhead-reduction
 
 ## 8.45.0
 

sentry/src/main/java/io/sentry/Breadcrumb.java

@@ -34,8 +34,10 @@ public final class Breadcrumb implements JsonUnknown, JsonSerializable, Comparab
   /** The type of breadcrumb. */
   private @Nullable String type;
 
+  private static final @NotNull Map<String, @NotNull Object> EMPTY_DATA = Collections.emptyMap();
+
   /** Data associated with this breadcrumb. */
-  private @NotNull Map<String, @NotNull Object> data = new ConcurrentHashMap<>();
+  private volatile @NotNull Map<String, @NotNull Object> data = EMPTY_DATA;
 
   /** Dotted strings that indicate what the crumb is or where it comes from. */
   private @Nullable String category;
@@ -78,9 +80,11 @@ public Breadcrumb(final long timestamp) {
     this.type = breadcrumb.type;
     this.category = breadcrumb.category;
     this.origin = breadcrumb.origin;
-    final Map<String, Object> dataClone = CollectionUtils.newConcurrentHashMap(breadcrumb.data);
-    if (dataClone != null) {
-      this.data = dataClone;
+    if (!breadcrumb.data.isEmpty()) {
+      final Map<String, Object> dataClone = CollectionUtils.newConcurrentHashMap(breadcrumb.data);
+      if (dataClone != null) {
+        this.data = dataClone;
+      }
     }
     this.unknown = CollectionUtils.newConcurrentHashMap(breadcrumb.unknown);
     this.level = breadcrumb.level;
@@ -100,7 +104,7 @@ public static Breadcrumb fromMap(
     @NotNull Date timestamp = DateUtils.getCurrentDateTime();
     String message = null;
     String type = null;
-    @NotNull Map<String, Object> data = new ConcurrentHashMap<>();
+    Map<String, Object> data = null;
     String category = null;
     String origin = null;
     SentryLevel level = null;
@@ -129,6 +133,9 @@ public static Breadcrumb fromMap(
           if (untypedData != null) {
             for (Map.Entry<Object, Object> dataEntry : untypedData.entrySet()) {
               if (dataEntry.getKey() instanceof String && dataEntry.getValue() != null) {
+                if (data == null) {
+                  data = new ConcurrentHashMap<>();
+                }
                 data.put((String) dataEntry.getKey(), dataEntry.getValue());
               } else {
                 options
@@ -166,7 +173,9 @@ public static Breadcrumb fromMap(
     final Breadcrumb breadcrumb = new Breadcrumb(timestamp);
     breadcrumb.message = message;
     breadcrumb.type = type;
-    breadcrumb.data = data;
+    if (data != null) {
+      breadcrumb.data = data;
+    }
     breadcrumb.category = category;
     breadcrumb.origin = origin;
     breadcrumb.level = level;
@@ -494,7 +503,7 @@ public static Breadcrumb fromMap(
       breadcrumb.setData("view.tag", viewTag);
     }
     for (final Map.Entry<String, Object> entry : additionalData.entrySet()) {
-      breadcrumb.getData().put(entry.getKey(), entry.getValue());
+      breadcrumb.setData(entry.getKey(), entry.getValue());
     }
     breadcrumb.setLevel(SentryLevel.INFO);
     return breadcrumb;
@@ -598,6 +607,20 @@ public void setType(@Nullable String type) {
     this.type = type;
   }
 
+  private @NotNull Map<String, @NotNull Object> getOrCreateData() {
+    Map<String, @NotNull Object> currentData = data;
+    if (currentData == EMPTY_DATA) {
+      synchronized (this) {
+        currentData = data;
+        if (currentData == EMPTY_DATA) {
+          currentData = new ConcurrentHashMap<>();
+          data = currentData;
+        }
+      }
+    }
+    return currentData;
+  }
+
   /**
    * Returns the data map
    *
@@ -606,7 +629,7 @@ public void setType(@Nullable String type) {
   @ApiStatus.Internal
   @NotNull
   public Map<String, Object> getData() {
-    return data;
+    return getOrCreateData();
   }
 
   /**
@@ -636,7 +659,7 @@ public void setData(@Nullable String key, @Nullable Object value) {
     if (value == null) {
       removeData(key);
     } else {
-      data.put(key, value);
+      getOrCreateData().put(key, value);
     }
   }
 
@@ -649,7 +672,10 @@ public void removeData(@Nullable String key) {
     if (key == null) {
       return;
     }
-    data.remove(key);
+    final Map<String, @NotNull Object> currentData = data;
+    if (currentData != EMPTY_DATA) {
+      currentData.remove(key);
+    }
   }
 
   /**
@@ -859,7 +885,7 @@ public static final class Deserializer implements JsonDeserializer<Breadcrumb> {
       @NotNull Date timestamp = DateUtils.getCurrentDateTime();
       String message = null;
       String type = null;
-      @NotNull Map<String, Object> data = new ConcurrentHashMap<>();
+      Map<String, Object> data = null;
       String category = null;
       String origin = null;
       SentryLevel level = null;
@@ -884,7 +910,7 @@ public static final class Deserializer implements JsonDeserializer<Breadcrumb> {
             Map<String, Object> deserializedData =
                 CollectionUtils.newConcurrentHashMap(
                     (Map<String, Object>) reader.nextObjectOrNull());
-            if (deserializedData != null) {
+            if (deserializedData != null && !deserializedData.isEmpty()) {
               data = deserializedData;
             }
             break;
@@ -913,7 +939,9 @@ public static final class Deserializer implements JsonDeserializer<Breadcrumb> {
       Breadcrumb breadcrumb = new Breadcrumb(timestamp);
       breadcrumb.message = message;
       breadcrumb.type = type;
-      breadcrumb.data = data;
+      if (data != null) {
+        breadcrumb.data = data;
+      }
       breadcrumb.category = category;
       breadcrumb.origin = origin;
       breadcrumb.level = level;

sentry/src/test/java/io/sentry/BreadcrumbTest.kt

@@ -1,6 +1,9 @@
 package io.sentry
 
 import java.util.Date
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.Executors
+import java.util.concurrent.TimeUnit
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -329,6 +332,39 @@ class BreadcrumbTest {
     breadcrumb.removeData(null)
   }
 
+  @Test
+  fun `getData returns mutable map for new breadcrumb`() {
+    val breadcrumb = Breadcrumb()
+
+    breadcrumb.data["k"] = "v"
+
+    assertEquals("v", breadcrumb.getData("k"))
+  }
+
+  @Test
+  fun `concurrent first writes keep all data entries`() {
+    val breadcrumb = Breadcrumb()
+    val count = 32
+    val executor = Executors.newFixedThreadPool(count)
+    val start = CountDownLatch(1)
+    val futures =
+      (0 until count).map { index ->
+        executor.submit {
+          start.await()
+          breadcrumb.setData("key-$index", index)
+        }
+      }
+
+    start.countDown()
+    futures.forEach { it.get(5, TimeUnit.SECONDS) }
+    executor.shutdown()
+
+    assertEquals(count, breadcrumb.data.size)
+    for (index in 0 until count) {
+      assertEquals(index, breadcrumb.data["key-$index"])
+    }
+  }
+
   class TestKey(val id: Long) {
     override fun toString(): String = id.toString()
   }