Defer the registration of thermal and power save mode listners to bac…

…kground thread (#548) ## Goal The services require making a system call to initialize the PowerManager, which can be expensive and take an unpredictable amount of time. Doing that on the main thread on startup isn't ideal, defer the init to a background thread. ## Testing Covered by existing tests
embrace-io · Mar 12, 2024 · fb350b3 · fb350b3
2 parents 17a2c05 + 2acccbf
commit fb350b3
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 35 deletions.
diff --git a/.../main/java/io/embrace/android/embracesdk/capture/powersave/EmbracePowerSaveModeService.kt b/.../main/java/io/embrace/android/embracesdk/capture/powersave/EmbracePowerSaveModeService.kt
@@ -6,7 +6,9 @@ import android.content.Intent
 import android.content.IntentFilter
 import android.os.PowerManager
 import android.os.PowerManager.ACTION_POWER_SAVE_MODE_CHANGED
+import io.embrace.android.embracesdk.internal.Systrace
 import io.embrace.android.embracesdk.internal.clock.Clock
+import io.embrace.android.embracesdk.internal.utils.Provider
 import io.embrace.android.embracesdk.logging.InternalStaticEmbraceLogger
 import io.embrace.android.embracesdk.logging.InternalStaticEmbraceLogger.Companion.logDebug
 import io.embrace.android.embracesdk.logging.InternalStaticEmbraceLogger.Companion.logDeveloper
@@ -18,7 +20,7 @@ internal class EmbracePowerSaveModeService(
  private val context: Context,
  private val backgroundWorker: BackgroundWorker,
  private val clock: Clock,
- private val powerManager: PowerManager?
+ powerManagerProvider: Provider<PowerManager?>
 ) : BroadcastReceiver(), PowerSaveModeService, ProcessStateListener {
 
  private companion object {
@@ -31,20 +33,26 @@ internal class EmbracePowerSaveModeService(
 
  private val powerSaveModeIntervals = mutableListOf<PowerChange>()
 
+ private val powerManager: PowerManager? by lazy(powerManagerProvider)
+
  init {
  registerPowerSaveModeReceiver()
  }
 
  private fun registerPowerSaveModeReceiver() {
  backgroundWorker.submit {
- try {
- context.registerReceiver(this, powerSaveIntentFilter)
- logDeveloper(tag, "registered power save mode changed")
- } catch (ex: Exception) {
- InternalStaticEmbraceLogger.logError(
- "Failed to register: $tag broadcast receiver. Power save mode status will be unavailable.",
- ex
- )
+ Systrace.traceSynchronous("power-service-registration") {
+ try {
+ if (powerManager != null) {
+ context.registerReceiver(this, powerSaveIntentFilter)
+ logDeveloper(tag, "registered power save mode changed")
+ }
+ } catch (ex: Exception) {
+ InternalStaticEmbraceLogger.logError(
+ "Failed to register: $tag broadcast receiver. Power save mode status will be unavailable.",
+ ex
+ )
+ }
  }
  }
  }

diff --git a/...in/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt b/...in/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt
@@ -3,20 +3,24 @@ package io.embrace.android.embracesdk.capture.thermalstate
 import android.os.Build
 import android.os.PowerManager
 import androidx.annotation.RequiresApi
+import io.embrace.android.embracesdk.internal.Systrace
 import io.embrace.android.embracesdk.internal.clock.Clock
+import io.embrace.android.embracesdk.internal.utils.Provider
 import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
 import io.embrace.android.embracesdk.payload.ThermalState
+import io.embrace.android.embracesdk.worker.BackgroundWorker
+import io.embrace.android.embracesdk.worker.TaskPriority
 import java.util.LinkedList
 import java.util.concurrent.Executor
 
 private const val CAPTURE_LIMIT = 100
 
 @RequiresApi(Build.VERSION_CODES.Q)
 internal class EmbraceThermalStatusService(
- executor: Executor,
+ private val backgroundWorker: BackgroundWorker,
  private val clock: Clock,
  private val logger: InternalEmbraceLogger,
- private val pm: PowerManager?
+ powerManagerProvider: Provider<PowerManager?>
 ) : ThermalStatusService {
 
  private val thermalStates = LinkedList<ThermalState>()
@@ -25,10 +29,23 @@ internal class EmbraceThermalStatusService(
  handleThermalStateChange(it)
  }
 
+ private val powerManager: PowerManager? by lazy(powerManagerProvider)
+
  init {
- pm?.let {
- logger.logDeveloper("ThermalStatusService", "Adding thermal status listener")
- it.addThermalStatusListener(executor, thermalStatusListener)
+ backgroundWorker.submit(TaskPriority.LOW) {
+ Systrace.traceSynchronous("thermal-service-registration") {
+ val pm = powerManager
+ if (pm != null) {
+ logger.logDeveloper("ThermalStatusService", "Adding thermal status listener")
+ // Android API only accepts an executor. We don't want to directly expose those
+ // to everything in the codebase so we decorate the BackgroundWorker here as an
+ // alternative
+ val executor = Executor {
+ backgroundWorker.submit(runnable = it)
+ }
+ pm.addThermalStatusListener(executor, thermalStatusListener)
+ }
+ }
  }
  }
 
@@ -55,9 +72,10 @@ internal class EmbraceThermalStatusService(
  override fun getCapturedData(): List<ThermalState> = thermalStates
 
  override fun close() {
- pm?.let {
+ val pm = powerManager
+ if (pm != null) {
  logger.logDeveloper("ThermalStatusService", "Removing thermal status listener")
- it.removeThermalStatusListener(thermalStatusListener)
+ pm.removeThermalStatusListener(thermalStatusListener)
  }
  }
 }
diff --git a/...oid-sdk/src/main/java/io/embrace/android/embracesdk/injection/DataCaptureServiceModule.kt b/...oid-sdk/src/main/java/io/embrace/android/embracesdk/injection/DataCaptureServiceModule.kt
@@ -25,7 +25,6 @@ import io.embrace.android.embracesdk.internal.utils.BuildVersionChecker
 import io.embrace.android.embracesdk.internal.utils.VersionChecker
 import io.embrace.android.embracesdk.worker.WorkerName
 import io.embrace.android.embracesdk.worker.WorkerThreadModule
-import java.util.concurrent.Executor
 
 /**
  * This modules provides services that capture data from within an application. It could be argued
@@ -92,6 +91,11 @@ internal class DataCaptureServiceModuleImpl @JvmOverloads constructor(
 
  private val backgroundWorker = workerThreadModule.backgroundWorker(WorkerName.BACKGROUND_REGISTRATION)
  private val configService = essentialServiceModule.configService
+ private val powerManagerProvider = {
+ Systrace.traceSynchronous("power-manager-load") {
+ systemServiceModule.powerManager
+ }
+ }
 
  override val memoryService: MemoryService by singleton {
  if (configService.autoDataCaptureBehavior.isMemoryServiceEnabled()) {
@@ -114,7 +118,7 @@ internal class DataCaptureServiceModuleImpl @JvmOverloads constructor(
  coreModule.context,
  backgroundWorker,
  initModule.clock,
- systemServiceModule.powerManager
+ powerManagerProvider
  )
  } else {
  NoOpPowerSaveModeService()
@@ -147,19 +151,12 @@ internal class DataCaptureServiceModuleImpl @JvmOverloads constructor(
  override val thermalStatusService: ThermalStatusService by singleton {
  Systrace.traceSynchronous("thermal-service-init") {
  if (configService.autoDataCaptureBehavior.isThermalStatusCaptureEnabled() && versionChecker.isAtLeast(Build.VERSION_CODES.Q)) {
- // Android API only accepts an executor. We don't want to directly expose those
- // to everything in the codebase so we decorate the BackgroundWorker here as an
- // alternative
  val backgroundWorker = workerThreadModule.backgroundWorker(WorkerName.BACKGROUND_REGISTRATION)
- val executor = Executor {
- backgroundWorker.submit(runnable = it)
- }
-
  EmbraceThermalStatusService(
- executor,
+ backgroundWorker,
  initModule.clock,
  coreModule.logger,
- systemServiceModule.powerManager
+ powerManagerProvider
  )
  } else {
  NoOpThermalStatusService()

diff --git a/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/EmbracePowerSaveModeServiceTest.kt b/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/EmbracePowerSaveModeServiceTest.kt
@@ -67,9 +67,8 @@ internal class EmbracePowerSaveModeServiceTest {
  service = EmbracePowerSaveModeService(
  context,
  worker,
- fakeClock,
- powerManager
- )
+ fakeClock
+ ) { powerManager }
  }
 
  /**

diff --git a/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/EmbraceThermalStatusServiceTest.kt b/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/EmbraceThermalStatusServiceTest.kt
@@ -1,11 +1,12 @@
 package io.embrace.android.embracesdk
 
 import android.os.PowerManager
-import com.google.common.util.concurrent.MoreExecutors
 import io.embrace.android.embracesdk.capture.thermalstate.EmbraceThermalStatusService
+import io.embrace.android.embracesdk.concurrency.BlockableExecutorService
 import io.embrace.android.embracesdk.fakes.system.mockPowerManager
 import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
 import io.embrace.android.embracesdk.payload.ThermalState
+import io.embrace.android.embracesdk.worker.BackgroundWorker
 import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
@@ -17,11 +18,10 @@ internal class EmbraceThermalStatusServiceTest {
  @Before
  fun setUp() {
  service = EmbraceThermalStatusService(
- MoreExecutors.directExecutor(),
+ BackgroundWorker(BlockableExecutorService()),
  { 0 },
- InternalEmbraceLogger(),
- mockPowerManager()
- )
+ InternalEmbraceLogger()
+ ) { mockPowerManager() }
  }
 
  @Test