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

Commit 3a4d67b9 authored by Jordan Silva's avatar Jordan Silva Committed by Android (Google) Code Review
Browse files

Revert "Update OverviewCommandHelper to use Executors.MAIN to reduce the... 

Revert "Update OverviewCommandHelper to use Executors.MAIN to reduce the percentage of missed frames"

This CL didn't improve the performance metrics significantly enough to use Executors.MAIN. b/366077002#comment12

This reverts commit 7eae20bc.
Reason for revert: 366077002

Change-Id: Ic92b83a65aef7f8cd5c00110fb1ab7343d4b12b6
parent 7eae20bc

quickstep/src/com/android/quickstep/OverviewCommandHelper.kt

+6 −4
Original line number Diff line number Diff line
@@ -53,7 +53,6 @@ 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
@@ -70,7 +69,6 @@ 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)
@@ -87,7 +85,7 @@ constructor( 
        get() = overviewComponentObserver.containerInterface



    private val visibleRecentsView: RecentsView<*, *>?

        get() = containerInterface.getVisibleRecentsView()

        get() = containerInterface.getVisibleRecentsView<RecentsView<*, *>>()



    /**

     * Adds a command to be executed next, after all pending tasks are completed. Max commands that
@@ -107,7 +105,11 @@ constructor( 


        if (commandQueue.size == 1) {

            Log.d(TAG, "execute: $command - queue size: ${commandQueue.size}")

            uiExecutor.execute { processNextCommand() }

            if (enableOverviewCommandHelperTimeout()) {

                coroutineScope.launch(dispatcherProvider.main) { processNextCommand() }

            } else {

                Executors.MAIN_EXECUTOR.execute { processNextCommand() }

            }

        } else {

            Log.d(TAG, "not executed: $command - queue size: ${commandQueue.size}")

        }

quickstep/tests/multivalentTests/src/com/android/quickstep/OverviewCommandHelperTest.kt

+5 −20
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ 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
@@ -55,12 +56,10 @@ 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 =
@@ -69,8 +68,7 @@ class OverviewCommandHelperTest { 
                    touchInteractionService = mock(),

                    overviewComponentObserver = mock(),

                    taskAnimationManager = mock(),

                    dispatcherProvider = TestDispatcherProvider(dispatcher),

                    uiExecutor = { runnable -> pendingCommandsToExecute += runnable },

                    dispatcherProvider = TestDispatcherProvider(dispatcher)

                )

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

        }
@@ -125,7 +114,7 @@ class OverviewCommandHelperTest { 
            val commandInfo: CommandInfo = sut.addCommand(commandType)!!

            assertThat(commandInfo.status).isEqualTo(CommandStatus.IDLE)



            executePendingCommands()

            runCurrent()

            assertThat(commandInfo.status).isEqualTo(CommandStatus.PROCESSING)



            advanceTimeBy(200L)
@@ -146,14 +135,12 @@ class OverviewCommandHelperTest { 
            val commandInfo2: CommandInfo = sut.addCommand(commandType2)!!

            assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)



            executePendingCommands()

            runCurrent()

            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)
@@ -174,14 +161,12 @@ class OverviewCommandHelperTest { 
            val commandInfo2: CommandInfo = sut.addCommand(commandType2)!!

            assertThat(commandInfo2.status).isEqualTo(CommandStatus.IDLE)



            executePendingCommands()

            runCurrent()

            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)