From 167f94169b5a502ad9675a3557f7cc4dbc958019 Mon Sep 17 00:00:00 2001
From: Ming-Shin Lu <lumark@google.com>
Date: Fri, 3 Mar 2023 06:55:38 +0000
Subject: [PATCH] Make ImePerfTest#testShowImeCold more accurate

In case killBaselineIme() will result the system busy on doing kill
process stuff and easier happen the locking contention in WM/ATM side
when measuring show IME with cold-lunch activity performance,

Make sure to start atrace after kill IME process being settled down
and dumping the trace for each iteration.

With this CL, following metrics can more accurate in reality without
being affected by unexptected delay:
- testShowImeCold_cv: 70 -> 10-20
- startInputOrWindowGainedFocus_mean: 6-8ms -> 2.8-4ms

Bug: 266708619
Test: atest ImePerfTest#testShowImeCold
Change-Id: Ibd004a36759e557c90bd6f156af3729d39f7721f
---
 .../src/android/inputmethod/ImePerfTest.java  | 33 ++++++++++++++-----
 .../perftests/utils/WindowPerfTestBase.java   |  2 +-
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/apct-tests/perftests/inputmethod/src/android/inputmethod/ImePerfTest.java b/apct-tests/perftests/inputmethod/src/android/inputmethod/ImePerfTest.java
index 2e44d82ca428d..e9c6c1ae0c87e 100644
--- a/apct-tests/perftests/inputmethod/src/android/inputmethod/ImePerfTest.java
+++ b/apct-tests/perftests/inputmethod/src/android/inputmethod/ImePerfTest.java
@@ -76,6 +76,7 @@ public class ImePerfTest extends ImePerfTestBase
         implements ManualBenchmarkState.CustomizedIterationListener {
     private static final String TAG = ImePerfTest.class.getSimpleName();
     private static final long ANIMATION_NOT_STARTED = -1;
+    private static final int WAIT_PROCESS_KILL_TIMEOUT_MS = 2000;
 
     @Rule
     public final PerfManualStatusReporter mPerfStatusReporter = new PerfManualStatusReporter();
@@ -248,19 +249,18 @@ public class ImePerfTest extends ImePerfTestBase
         boolean shouldRetry = false;
         while (shouldRetry || state.keepRunning(measuredTimeNs)) {
             shouldRetry = false;
-            killBaselineIme();
+            killBaselineImeSync();
             try (ImeSession imeSession = new ImeSession(BaselineIme.getName(
                     getInstrumentation().getContext()))) {
+                if (!mIsTraceStarted) {
+                    startAsyncAtrace();
+                }
                 final AtomicReference<CountDownLatch> latchStart = new AtomicReference<>();
                 final Activity activity = getActivityWithFocus();
 
                 setImeListener(activity, latchStart, null /* latchEnd */);
                 latchStart.set(new CountDownLatch(1));
 
-                if (!mIsTraceStarted) {
-                    startAsyncAtrace();
-                }
-
                 final WindowInsetsController controller =
                         activity.getWindow().getDecorView().getWindowInsetsController();
                 AtomicLong startTime = new AtomicLong();
@@ -270,6 +270,7 @@ public class ImePerfTest extends ImePerfTestBase
                 });
 
                 measuredTimeNs = waitForAnimationStart(latchStart, startTime);
+                stopAsyncAtraceAndDumpTraces();
 
                 if (measuredTimeNs == ANIMATION_NOT_STARTED) {
                     // Animation didn't start within timeout,
@@ -285,7 +286,7 @@ public class ImePerfTest extends ImePerfTestBase
         addResultToState(state);
     }
 
-    private void killBaselineIme() {
+    private void killBaselineImeSync() {
         // pidof returns a space separated list of numeric PIDs.
         String result = SystemUtil.runShellCommand(
                 "pidof com.android.perftests.inputmethod:BaselineIME");
@@ -294,7 +295,13 @@ public class ImePerfTest extends ImePerfTestBase
             if (TextUtils.isEmpty(pid)) {
                 continue;
             }
-            Process.killProcess(Integer.parseInt(pid));
+            final int pidToKill = Integer.parseInt(pid);
+            Process.killProcess(pidToKill);
+            try {
+                // Wait kill IME process being settled down.
+                Process.waitForProcessDeath(pidToKill, WAIT_PROCESS_KILL_TIMEOUT_MS);
+            } catch (Exception e) {
+            }
         }
     }
 
@@ -381,7 +388,7 @@ public class ImePerfTest extends ImePerfTestBase
             }
         } finally {
             if (mIsTraceStarted) {
-                stopAsyncAtrace();
+                stopAsyncAtraceAndDumpTraces();
             }
         }
         mActivityRule.finishActivity();
@@ -488,7 +495,7 @@ public class ImePerfTest extends ImePerfTestBase
         startAsyncAtrace("wm view");
     }
 
-    private void stopAsyncAtrace() {
+    private void stopAsyncAtraceAndDumpTraces() {
         if (!mIsTraceStarted) {
             return;
         }
@@ -504,6 +511,14 @@ public class ImePerfTest extends ImePerfTestBase
         }
     }
 
+    private void stopAsyncAtrace() {
+        if (!mIsTraceStarted) {
+            return;
+        }
+        mIsTraceStarted = false;
+        getUiAutomation().executeShellCommand("atrace --async_stop");
+    }
+
     @Override
     public void onStart(int iteration) {
         // Do not capture trace when profiling because the result will be much slower.
diff --git a/apct-tests/perftests/utils/src/android/perftests/utils/WindowPerfTestBase.java b/apct-tests/perftests/utils/src/android/perftests/utils/WindowPerfTestBase.java
index ca5913701d3b4..804baf4e65043 100644
--- a/apct-tests/perftests/utils/src/android/perftests/utils/WindowPerfTestBase.java
+++ b/apct-tests/perftests/utils/src/android/perftests/utils/WindowPerfTestBase.java
@@ -73,7 +73,7 @@ public class WindowPerfTestBase {
     }
 
     public static void startAsyncAtrace(String tags) {
-        getUiAutomation().executeShellCommand("atrace -b 32768 --async_start " + tags);
+        getUiAutomation().executeShellCommand("atrace --async_start -b 32768 -c " + tags);
         // Avoid atrace isn't ready immediately.
         SystemClock.sleep(TimeUnit.NANOSECONDS.toMillis(TIME_1_S_IN_NS));
     }