From a602579e5008e81b67596a3c8155163fa90c14f7 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 17 Jun 2021 14:55:21 +0100 Subject: [PATCH 1/2] perf: optimize metadata implementation --- CHANGELOG.md | 3 ++ .../main/java/com/bugsnag/android/Client.java | 6 ++- .../main/java/com/bugsnag/android/Metadata.kt | 45 +++++++------------ .../MetadataConcurrentModificationTest.kt | 2 +- .../bugsnag/android/MetadataDeserializer.java | 6 ++- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 424a5bf981..f3b8991c47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ * Optimize implementation of internal state change observers [#1274](https://github.com/bugsnag/bugsnag-android/pull/1274) +* Optimize metadata implementation by reducing type casts + [#1277](https://github.com/bugsnag/bugsnag-android/pull/1277) + ## 5.9.4 (2021-05-26) * Unity: add methods for setting autoNotify and autoDetectAnrs diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index 39c4101f95..0586c3ebea 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.RejectedExecutionException; /** @@ -865,9 +866,12 @@ public Object getMetadata(@NonNull String section, @NonNull String key) { } } + // cast map to retain original signature until next major version bump, as this + // method signature is used by Unity/React native @NonNull + @SuppressWarnings({"unchecked", "rawtypes"}) Map getMetadata() { - return metadataState.getMetadata().toMap(); + return (Map) metadataState.getMetadata().toMap(); } /** diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt index a2e07ae2d1..40ad168746 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt @@ -12,7 +12,7 @@ import java.util.concurrent.ConcurrentHashMap * Diagnostic information is presented on your Bugsnag dashboard in tabs. */ internal data class Metadata @JvmOverloads constructor( - internal val store: ConcurrentHashMap = ConcurrentHashMap() + internal val store: MutableMap> = ConcurrentHashMap() ) : JsonStream.Streamable, MetadataAware { val jsonStreamer: ObjectJsonStreamer = ObjectJsonStreamer() @@ -38,12 +38,9 @@ internal data class Metadata @JvmOverloads constructor( if (value == null) { clearMetadata(section, key) } else { - var tab = store[section] - if (tab !is MutableMap<*, *>) { - tab = ConcurrentHashMap() - store[section] = tab - } - insertValue(tab as MutableMap, key, value) + val tab = store[section] ?: ConcurrentHashMap() + store[section] = tab + insertValue(tab, key, value) } } @@ -52,7 +49,7 @@ internal data class Metadata @JvmOverloads constructor( // only merge if both the existing and new value are maps val existingValue = map[key] - if (obj is MutableMap<*, *> && existingValue is MutableMap<*, *>) { + if (existingValue != null && obj is Map<*, *>) { val maps = listOf(existingValue as Map, newValue as Map) obj = mergeMaps(maps) } @@ -65,49 +62,41 @@ internal data class Metadata @JvmOverloads constructor( override fun clearMetadata(section: String, key: String) { val tab = store[section] + tab?.remove(key) - if (tab is MutableMap<*, *>) { - tab.remove(key) - - if (tab.isEmpty()) { - store.remove(section) - } + if (tab?.isEmpty() == true) { + store.remove(section) } } override fun getMetadata(section: String): Map? { - return store[section] as (Map?) + return store[section] } override fun getMetadata(section: String, key: String): Any? { - return when (val tab = store[section]) { - is Map<*, *> -> (tab as Map?)!![key] - else -> tab - } + return getMetadata(section)?.get(key) } - fun toMap(): ConcurrentHashMap { - val hashMap = ConcurrentHashMap(store) + fun toMap(): MutableMap> { + val copy = ConcurrentHashMap(store) // deep copy each section store.entries.forEach { - if (it.value is ConcurrentHashMap<*, *>) { - hashMap[it.key] = ConcurrentHashMap(it.value as ConcurrentHashMap<*, *>) - } + copy[it.key] = ConcurrentHashMap(it.value) } - return hashMap + return copy } companion object { fun merge(vararg data: Metadata): Metadata { val stores = data.map { it.toMap() } val redactKeys = data.flatMap { it.jsonStreamer.redactedKeys } - val newMeta = Metadata(mergeMaps(stores)) + val newMeta = Metadata(mergeMaps(stores) as MutableMap>) newMeta.redactedKeys = redactKeys.toSet() return newMeta } - internal fun mergeMaps(data: List>): ConcurrentHashMap { + internal fun mergeMaps(data: List>): MutableMap { val keys = data.flatMap { it.keys }.toSet() val result = ConcurrentHashMap() @@ -120,7 +109,7 @@ internal data class Metadata @JvmOverloads constructor( } private fun getMergeValue( - result: ConcurrentHashMap, + result: MutableMap, key: String, map: Map ) { diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt index cbe12752d2..2db90f9de3 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt @@ -16,7 +16,7 @@ internal class MetadataConcurrentModificationTest { repeat(100) { count -> assertNotNull(metadata.toMap()) executor.execute { - metadata.store["$count"] = count + metadata.store["$count"] = mutableMapOf(Pair("$count", count)) } } } diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java index 1aebf3bc16..766835681d 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java @@ -6,7 +6,11 @@ class MetadataDeserializer implements MapDeserializer { @Override public Metadata deserialize(Map map) { - ConcurrentHashMap store = new ConcurrentHashMap<>(map); + // cast map to retain original signature until next major version bump, as this + // method signature is used by Unity/React native + @SuppressWarnings({"unchecked", "rawtypes"}) + Map> data = (Map) map; + ConcurrentHashMap> store = new ConcurrentHashMap<>(data); return new Metadata(store); } } From 8523f72e20d9980745d34555ca0ee3526691ded2 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Fri, 18 Jun 2021 09:36:19 +0100 Subject: [PATCH 2/2] Update bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt Co-authored-by: Jason --- .../src/main/java/com/bugsnag/android/Metadata.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt index 40ad168746..4dc8a51668 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt @@ -64,7 +64,7 @@ internal data class Metadata @JvmOverloads constructor( val tab = store[section] tab?.remove(key) - if (tab?.isEmpty() == true) { + if (tab.isNullOrEmpty()) { store.remove(section) } }