From c0537b750d0f29dab4a792e4d9d728d7ef69354e Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Fri, 13 May 2022 14:50:03 -0600 Subject: [PATCH 1/2] Defer sending config change to cached process Though there was commit e839c2c to defer on client side, if CachedAppOptimizer decides to freeze the cached process, the async binder buffer may still be filled, and may lead to binder transaction fail right after unfreezing because the free buffer is not enough. So now there is no IPC for config change of cached process, and will send the latest change to the client when the process state is changed from cached to non-cached. Bug: 217934273 Test: WindowProcessControllerTests#testCachedStateConfigurationChange Change-Id: I09c2feffba2fc124c623796af69de9f6adb60a39 --- .../server/wm/WindowProcessController.java | 107 ++++++++++++------ .../wm/WindowProcessControllerTests.java | 40 +++++++ 2 files changed, 112 insertions(+), 35 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 40417a4857d3d..1c64a06036f0b 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -16,6 +16,7 @@ package com.android.server.wm; +import static android.app.ActivityManager.PROCESS_STATE_CACHED_ACTIVITY; import static android.app.ActivityManager.PROCESS_STATE_NONEXISTENT; import static android.app.WindowConfiguration.ACTIVITY_TYPE_UNDEFINED; import static android.content.res.Configuration.ASSETS_SEQ_UNDEFINED; @@ -195,6 +196,11 @@ public class WindowProcessController extends ConfigurationContainer= CACHED_CONFIG_PROC_STATE && repProcState < CACHED_CONFIG_PROC_STATE + && thread != null && mHasCachedConfiguration) { + final Configuration config; + synchronized (mLastReportedConfiguration) { + config = new Configuration(mLastReportedConfiguration); + } + // Schedule immediately to make sure the app component (e.g. receiver, service) can get + // the latest configuration in their lifecycle callbacks (e.g. onReceive, onCreate). + scheduleConfigurationChange(thread, config); + } } int getReportedProcState() { @@ -1328,12 +1353,22 @@ public class WindowProcessController extends ConfigurationContainer 0) { + mHasPendingConfigurationChange = true; + return; + } + dispatchConfiguration(config); } @Override @@ -1359,25 +1394,6 @@ public class WindowProcessController extends ConfigurationContainer 0) { - mHasPendingConfigurationChange = true; - return; - } - dispatchConfiguration(config); - } - void dispatchConfiguration(Configuration config) { mHasPendingConfigurationChange = false; if (mThread == null) { @@ -1388,29 +1404,47 @@ public class WindowProcessController extends ConfigurationContainer= CACHED_CONFIG_PROC_STATE) { + mHasCachedConfiguration = true; + // Because there are 2 volatile accesses in setReportedProcState(): mRepProcState and + // mHasCachedConfiguration, check again in case mRepProcState is changed but hasn't + // read the change of mHasCachedConfiguration. + if (mRepProcState >= CACHED_CONFIG_PROC_STATE) { + return; + } + } + + scheduleConfigurationChange(mThread, config); + } + + private void scheduleConfigurationChange(IApplicationThread thread, Configuration config) { ProtoLog.v(WM_DEBUG_CONFIGURATION, "Sending to proc %s new config %s", mName, config); if (Build.IS_DEBUGGABLE && mHasImeService) { // TODO (b/135719017): Temporary log for debugging IME service. Slog.v(TAG_CONFIGURATION, "Sending to IME proc " + mName + " new config " + config); } - + mHasCachedConfiguration = false; try { - config.seq = mAtm.increaseConfigurationSeqLocked(); - mAtm.getLifecycleManager().scheduleTransaction(mThread, + mAtm.getLifecycleManager().scheduleTransaction(thread, ConfigurationChangeItem.obtain(config)); - setLastReportedConfiguration(config); } catch (Exception e) { - Slog.e(TAG_CONFIGURATION, "Failed to schedule configuration change", e); + Slog.e(TAG_CONFIGURATION, "Failed to schedule configuration change: " + mOwner, e); } } void setLastReportedConfiguration(Configuration config) { - mLastReportedConfiguration.setTo(config); - } - - Configuration getLastReportedConfiguration() { - return mLastReportedConfiguration; + // Synchronize for the access from setReportedProcState(). + synchronized (mLastReportedConfiguration) { + mLastReportedConfiguration.setTo(config); + } } void pauseConfigurationDispatch() { @@ -1461,6 +1495,8 @@ public class WindowProcessController extends ConfigurationContainer non-cached will send the previous deferred config immediately. + mWpc.setReportedProcState(ActivityManager.PROCESS_STATE_RECEIVER); + final ArgumentCaptor captor = + ArgumentCaptor.forClass(ConfigurationChangeItem.class); + verify(clientManager).scheduleTransaction(eq(thread), captor.capture()); + final ClientTransactionHandler client = mock(ClientTransactionHandler.class); + captor.getValue().preExecute(client, null /* token */); + final ArgumentCaptor configCaptor = + ArgumentCaptor.forClass(Configuration.class); + verify(client).updatePendingConfiguration(configCaptor.capture()); + assertEquals(newConfig, configCaptor.getValue()); + } + @Test public void testComputeOomAdjFromActivities() { final ActivityRecord activity = createActivityRecord(mWpc); From 7371d0be415827866b6c43c17b526930a9dd6362 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Fri, 13 May 2022 14:50:09 -0600 Subject: [PATCH 2/2] Remove client side deferred config for cached state Since [1], the server side won't send the config change to a cached state process, until it has an active state again. So the logic of client side deferring can be removed. [1]: I09c2feffba2fc124c623796af69de9f6adb60a39 Bug: 217934273 Test: ActivityThreadTest Change-Id: Ibd113f4fb3b566e4e9cf5d7960b63a5c7790fb75 --- core/java/android/app/ActivityThread.java | 30 ------------ .../android/app/ActivityThreadInternal.java | 2 - .../android/app/ConfigurationController.java | 6 --- .../app/activity/ActivityThreadTest.java | 48 ------------------- 4 files changed, 86 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index b4cabada05228..98683f0bd87a8 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -3410,26 +3410,12 @@ public final class ActivityThread extends ClientTransactionHandler } } - /** - * Returns {@code true} if the {@link android.app.ActivityManager.ProcessState} of the current - * process is cached. - */ - @Override - @VisibleForTesting - public boolean isCachedProcessState() { - synchronized (mAppThread) { - return mLastProcessState >= ActivityManager.PROCESS_STATE_CACHED_ACTIVITY; - } - } - @Override public void updateProcessState(int processState, boolean fromIpc) { - final boolean wasCached; synchronized (mAppThread) { if (mLastProcessState == processState) { return; } - wasCached = isCachedProcessState(); mLastProcessState = processState; // Defer the top state for VM to avoid aggressive JIT compilation affecting activity // launch time. @@ -3446,22 +3432,6 @@ public final class ActivityThread extends ClientTransactionHandler + (fromIpc ? " (from ipc" : "")); } } - - // Handle the pending configuration if the process state is changed from cached to - // non-cached. Except the case where there is a launching activity because the - // LaunchActivityItem will handle it. - if (wasCached && !isCachedProcessState() && mNumLaunchingActivities.get() == 0) { - final Configuration pendingConfig = - mConfigurationController.getPendingConfiguration(false /* clearPending */); - if (pendingConfig == null) { - return; - } - if (Looper.myLooper() == mH.getLooper()) { - handleConfigurationChanged(pendingConfig); - } else { - sendMessage(H.CONFIGURATION_CHANGED, pendingConfig); - } - } } /** Update VM state based on ActivityManager.PROCESS_STATE_* constants. */ diff --git a/core/java/android/app/ActivityThreadInternal.java b/core/java/android/app/ActivityThreadInternal.java index b9ad5c3378137..72506b9fcdbbd 100644 --- a/core/java/android/app/ActivityThreadInternal.java +++ b/core/java/android/app/ActivityThreadInternal.java @@ -32,8 +32,6 @@ interface ActivityThreadInternal { boolean isInDensityCompatMode(); - boolean isCachedProcessState(); - Application getApplication(); ArrayList collectComponentCallbacks(boolean includeUiContexts); diff --git a/core/java/android/app/ConfigurationController.java b/core/java/android/app/ConfigurationController.java index 1a77b65c8ef6b..18dc1ce18bafb 100644 --- a/core/java/android/app/ConfigurationController.java +++ b/core/java/android/app/ConfigurationController.java @@ -124,12 +124,6 @@ class ConfigurationController { * @param config The new configuration. */ void handleConfigurationChanged(@NonNull Configuration config) { - if (mActivityThread.isCachedProcessState()) { - updatePendingConfiguration(config); - // If the process is in a cached state, delay the handling until the process is no - // longer cached. - return; - } Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "configChanged"); handleConfigurationChanged(config, null /* compat */); Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); diff --git a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java index bfb2fd57975f8..a2d4bafef41cd 100644 --- a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java +++ b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java @@ -31,7 +31,6 @@ import static org.junit.Assert.assertTrue; import android.annotation.Nullable; import android.app.Activity; -import android.app.ActivityManager; import android.app.ActivityThread; import android.app.ActivityThread.ActivityClientRecord; import android.app.IApplicationThread; @@ -570,53 +569,6 @@ public class ActivityThreadTest { }); } - @Test - public void testHandleProcessConfigurationChanged_DependOnProcessState() { - final ActivityThread activityThread = ActivityThread.currentActivityThread(); - final Configuration origConfig = activityThread.getConfiguration(); - final int newDpi = origConfig.densityDpi + 10; - final Configuration newConfig = new Configuration(origConfig); - newConfig.seq++; - newConfig.densityDpi = newDpi; - - activityThread.updateProcessState(ActivityManager.PROCESS_STATE_CACHED_ACTIVITY, - false /* fromIPC */); - - applyProcessConfiguration(activityThread, newConfig); - try { - // In the cached state, the configuration is only set as pending and not applied. - assertEquals(origConfig.densityDpi, activityThread.getConfiguration().densityDpi); - assertTrue(activityThread.isCachedProcessState()); - } finally { - // The foreground state is the default state of instrumentation. - activityThread.updateProcessState(ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE, - false /* fromIPC */); - } - InstrumentationRegistry.getInstrumentation().waitForIdleSync(); - - try { - // The state becomes non-cached, the pending configuration should be applied. - assertEquals(newConfig.densityDpi, activityThread.getConfiguration().densityDpi); - assertFalse(activityThread.isCachedProcessState()); - } finally { - // Restore to the original configuration. - activityThread.getConfiguration().seq = origConfig.seq - 1; - applyProcessConfiguration(activityThread, origConfig); - } - } - - private static void applyProcessConfiguration(ActivityThread thread, Configuration config) { - final ClientTransaction clientTransaction = newTransaction(thread, - null /* activityToken */); - clientTransaction.addCallback(ConfigurationChangeItem.obtain(config)); - final IApplicationThread appThread = thread.getApplicationThread(); - try { - appThread.scheduleTransaction(clientTransaction); - } catch (Exception ignored) { - } - InstrumentationRegistry.getInstrumentation().waitForIdleSync(); - } - @Test public void testResumeAfterNewIntent() { final Activity activity = mActivityTestRule.launchActivity(new Intent());