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

Commit 819b63c6 authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Eliminate acquiring WM lock when bind/unbind service 

There were lock contention between AM and WM when an activity
binds/unbinds service from:
- Associate the connection to the activity
- Add connection
- Remove connection
when WM is running layout/animation or updating config.

This change applies:
- Use a separated lock to manage the connection.
- Cache the visible state for BIND_ADJUST_WITH_ACTIVITY.
to eliminate the lock contention.

Bug: 263755904
Bug: 204870457
Test: ActivityRecordTests#testActivityServiceConnectionsHolder
Change-Id: Id5b7862866180064cb8ec863cd9cd6ccebdaea1e
parent 99db8bd9

services/core/java/com/android/server/wm/ActivityRecord.java

+37 −9
Original line number Diff line number Diff line
@@ -341,6 +341,7 @@ import android.window.TransitionInfo.AnimationOptions; 
import android.window.WindowContainerToken;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.app.ResolverActivity;

import com.android.internal.content.ReferrerIntent;
@@ -503,7 +504,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
    private RemoteTransition mPendingRemoteTransition;

    ActivityOptions returningOptions; // options that are coming back via convertToTranslucent

    AppTimeTracker appTimeTracker; // set if we are tracking the time in this app/task/activity

    @GuardedBy("this")

    ActivityServiceConnectionsHolder mServiceConnectionsHolder; // Service connections.

    /** @see android.content.Context#BIND_ADJUST_WITH_ACTIVITY */

    volatile boolean mVisibleForServiceConnection;

    UriPermissionOwner uriPermissions; // current special URI access perms.

    WindowProcessController app;      // if non-null, hosting application

    private State mState;    // current state we are in
@@ -553,7 +557,8 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
    long lastLaunchTime;    // time of last launch of this activity

    ComponentName requestedVrComponent; // the requested component for handling VR mode.



    boolean inHistory;  // are we in the history task?

    /** Whether this activity is reachable from hierarchy. */

    volatile boolean inHistory;

    final ActivityTaskSupervisor mTaskSupervisor;

    final RootWindowContainer mRootWindowContainer;
@@ -1904,7 +1909,9 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
        }

    }



    static @Nullable ActivityRecord forTokenLocked(IBinder token) {

    /** Gets the corresponding record by the token. Note that it may not exist in the hierarchy. */

    @Nullable

    static ActivityRecord forToken(IBinder token) {

        if (token == null) return null;

        final Token activityToken;

        try {
@@ -1913,7 +1920,11 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
            Slog.w(TAG, "Bad activity token: " + token, e);

            return null;

        }

        final ActivityRecord r = activityToken.mActivityRef.get();

        return activityToken.mActivityRef.get();

    }



    static @Nullable ActivityRecord forTokenLocked(IBinder token) {

        final ActivityRecord r = forToken(token);

        return r == null || r.getRootTask() == null ? null : r;

    }
@@ -4036,10 +4047,20 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
        mDisplayContent.getDisplayPolicy().removeRelaunchingApp(this);

    }



    ActivityServiceConnectionsHolder getOrCreateServiceConnectionsHolder() {

        synchronized (this) {

            if (mServiceConnectionsHolder == null) {

                mServiceConnectionsHolder = new ActivityServiceConnectionsHolder(this);

            }

            return mServiceConnectionsHolder;

        }

    }



    /**

     * Perform clean-up of service connections in an activity record.

     */

    private void cleanUpActivityServices() {

        synchronized (this) {

            if (mServiceConnectionsHolder == null) {

                return;

            }
@@ -4048,6 +4069,11 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
            // This activity record is removing, make sure not to disconnect twice.

            mServiceConnectionsHolder = null;

        }

    }



    private void updateVisibleForServiceConnection() {

        mVisibleForServiceConnection = mVisibleRequested || mState == RESUMED || mState == PAUSING;

    }



    /**

     * Detach this activity from process and clear the references to it. If the activity is
@@ -5139,6 +5165,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
            taskFragment.onActivityVisibleRequestedChanged();

        }

        setInsetsFrozen(!visible);

        updateVisibleForServiceConnection();

        if (app != null) {

            mTaskSupervisor.onProcessActivityStateChanged(app, false /* forceBatch */);

        }
@@ -5617,6 +5644,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
                return;

            }

        }

        updateVisibleForServiceConnection();

        if (app != null) {

            mTaskSupervisor.onProcessActivityStateChanged(app, false /* forceBatch */);

        }

services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java

+15 −20
Original line number Diff line number Diff line
@@ -16,14 +16,14 @@ 


package com.android.server.wm;



import static com.android.server.wm.ActivityRecord.State.PAUSING;

import static com.android.server.wm.ActivityRecord.State.RESUMED;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_CLEANUP;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_ATM;



import android.util.ArraySet;

import android.util.Slog;



import com.android.internal.annotations.GuardedBy;



import java.io.PrintWriter;

import java.util.function.Consumer;
@@ -38,8 +38,6 @@ import java.util.function.Consumer; 
 */

public class ActivityServiceConnectionsHolder<T> {



    private final ActivityTaskManagerService mService;



    /** The activity the owns this service connection object. */

    private final ActivityRecord mActivity;
@@ -49,19 +47,19 @@ public class ActivityServiceConnectionsHolder<T> { 
     * on the WM side since we don't perform operations on the object. Mainly here for communication

     * and booking with the AM side.

     */

    @GuardedBy("mActivity")

    private ArraySet<T> mConnections;



    /** Whether all connections of {@link #mActivity} are being removed. */

    private volatile boolean mIsDisconnecting;



    ActivityServiceConnectionsHolder(ActivityTaskManagerService service, ActivityRecord activity) {

        mService = service;

    ActivityServiceConnectionsHolder(ActivityRecord activity) {

        mActivity = activity;

    }



    /** Adds a connection record that the activity has bound to a specific service. */

    public void addConnection(T c) {

        synchronized (mService.mGlobalLock) {

        synchronized (mActivity) {

            if (mIsDisconnecting) {

                // This is unlikely to happen because the caller should create a new holder.

                if (DEBUG_CLEANUP) {
@@ -79,7 +77,7 @@ public class ActivityServiceConnectionsHolder<T> { 


    /** Removed a connection record between the activity and a specific service. */

    public void removeConnection(T c) {

        synchronized (mService.mGlobalLock) {

        synchronized (mActivity) {

            if (mConnections == null) {

                return;

            }
@@ -90,20 +88,18 @@ public class ActivityServiceConnectionsHolder<T> { 
        }

    }



    /** @see android.content.Context#BIND_ADJUST_WITH_ACTIVITY */

    public boolean isActivityVisible() {

        synchronized (mService.mGlobalLock) {

            return mActivity.isVisibleRequested() || mActivity.isState(RESUMED, PAUSING);

        }

        return mActivity.mVisibleForServiceConnection;

    }



    public int getActivityPid() {

        synchronized (mService.mGlobalLock) {

            return mActivity.hasProcess() ? mActivity.app.getPid() : -1;

        }

        final WindowProcessController wpc = mActivity.app;

        return wpc != null ? wpc.getPid() : -1;

    }



    public void forEachConnection(Consumer<T> consumer) {

        synchronized (mService.mGlobalLock) {

        synchronized (mActivity) {

            if (mConnections == null || mConnections.isEmpty()) {

                return;

            }
@@ -118,6 +114,7 @@ public class ActivityServiceConnectionsHolder<T> { 
     * general, this method is used to clean up if the activity didn't unbind services before it

     * is destroyed.

     */

    @GuardedBy("mActivity")

    void disconnectActivityFromServices() {

        if (mConnections == null || mConnections.isEmpty() || mIsDisconnecting) {

            return;
@@ -130,17 +127,15 @@ public class ActivityServiceConnectionsHolder<T> { 
        // still in the message queue, so keep the reference of {@link #mConnections} to make sure

        // the connection list is up-to-date.

        mIsDisconnecting = true;

        mService.mH.post(() -> {

            mService.mAmInternal.disconnectActivityFromServices(this);

        mActivity.mAtmService.mH.post(() -> {

            mActivity.mAtmService.mAmInternal.disconnectActivityFromServices(this);

            mIsDisconnecting = false;

        });

    }



    public void dump(PrintWriter pw, String prefix) {

        synchronized (mService.mGlobalLock) {

        pw.println(prefix + "activity=" + mActivity);

    }

    }



    /** Used by {@link ActivityRecord#dump}. */

    @Override

services/core/java/com/android/server/wm/ActivityTaskManagerService.java

+4 −11
Original line number Diff line number Diff line
@@ -6000,18 +6000,11 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { 


        @Override

        public ActivityServiceConnectionsHolder getServiceConnectionsHolder(IBinder token) {

            synchronized (mGlobalLock) {

                final ActivityRecord r = ActivityRecord.isInRootTaskLocked(token);

                if (r == null) {

            final ActivityRecord r = ActivityRecord.forToken(token);

            if (r == null || !r.inHistory) {

                return null;

            }

                if (r.mServiceConnectionsHolder == null) {

                    r.mServiceConnectionsHolder = new ActivityServiceConnectionsHolder(

                            ActivityTaskManagerService.this, r);

                }



                return r.mServiceConnectionsHolder;

            }

            return r.getOrCreateServiceConnectionsHolder();

        }



        @Override

services/core/java/com/android/server/wm/TaskFragment.java

+1 −1
Original line number Diff line number Diff line
@@ -1953,7 +1953,7 @@ class TaskFragment extends WindowContainer<WindowContainer> { 
                        1f);

                mBackScreenshots.put(r.mActivityComponent.flattenToString(), backBuffer);

            }

            child.asActivityRecord().inHistory = true;

            addingActivity.inHistory = true;

            task.onDescendantActivityAdded(taskHadActivity, activityType, addingActivity);

        }

    }

services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java

+26 −0
Original line number Diff line number Diff line
@@ -2323,6 +2323,32 @@ public class ActivityRecordTests extends WindowTestsBase { 
        assertTrue(activity.pictureInPictureArgs.isLaunchIntoPip());

    }



    @Test

    public void testActivityServiceConnectionsHolder() {

        final ActivityRecord activity = new ActivityBuilder(mAtm).setCreateTask(true).build();

        final ActivityServiceConnectionsHolder<Object> holder =

                mAtm.mInternal.getServiceConnectionsHolder(activity.token);

        assertNotNull(holder);

        final Object connection = new Object();

        holder.addConnection(connection);

        assertTrue(holder.isActivityVisible());

        final int[] count = new int[1];

        final Consumer<Object> c = conn -> count[0]++;

        holder.forEachConnection(c);

        assertEquals(1, count[0]);



        holder.removeConnection(connection);

        holder.forEachConnection(c);

        assertEquals(1, count[0]);



        activity.setVisibleRequested(false);

        activity.setState(STOPPED, "test");

        assertFalse(holder.isActivityVisible());



        activity.removeImmediately();

        assertNull(mAtm.mInternal.getServiceConnectionsHolder(activity.token));

    }



    @Test

    public void testTransferLaunchCookieWhenFinishing() {

        final ActivityRecord activity1 = createActivityWithTask();