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

Commit f4887903 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Remove ImfLock dependencies from *Repository 

This CL reworks how the following two per-user data stores are
guaranteed to be initialized for each user with also making them
independent of ImfLock.

 * AdditionalSubtypeMapRepository
 * InputMethodSettingsRepository

Here is a quick summary on how the above per-user data stores will be
initialized.

 1. IMMS.Lifecycle(Context context) gets called
  1.1 InputMethodManagerService is initialized
  1.2 IMMS.Lifecycle#initializeUsersAsync() is called for existing
      users, which initializes the above two data stores in the
      I/O thread.
 2. IMMS.Lifecycle#onBootPhase(int phase) gets called for
    PHASE_ACTIVITY_MANAGER_READY
  2.1 IMMS#systemRunning() first calls #waitForUserInitialization()
      to make sure that initializeUsersAsync() is completed with
      a certain timeout (3 sec).
  2.2 IMMS#systemRunning() starts tasks with an assumption that the
      above per-user data stores are initialized.

Here are useful guarantees we can start relying on with this CL.

 * AdditionalSubtypeMap#load() is guaranteed to be called only once
   for each user on the I/O thread without ImfLock.
 * When IMMS#systemRunning() is running *Repository are already
   initialized (unless it's timed out).

This CL should improve the resource utilizations during boot time
without changing the observable semantics.

Bug: 343601565
Fix: 352354308
Test: presubmit
Test: manually verified by checking logcat
Flag: EXEMPT refactor
Change-Id: I4400519f133638c03205f7a8902f7e00c291eda8
parent d2715b13

services/core/java/com/android/server/inputmethod/AdditionalSubtypeMapRepository.java

+71 −22
Original line number Diff line number Diff line
@@ -20,9 +20,9 @@ import android.annotation.AnyThread; 
import android.annotation.NonNull;

import android.annotation.UserIdInt;

import android.annotation.WorkerThread;

import android.os.Handler;

import android.os.Process;

import android.util.IntArray;

import android.util.Slog;

import android.util.SparseArray;



import com.android.internal.annotations.GuardedBy;
@@ -36,7 +36,10 @@ import java.util.concurrent.locks.ReentrantLock; 
 * persistent storages.

 */

final class AdditionalSubtypeMapRepository {

    @GuardedBy("ImfLock.class")

    private static final String TAG = "AdditionalSubtypeMapRepository";



    // TODO(b/352594784): Should we user other lock primitives?

    @GuardedBy("sPerUserMap")

    @NonNull

    private static final SparseArray<AdditionalSubtypeMap> sPerUserMap = new SparseArray<>();
@@ -192,21 +195,67 @@ final class AdditionalSubtypeMapRepository { 
    private AdditionalSubtypeMapRepository() {

    }



    /**

     * Returns {@link AdditionalSubtypeMap} for the given user.

     *

     * <p>This method is expected be called after {@link #ensureInitializedAndGet(int)}. Otherwise

     * {@link AdditionalSubtypeMap#EMPTY_MAP} will be returned.</p>

     *

     * @param userId the user to be queried about

     * @return {@link AdditionalSubtypeMap} for the given user

     */

    @AnyThread

    @NonNull

    @GuardedBy("ImfLock.class")

    static AdditionalSubtypeMap get(@UserIdInt int userId) {

        final AdditionalSubtypeMap map = sPerUserMap.get(userId);

        if (map != null) {

        final AdditionalSubtypeMap map;

        synchronized (sPerUserMap) {

            map = sPerUserMap.get(userId);

        }

        if (map == null) {

            Slog.e(TAG, "get(userId=" + userId + ") is called before loadInitialDataAndGet()."

                    + " Returning an empty map");

            return AdditionalSubtypeMap.EMPTY_MAP;

        }

        return map;

    }

        final AdditionalSubtypeMap newMap = AdditionalSubtypeUtils.load(userId);

        sPerUserMap.put(userId, newMap);

        return newMap;



    /**

     * Ensures that {@link AdditionalSubtypeMap} is initialized for the given user. Load it from

     * the persistent storage if {@link #putAndSave(int, AdditionalSubtypeMap, InputMethodMap)} has

     * not been called yet.

     *

     * @param userId the user to be initialized

     * @return {@link AdditionalSubtypeMap} that is associated with the given user. If

     *         {@link #putAndSave(int, AdditionalSubtypeMap, InputMethodMap)} is already called

     *         then the given {@link AdditionalSubtypeMap}.

     */

    @AnyThread

    @NonNull

    static AdditionalSubtypeMap ensureInitializedAndGet(@UserIdInt int userId) {

        final var map = AdditionalSubtypeUtils.load(userId);

        synchronized (sPerUserMap) {

            final AdditionalSubtypeMap previous = sPerUserMap.get(userId);

            // If putAndSave() has already been called, then use it.

            if (previous != null) {

                return previous;

            }

            sPerUserMap.put(userId, map);

        }

        return map;

    }



    @GuardedBy("ImfLock.class")

    /**

     * Puts {@link AdditionalSubtypeMap} for the given user then schedule an I/O task to save it

     * to the storage.

     *

     * @param userId         the user for the given {@link AdditionalSubtypeMap} is to be saved

     * @param map            {@link AdditionalSubtypeMap} to be saved

     * @param inputMethodMap {@link InputMethodMap} to be used while saving the data

     */

    @AnyThread

    static void putAndSave(@UserIdInt int userId, @NonNull AdditionalSubtypeMap map,

            @NonNull InputMethodMap inputMethodMap) {

        synchronized (sPerUserMap) {

            final AdditionalSubtypeMap previous = sPerUserMap.get(userId);

            if (previous == map) {

                return;
@@ -214,7 +263,9 @@ final class AdditionalSubtypeMapRepository { 
            sPerUserMap.put(userId, map);

            sWriter.scheduleWriteTask(userId, map, inputMethodMap);

        }

    }



    @AnyThread

    static void startWriterThread() {

        sWriter.startThread();

    }
@@ -225,12 +276,10 @@ final class AdditionalSubtypeMapRepository { 
    }



    @AnyThread

    static void remove(@UserIdInt int userId, @NonNull Handler ioHandler) {

    static void remove(@UserIdInt int userId) {

        synchronized (sPerUserMap) {

            sWriter.onUserRemoved(userId);

        ioHandler.post(() -> {

            synchronized (ImfLock.class) {

            sPerUserMap.remove(userId);

        }

        });

    }

}

services/core/java/com/android/server/inputmethod/InputMethodManagerService.java

+52 −21
Original line number Diff line number Diff line
@@ -218,6 +218,13 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl. 
    static final String TAG = "InputMethodManagerService";

    public static final String PROTO_ARG = "--proto";



    /**

     * Timeout in milliseconds in {@link #systemRunning()} to make sure that users are initialized

     * in {@link Lifecycle#initializeUsersAsync(int[])}.

     */

    @DurationMillisLong

    private static final long SYSTEM_READY_USER_INIT_TIMEOUT = 3000;



    @Retention(SOURCE)

    @IntDef({ShellCommandResult.SUCCESS, ShellCommandResult.FAILURE})

    private @interface ShellCommandResult {
@@ -1023,7 +1030,7 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl. 
            // Called directly from UserManagerService. Do not block the calling thread.

            final int userId = user.id;

            SecureSettingsWrapper.onUserRemoved(userId);

            AdditionalSubtypeMapRepository.remove(userId, mService.mIoHandler);

            AdditionalSubtypeMapRepository.remove(userId);

            InputMethodSettingsRepository.remove(userId);

            mService.mUserDataRepository.remove(userId);

        }
@@ -1054,39 +1061,31 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl. 


        @AnyThread

        private void initializeUsersAsync(@UserIdInt int[] userIds) {

            Slog.d(TAG, "Schedule initialization for users=" + Arrays.toString(userIds));

            mService.mIoHandler.post(() -> {

                final var service = mService;

                final var context = service.mContext;

                final var userManagerInternal = service.mUserManagerInternal;



                // We first create InputMethodMap for each user without loading AdditionalSubtypes.

                final int numUsers = userIds.length;

                final InputMethodMap[] rawMethodMaps = new InputMethodMap[numUsers];

                for (int i = 0; i < numUsers; ++i) {

                    final int userId = userIds[i];

                    rawMethodMaps[i] = InputMethodManagerService.queryInputMethodServicesInternal(

                            context, userId, AdditionalSubtypeMap.EMPTY_MAP,

                for (int userId : userIds) {

                    Slog.d(TAG, "Start initialization for user=" + userId);

                    final var additionalSubtypeMap =

                            AdditionalSubtypeMapRepository.ensureInitializedAndGet(userId);

                    final var settings = InputMethodManagerService.queryInputMethodServicesInternal(

                            context, userId, additionalSubtypeMap,

                            DirectBootAwareness.AUTO).getMethodMap();

                    InputMethodSettingsRepository.put(userId,

                            InputMethodSettings.create(settings, userId));



                    final int profileParentId = userManagerInternal.getProfileParentId(userId);

                    final boolean value =

                            InputMethodDrawsNavBarResourceMonitor.evaluate(context,

                                    profileParentId);

                    final var userData = mService.getUserData(userId);

                    userData.mImeDrawsNavBar.set(value);

                }



                // Then create full InputMethodMap for each user. Note that

                // AdditionalSubtypeMapRepository#get() and InputMethodSettingsRepository#put()

                // need to be called with ImfLock held (b/352387655).

                // TODO(b/343601565): Avoid ImfLock after fixing b/352387655.

                synchronized (ImfLock.class) {

                    for (int i = 0; i < numUsers; ++i) {

                        final int userId = userIds[i];

                        final var map = AdditionalSubtypeMapRepository.get(userId);

                        final var methodMap = rawMethodMaps[i].applyAdditionalSubtypes(map);

                        final var settings = InputMethodSettings.create(methodMap, userId);

                        InputMethodSettingsRepository.put(userId, settings);

                    }

                    userData.mBackgroundLoadLatch.countDown();

                    Slog.d(TAG, "Complete initialization for user=" + userId);

                }

            });

        }
@@ -1331,10 +1330,42 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl. 
        }

    }



    private void waitForUserInitialization() {

        final int[] userIds = mUserManagerInternal.getUserIds();

        final long deadlineNanos = SystemClock.elapsedRealtimeNanos()

                + TimeUnit.MILLISECONDS.toNanos(SYSTEM_READY_USER_INIT_TIMEOUT);

        boolean interrupted = false;

        try {

            for (int userId : userIds) {

                final var latch = getUserData(userId).mBackgroundLoadLatch;

                boolean awaitResult;

                while (true) {

                    try {

                        final long remainingNanos =

                                Math.max(deadlineNanos - SystemClock.elapsedRealtimeNanos(), 0);

                        awaitResult = latch.await(remainingNanos, TimeUnit.NANOSECONDS);

                        break;

                    } catch (InterruptedException ignored) {

                        interrupted = true;

                    }

                }

                if (!awaitResult) {

                    Slog.w(TAG, "Timed out for user#" + userId + " to be initialized");

                }

            }

        } finally {

            if (interrupted) {

                Thread.currentThread().interrupt();

            }

        }

    }



    /**

     * TODO(b/32343335): The entire systemRunning() method needs to be revisited.

     */

    public void systemRunning() {

        waitForUserInitialization();



        synchronized (ImfLock.class) {

            if (DEBUG) {

                Slog.d(TAG, "--- systemReady");

services/core/java/com/android/server/inputmethod/InputMethodSettingsRepository.java

+12 −6
Original line number Diff line number Diff line
@@ -24,7 +24,8 @@ import android.util.SparseArray; 
import com.android.internal.annotations.GuardedBy;



final class InputMethodSettingsRepository {

    @GuardedBy("ImfLock.class")

    // TODO(b/352594784): Should we user other lock primitives?

    @GuardedBy("sPerUserMap")

    @NonNull

    private static final SparseArray<InputMethodSettings> sPerUserMap = new SparseArray<>();
@@ -35,23 +36,28 @@ final class InputMethodSettingsRepository { 
    }



    @NonNull

    @GuardedBy("ImfLock.class")

    @AnyThread

    static InputMethodSettings get(@UserIdInt int userId) {

        final InputMethodSettings obj = sPerUserMap.get(userId);

        final InputMethodSettings obj;

        synchronized (sPerUserMap) {

            obj = sPerUserMap.get(userId);

        }

        if (obj != null) {

            return obj;

        }

        return InputMethodSettings.createEmptyMap(userId);

    }



    @GuardedBy("ImfLock.class")

    @AnyThread

    static void put(@UserIdInt int userId, @NonNull InputMethodSettings obj) {

        synchronized (sPerUserMap) {

            sPerUserMap.put(userId, obj);

        }

    }



    @AnyThread

    static void remove(@UserIdInt int userId) {

        synchronized (ImfLock.class) {

        synchronized (sPerUserMap) {

            sPerUserMap.remove(userId);

        }

    }

services/core/java/com/android/server/inputmethod/UserData.java

+8 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import com.android.internal.annotations.GuardedBy; 
import com.android.internal.inputmethod.IRemoteAccessibilityInputConnection;

import com.android.internal.inputmethod.IRemoteInputConnection;



import java.util.concurrent.CountDownLatch;

import java.util.concurrent.atomic.AtomicBoolean;



/** Placeholder for all IMMS user specific fields */
@@ -35,6 +36,13 @@ final class UserData { 
    @UserIdInt

    final int mUserId;



    /**

     * Tells whether {@link InputMethodManagerService.Lifecycle#initializeUsersAsync(int[])} is

     * completed for this user or not.

     */

    @NonNull

    final CountDownLatch mBackgroundLoadLatch = new CountDownLatch(1);



    @NonNull

    final InputMethodBindingController mBindingController;

services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/InputMethodManagerServiceTestBase.java

+6 −0
Original line number Diff line number Diff line
@@ -161,6 +161,7 @@ public class InputMethodManagerServiceTestBase { 
                        .spyStatic(InputMethodUtils.class)

                        .mockStatic(ServiceManager.class)

                        .spyStatic(AdditionalSubtypeMapRepository.class)

                        .spyStatic(AdditionalSubtypeUtils.class)

                        .startMocking();



        mContext = spy(InstrumentationRegistry.getInstrumentation().getContext());
@@ -235,6 +236,7 @@ public class InputMethodManagerServiceTestBase { 


        // The background writer thread in AdditionalSubtypeMapRepository should be stubbed out.

        doNothing().when(AdditionalSubtypeMapRepository::startWriterThread);

        doReturn(AdditionalSubtypeMap.EMPTY_MAP).when(() -> AdditionalSubtypeUtils.load(anyInt()));



        mServiceThread =

                new ServiceThread(
@@ -267,6 +269,10 @@ public class InputMethodManagerServiceTestBase { 
        LocalServices.removeServiceForTest(InputMethodManagerInternal.class);

        lifecycle.onStart();



        // Emulate that the user initialization is done.

        AdditionalSubtypeMapRepository.ensureInitializedAndGet(mCallingUserId);

        mInputMethodManagerService.getUserData(mCallingUserId).mBackgroundLoadLatch.countDown();



        // After this boot phase, services can broadcast Intents.

        lifecycle.onBootPhase(SystemService.PHASE_ACTIVITY_MANAGER_READY);