Skip to content

Commit

Permalink
perf: optimize metadata implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed Jun 17, 2021
1 parent e4cf4bc commit af6e5c3
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 31 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
* Optimize execution of callbacks
[#1276](https://github.com/bugsnag/bugsnag-android/pull/1276)

* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Map;
import java.util.Observer;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.RejectedExecutionException;

/**
Expand Down Expand Up @@ -864,9 +865,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<String, Object> getMetadata() {
return metadataState.getMetadata().toMap();
return (Map) metadataState.getMetadata().toMap();
}

/**
Expand Down
45 changes: 17 additions & 28 deletions bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Any> = ConcurrentHashMap()
internal val store: MutableMap<String, MutableMap<String, Any>> = ConcurrentHashMap()
) : JsonStream.Streamable, MetadataAware {

val jsonStreamer: ObjectJsonStreamer = ObjectJsonStreamer()
Expand All @@ -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<Any, Any>()
store[section] = tab
}
insertValue(tab as MutableMap<String, Any>, key, value)
val tab = store[section] ?: ConcurrentHashMap()
store[section] = tab
insertValue(tab, key, value)
}
}

Expand All @@ -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<String, Any>, newValue as Map<String, Any>)
obj = mergeMaps(maps)
}
Expand All @@ -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<String, Any>? {
return store[section] as (Map<String, Any>?)
return store[section]
}

override fun getMetadata(section: String, key: String): Any? {
return when (val tab = store[section]) {
is Map<*, *> -> (tab as Map<String, Any>?)!![key]
else -> tab
}
return getMetadata(section)?.get(key)
}

fun toMap(): ConcurrentHashMap<String, Any> {
val hashMap = ConcurrentHashMap(store)
fun toMap(): MutableMap<String, MutableMap<String, Any>> {
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<String, MutableMap<String, Any>>)
newMeta.redactedKeys = redactKeys.toSet()
return newMeta
}

internal fun mergeMaps(data: List<Map<String, Any>>): ConcurrentHashMap<String, Any> {
internal fun mergeMaps(data: List<Map<String, Any>>): MutableMap<String, Any> {
val keys = data.flatMap { it.keys }.toSet()
val result = ConcurrentHashMap<String, Any>()

Expand All @@ -120,7 +109,7 @@ internal data class Metadata @JvmOverloads constructor(
}

private fun getMergeValue(
result: ConcurrentHashMap<String, Any>,
result: MutableMap<String, Any>,
key: String,
map: Map<String, Any>
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal class MetadataConcurrentModificationTest {
repeat(100) { count ->
assertNotNull(metadata.toMap())
executor.execute {
metadata.store["$count"] = count
metadata.store["$count"] = mutableMapOf<String, Any>(Pair("$count", count))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
class MetadataDeserializer implements MapDeserializer<Metadata> {
@Override
public Metadata deserialize(Map<String, Object> map) {
ConcurrentHashMap<String, Object> 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<String, Map<String, Object>> data = (Map) map;
ConcurrentHashMap<String, Map<String, Object>> store = new ConcurrentHashMap<>(data);
return new Metadata(store);
}
}

0 comments on commit af6e5c3

Please sign in to comment.