Update OverviewCommandHelper to use Executors.MAIN to reduce the percentage of missed frames
This change will reduce the increase in the amount of missed frames measured by crystalball. I believe coroutines launch is competing with Executors.MAIN for scheduling and running tasks in the main thread, thus the increase in missing frames.
Fix: 366077002
Flag: com.android.launcher3.enable_overview_command_helper_timeout
Test: OverviewCommandHelperTest
Test: android.platform.test.scenario.launcher.CloseApp3ButtonModeMicrobenchmark#testOpenLauncher
Test: atp:v2/android-crystalball-eng/health/microbench/launcher/main/launcher-action-suite
Change-Id: I4477879d4f065ec3883f2c3cb3ef044e973ce0cb
diff --git a/quickstep/src/com/android/quickstep/OverviewCommandHelper.kt b/quickstep/src/com/android/quickstep/OverviewCommandHelper.kt
index 461f963..e23947b 100644
--- a/quickstep/src/com/android/quickstep/OverviewCommandHelper.kt
+++ b/quickstep/src/com/android/quickstep/OverviewCommandHelper.kt
@@ -53,6 +53,7 @@
import com.android.systemui.shared.system.InteractionJankMonitorWrapper
import java.io.PrintWriter
import java.util.concurrent.ConcurrentLinkedDeque
+import java.util.concurrent.Executor
import kotlin.coroutines.resume
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.SupervisorJob
@@ -69,6 +70,7 @@
private val overviewComponentObserver: OverviewComponentObserver,
private val taskAnimationManager: TaskAnimationManager,
private val dispatcherProvider: DispatcherProvider = ProductionDispatchers,
+ private val uiExecutor: Executor = Executors.MAIN_EXECUTOR,
) {
private val coroutineScope = CoroutineScope(SupervisorJob() + dispatcherProvider.default)
@@ -85,7 +87,7 @@
get() = overviewComponentObserver.containerInterface
private val visibleRecentsView: RecentsView<*, *>?
- get() = containerInterface.getVisibleRecentsView<RecentsView<*, *>>()
+ get() = containerInterface.getVisibleRecentsView()
/**
* Adds a command to be executed next, after all pending tasks are completed. Max commands that
@@ -105,11 +107,7 @@
if (commandQueue.size == 1) {
Log.d(TAG, "execute: $command - queue size: ${commandQueue.size}")
- if (enableOverviewCommandHelperTimeout()) {
- coroutineScope.launch(dispatcherProvider.main) { processNextCommand() }
- } else {
- Executors.MAIN_EXECUTOR.execute { processNextCommand() }
- }
+ uiExecutor.execute { processNextCommand() }
} else {
Log.d(TAG, "not executed: $command - queue size: ${commandQueue.size}")
}
diff --git a/quickstep/tests/multivalentTests/src/com/android/quickstep/OverviewCommandHelperTest.kt b/quickstep/tests/multivalentTests/src/com/android/quickstep/OverviewCommandHelperTest.kt
index 0ae710f..b0db737 100644
--- a/quickstep/tests/multivalentTests/src/com/android/quickstep/OverviewCommandHelperTest.kt
+++ b/quickstep/tests/multivalentTests/src/com/android/quickstep/OverviewCommandHelperTest.kt
@@ -41,7 +41,6 @@
import org.junit.runner.RunWith
import org.mockito.Mockito.doAnswer
import org.mockito.Mockito.spy
-import org.mockito.Mockito.`when`
import org.mockito.kotlin.any
import org.mockito.kotlin.mock
@@ -56,10 +55,12 @@
private val testScope = TestScope(dispatcher)
private var pendingCallbacksWithDelays = mutableListOf<Long>()
+ private lateinit var pendingCommandsToExecute: MutableList<Runnable>
@Suppress("UNCHECKED_CAST")
@Before
fun setup() {
+ pendingCommandsToExecute = mutableListOf()
setFlagsRule.setFlags(true, Flags.FLAG_ENABLE_OVERVIEW_COMMAND_HELPER_TIMEOUT)
sut =
@@ -68,7 +69,8 @@
touchInteractionService = mock(),
overviewComponentObserver = mock(),
taskAnimationManager = mock(),
- dispatcherProvider = TestDispatcherProvider(dispatcher)
+ dispatcherProvider = TestDispatcherProvider(dispatcher),
+ uiExecutor = { runnable -> pendingCommandsToExecute += runnable },
)
)
@@ -94,12 +96,21 @@
pendingCallbacksWithDelays.add(delayInMillis)
}
+ /**
+ * This function runs all the pending commands from the Executor for testing purposes. Replacing
+ * the uiExecutor allows the test to execute the command queue manually, making it possible to
+ * assert each state of the commands in the queue individually.
+ */
+ private fun executePendingCommands() = pendingCommandsToExecute.forEach { it.run() }
+
@Test
fun whenFirstCommandIsAdded_executeCommandImmediately() =
testScope.runTest {
// Add command to queue
val commandInfo: CommandInfo = sut.addCommand(CommandType.HOME)!!
assertThat(commandInfo.status).isEqualTo(CommandStatus.IDLE)
+ executePendingCommands()
+ assertThat(commandInfo.status).isEqualTo(CommandStatus.PROCESSING)
runCurrent()
assertThat(commandInfo.status).isEqualTo(CommandStatus.COMPLETED)
}
@@ -114,7 +125,7 @@
val commandInfo: CommandInfo = sut.addCommand(commandType)!!
assertThat(commandInfo.status).isEqualTo(CommandStatus.IDLE)
- runCurrent()
+ executePendingCommands()
assertThat(commandInfo.status).isEqualTo(CommandStatus.PROCESSING)
advanceTimeBy(200L)
@@ -135,12 +146,14 @@
val commandInfo2: CommandInfo = sut.addCommand(commandType2)!!
assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)
- runCurrent()
+ executePendingCommands()
assertThat(commandInfo1.status).isEqualTo(CommandStatus.PROCESSING)
assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)
advanceTimeBy(101L)
assertThat(commandInfo1.status).isEqualTo(CommandStatus.COMPLETED)
+
+ executePendingCommands()
assertThat(commandInfo2.status).isEqualTo(CommandStatus.PROCESSING)
advanceTimeBy(101L)
@@ -161,12 +174,14 @@
val commandInfo2: CommandInfo = sut.addCommand(commandType2)!!
assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)
- runCurrent()
+ executePendingCommands()
assertThat(commandInfo1.status).isEqualTo(CommandStatus.PROCESSING)
assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)
advanceTimeBy(QUEUE_TIMEOUT)
assertThat(commandInfo1.status).isEqualTo(CommandStatus.CANCELED)
+
+ executePendingCommands()
assertThat(commandInfo2.status).isEqualTo(CommandStatus.PROCESSING)
advanceTimeBy(101)