From a408a2620a8f901ef943b4066918c6b529b8a19f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Wed, 23 Jun 2021 17:13:08 +0100 Subject: [PATCH 1/3] perf: cache results from PackageManager --- CHANGELOG.md | 3 +++ .../com/bugsnag/android/BugsnagTestUtils.java | 2 +- .../bugsnag/android/LastRunInfoStoreTest.kt | 4 +++- .../com/bugsnag/android/AppDataCollector.kt | 4 ++-- .../android/internal/ImmutableConfig.kt | 18 ++++++++++++++---- .../android/AppMetadataSerializationTest.kt | 6 +++--- .../java/com/bugsnag/android/TestData.java | 4 +++- 7 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ad3d144dd..d43f37ed40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ * Include `app.binaryArch` in all events [#1287](https://github.com/bugsnag/bugsnag-android/pull/1287) +* Cache results from PackageManager + [#1288](https://github.com/bugsnag/bugsnag-android/pull/1288) + ## 5.9.4 (2021-05-26) * Unity: add methods for setting autoNotify and autoDetectAnrs diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java index b9cc679b7c..92ac16e01e 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -90,7 +90,7 @@ static ImmutableConfig convert(Configuration config) { } catch (IOException ignored) { // swallow } - return ImmutableConfigKt.convertToImmutableConfig(config, null); + return ImmutableConfigKt.convertToImmutableConfig(config, null, null, null); } static Device generateDevice() { diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt index 30e0b21f29..e6b503264d 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt @@ -22,7 +22,9 @@ internal class LastRunInfoStoreTest { val config = convertToImmutableConfig( generateConfiguration().apply { persistenceDirectory = ApplicationProvider.getApplicationContext().cacheDir - } + }, + packageInfo = null, + appInfo = null ) file = File(config.persistenceDirectory, "last-run-info") file.delete() diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt index 26ce4cbeb9..f3da7e3641 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt @@ -23,8 +23,8 @@ internal class AppDataCollector( var codeBundleId: String? = null private val packageName: String = appContext.packageName - private var packageInfo = packageManager?.getPackageInfo(packageName, 0) - private var appInfo: ApplicationInfo? = packageManager?.getApplicationInfo(packageName, 0) + private var packageInfo = config.packageInfo + private var appInfo: ApplicationInfo? = config.appInfo private val bgWorkRestricted = isBackgroundWorkRestricted() private var binaryArch: String? = null diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index cbe8bfcbfb..5c434a7d32 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -2,6 +2,7 @@ package com.bugsnag.android.internal import android.content.Context import android.content.pm.ApplicationInfo +import android.content.pm.PackageInfo import android.content.pm.PackageManager import androidx.annotation.VisibleForTesting import com.bugsnag.android.BreadcrumbType @@ -47,7 +48,11 @@ data class ImmutableConfig( val maxPersistedEvents: Int, val maxPersistedSessions: Int, val persistenceDirectory: File, - val sendLaunchCrashesSynchronously: Boolean + val sendLaunchCrashesSynchronously: Boolean, + + // results cached here to avoid unnecessary lookups in Client. + val packageInfo: PackageInfo?, + val appInfo: ApplicationInfo? ) { @JvmName("getErrorApiDeliveryParams") @@ -118,9 +123,12 @@ data class ImmutableConfig( } } +@JvmOverloads internal fun convertToImmutableConfig( config: Configuration, - buildUuid: String? = null + buildUuid: String? = null, + packageInfo: PackageInfo? = null, + appInfo: ApplicationInfo? = null ): ImmutableConfig { val errorTypes = when { config.autoDetectErrors -> config.enabledErrorTypes.copy() @@ -151,7 +159,9 @@ internal fun convertToImmutableConfig( maxPersistedSessions = config.maxPersistedSessions, enabledBreadcrumbTypes = config.enabledBreadcrumbTypes?.toSet(), persistenceDirectory = config.persistenceDirectory!!, - sendLaunchCrashesSynchronously = config.sendLaunchCrashesSynchronously + sendLaunchCrashesSynchronously = config.sendLaunchCrashesSynchronously, + packageInfo = packageInfo, + appInfo = appInfo ) } @@ -208,7 +218,7 @@ internal fun sanitiseConfiguration( if (configuration.persistenceDirectory == null) { configuration.persistenceDirectory = appContext.cacheDir } - return convertToImmutableConfig(configuration, buildUuid) + return convertToImmutableConfig(configuration, buildUuid, packageInfo, appInfo) } internal const val RELEASE_STAGE_DEVELOPMENT = "development" diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt index 264fa20cbd..5c87ad95e7 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/AppMetadataSerializationTest.kt @@ -4,7 +4,7 @@ import android.app.ActivityManager import android.content.Context import android.content.pm.ApplicationInfo import android.content.pm.PackageManager -import com.bugsnag.android.BugsnagTestUtils.convert +import com.bugsnag.android.internal.convertToImmutableConfig import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized @@ -40,14 +40,14 @@ internal class AppMetadataSerializationTest { // populate metadata fields `when`(sessionTracker.contextActivity).thenReturn("MyActivity") - `when`(pm.getApplicationInfo(any(), anyInt())).thenReturn(ApplicationInfo()) `when`(pm.getApplicationLabel(any())).thenReturn("MyApp") + `when`(pm.getApplicationInfo(any(), anyInt())).thenReturn(ApplicationInfo()) // construct AppDataCollector object val appData = AppDataCollector( context, pm, - convert(config), + convertToImmutableConfig(config, null, null, ApplicationInfo()), sessionTracker, am, launchCrashTracker, diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java index 4928946d03..58bccc8f1b 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java @@ -34,7 +34,9 @@ static ImmutableConfig generateConfig() throws IOException { 32, 32, Files.createTempDirectory("foo").toFile(), - true + true, + null, + null ); } From 4a3bf6528781d5524e5a190c51c99f0e29f17503 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Thu, 24 Jun 2021 10:54:24 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Jason --- .../src/main/java/com/bugsnag/android/AppDataCollector.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt index f3da7e3641..e731d01d3b 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt @@ -23,8 +23,8 @@ internal class AppDataCollector( var codeBundleId: String? = null private val packageName: String = appContext.packageName - private var packageInfo = config.packageInfo - private var appInfo: ApplicationInfo? = config.appInfo + private var packageInfo get() = config.packageInfo + private var appInfo: ApplicationInfo? get() = config.appInfo private val bgWorkRestricted = isBackgroundWorkRestricted() private var binaryArch: String? = null From d5d8fa03d095110158b623b64e5faa6485b05000 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 24 Jun 2021 11:08:31 +0100 Subject: [PATCH 3/3] remove unnecessary fields --- .../src/main/java/com/bugsnag/android/AppDataCollector.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt index e731d01d3b..625f3f8b33 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppDataCollector.kt @@ -2,7 +2,6 @@ package com.bugsnag.android import android.app.ActivityManager import android.content.Context -import android.content.pm.ApplicationInfo import android.content.pm.PackageManager import android.os.Build import android.os.SystemClock @@ -23,14 +22,12 @@ internal class AppDataCollector( var codeBundleId: String? = null private val packageName: String = appContext.packageName - private var packageInfo get() = config.packageInfo - private var appInfo: ApplicationInfo? get() = config.appInfo private val bgWorkRestricted = isBackgroundWorkRestricted() private var binaryArch: String? = null private val appName = getAppName() private val releaseStage = config.releaseStage - private val versionName = config.appVersion ?: packageInfo?.versionName + private val versionName = config.appVersion ?: config.packageInfo?.versionName fun generateApp(): App = App(config, binaryArch, packageName, releaseStage, versionName, codeBundleId) @@ -131,7 +128,7 @@ internal class AppDataCollector( * AndroidManifest.xml */ private fun getAppName(): String? { - val copy = appInfo + val copy = config.appInfo return when { packageManager != null && copy != null -> { packageManager.getApplicationLabel(copy).toString()