Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7eae20bc authored by Jordan Silva's avatar Jordan Silva
Browse files

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
parent 0bf39fdf
Loading
Loading
Loading
Loading
+4 −6
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ import com.android.systemui.shared.recents.model.ThumbnailData
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 @@ constructor(
    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 @@ constructor(
        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 @@ constructor(

        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}")
        }
+20 −5
Original line number Diff line number Diff line
@@ -41,7 +41,6 @@ import org.junit.Test
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 @@ class OverviewCommandHelperTest {
    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 @@ class OverviewCommandHelperTest {
                    touchInteractionService = mock(),
                    overviewComponentObserver = mock(),
                    taskAnimationManager = mock(),
                    dispatcherProvider = TestDispatcherProvider(dispatcher)
                    dispatcherProvider = TestDispatcherProvider(dispatcher),
                    uiExecutor = { runnable -> pendingCommandsToExecute += runnable },
                )
            )

@@ -94,12 +96,21 @@ class OverviewCommandHelperTest {
        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 @@ class OverviewCommandHelperTest {
            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 @@ class OverviewCommandHelperTest {
            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 @@ class OverviewCommandHelperTest {
            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)