Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer the registration of thermal and power save mode listners to background thread #548

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Defer the registration of thermal and power save mode listners to bac…
…kground thread
  • Loading branch information
bidetofevil committed Mar 12, 2024
commit 2acccbffcd54a5ffcc3b300ab61acc5a31c3990f
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
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
Expand All @@ -18,7 +20,7 @@
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 {
Expand All @@ -31,20 +33,26 @@

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

Check warning on line 53 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/powersave/EmbracePowerSaveModeService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/powersave/EmbracePowerSaveModeService.kt#L53

Added line #L53 was not covered by tests
)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@
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>()
Expand All @@ -25,10 +29,23 @@
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)

Check warning on line 44 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt#L44

Added line #L44 was not covered by tests
}
pm.addThermalStatusListener(executor, thermalStatusListener)
}
}
}
}

Expand All @@ -55,9 +72,10 @@
override fun getCapturedData(): List<ThermalState> = thermalStates

override fun close() {
pm?.let {
val pm = powerManager

Check warning on line 75 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt#L75

Added line #L75 was not covered by tests
if (pm != null) {
logger.logDeveloper("ThermalStatusService", "Removing thermal status listener")
it.removeThermalStatusListener(thermalStatusListener)
pm.removeThermalStatusListener(thermalStatusListener)

Check warning on line 78 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/thermalstate/EmbraceThermalStatusService.kt#L78

Added line #L78 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -114,7 +118,7 @@ internal class DataCaptureServiceModuleImpl @JvmOverloads constructor(
coreModule.context,
backgroundWorker,
initModule.clock,
systemServiceModule.powerManager
powerManagerProvider
)
} else {
NoOpPowerSaveModeService()
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ internal class EmbracePowerSaveModeServiceTest {
service = EmbracePowerSaveModeService(
context,
worker,
fakeClock,
powerManager
)
fakeClock
) { powerManager }
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,11 +18,10 @@ internal class EmbraceThermalStatusServiceTest {
@Before
fun setUp() {
service = EmbraceThermalStatusService(
MoreExecutors.directExecutor(),
BackgroundWorker(BlockableExecutorService()),
{ 0 },
InternalEmbraceLogger(),
mockPowerManager()
)
InternalEmbraceLogger()
) { mockPowerManager() }
}

@Test
Expand Down
Loading