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

Commit 67bd9adf authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Offload I/O writes to a bg thread in AdditionalSubtypeMapRepository

This is a part of our on-going effort to ensure that slow I/O
operations will not block the entire system in
InputMethodManagerService (Bug 343601565).

This CL introduces a dedicated worker thread to enable
AdditionalSubtypeMapRepository to

 A. reduce potential lock contentions and the risk of ANRs around
    ImfLock.class due to slow I/O.
 B. merge multiple write operations into a single write operation.

without changing user-observable semantics.

Of course there is a risk that additional subtypes can be lost if the
system_server is unexpectedly rebooted before the pending write
operations are fulfilled, but the overall benefits are supposed to be
much larger than such a risk, which can be easily recovered when the
IME calls InputMethodManager#setAdditionalInputMethodSubtypes() again.

Bug: 341158961
Test: presubmit
Test: Manually verified as follows
  1. Build aosp_cf_x86_64_only_phone-trunk_staging-eng and run it
  2. Wait until the device fully boots up
  3. adb reboot  # to avoid Bug 121259290
  4. adb root
  5. adb shell "abx2xml /data/system/inputmethod/subtypes.xml -"
      -> make sure the content looks as follows:
         <subtypes>
           <imi id="com.android.inputmethod.latin/.LatinIME">
             <subtype ....>
             <subtype ....>
           </imi/>
         </subtypes>
  6. Open AOSP Keyboard settings
  7. Go to "Appearance & Layouts" -> "Custom input styles"
  8. Remove all layouts
  9. adb shell "abx2xml /data/system/inputmethod/subtypes.xml -"
      -> make sure the file no longer exists
 10. adb shell ls /data/system/inputmethod
      -> make sure the directory no longer exists
 11. adb reboot
 12. adb logcat -s InputMethodManagerService:*
      -> no error message about /data/system/inputmethod
 13. Open AOSP Keyboard settings
 14. Go to "Appearance & Layouts" -> "Custom input styles"
 15. Add English (US) - Dvorak
 16. adb shell ls /data/system/inputmethod
      -> make sure the directory exists and only the system user can
         access it
Change-Id: If51063a272317907505964cf2947e239661cc4ae
parent 61ed776f
Loading
Loading
Loading
Loading
+161 −3
Original line number Diff line number Diff line
@@ -16,12 +16,16 @@

package com.android.server.inputmethod;

import android.annotation.AnyThread;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.UserIdInt;
import android.annotation.WorkerThread;
import android.content.Context;
import android.content.pm.UserInfo;
import android.os.Handler;
import android.os.Process;
import android.util.IntArray;
import android.util.SparseArray;

import com.android.internal.annotations.GuardedBy;
@@ -29,6 +33,10 @@ import com.android.internal.inputmethod.DirectBootAwareness;
import com.android.server.LocalServices;
import com.android.server.pm.UserManagerInternal;

import java.util.ArrayList;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;

/**
 * Provides accesses to per-user additional {@link android.view.inputmethod.InputMethodSubtype}
 * persistent storages.
@@ -38,6 +46,152 @@ final class AdditionalSubtypeMapRepository {
    @NonNull
    private static final SparseArray<AdditionalSubtypeMap> sPerUserMap = new SparseArray<>();

    record WriteTask(@UserIdInt int userId, @NonNull AdditionalSubtypeMap subtypeMap,
                     @NonNull InputMethodMap inputMethodMap) {
    }

    static final class SingleThreadedBackgroundWriter {
        /**
         * A {@link ReentrantLock} used to guard {@link #mPendingTasks} and {@link #mRemovedUsers}.
         */
        @NonNull
        private final ReentrantLock mLock = new ReentrantLock();
        /**
         * A {@link Condition} associated with {@link #mLock} for producer to unblock consumer.
         */
        @NonNull
        private final Condition mLockNotifier = mLock.newCondition();

        @GuardedBy("mLock")
        @NonNull
        private final SparseArray<WriteTask> mPendingTasks = new SparseArray<>();

        @GuardedBy("mLock")
        private final IntArray mRemovedUsers = new IntArray();

        @NonNull
        private final Thread mWriterThread = new Thread("android.ime.as") {

            /**
             * Waits until the next data has come then return the result after filtering out any
             * already removed users.
             *
             * @return A list of {@link WriteTask} to be written into persistent storage
             */
            @WorkerThread
            private ArrayList<WriteTask> fetchNextTasks() {
                final SparseArray<WriteTask> tasks;
                final IntArray removedUsers;
                mLock.lock();
                try {
                    while (true) {
                        if (mPendingTasks.size() != 0) {
                            tasks = mPendingTasks.clone();
                            mPendingTasks.clear();
                            if (mRemovedUsers.size() == 0) {
                                removedUsers = null;
                            } else {
                                removedUsers = mRemovedUsers.clone();
                            }
                            break;
                        }
                        mLockNotifier.awaitUninterruptibly();
                    }
                } finally {
                    mLock.unlock();
                }
                final int size = tasks.size();
                final ArrayList<WriteTask> result = new ArrayList<>(size);
                for (int i = 0; i < size; ++i) {
                    final int userId = tasks.keyAt(i);
                    if (removedUsers != null && removedUsers.contains(userId)) {
                        continue;
                    }
                    result.add(tasks.valueAt(i));
                }
                return result;
            }

            @WorkerThread
            @Override
            public void run() {
                Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);

                while (true) {
                    final ArrayList<WriteTask> tasks = fetchNextTasks();
                    tasks.forEach(task -> AdditionalSubtypeUtils.save(
                            task.subtypeMap, task.inputMethodMap, task.userId));
                }
            }
        };

        /**
         * Schedules a write operation
         *
         * @param userId the target user ID of this operation
         * @param subtypeMap {@link AdditionalSubtypeMap} to be saved
         * @param inputMethodMap {@link InputMethodMap} to be used to filter our {@code subtypeMap}
         */
        @AnyThread
        void scheduleWriteTask(@UserIdInt int userId, @NonNull AdditionalSubtypeMap subtypeMap,
                @NonNull InputMethodMap inputMethodMap) {
            final var task = new WriteTask(userId, subtypeMap, inputMethodMap);
            mLock.lock();
            try {
                if (mRemovedUsers.contains(userId)) {
                    return;
                }
                mPendingTasks.put(userId, task);
                mLockNotifier.signalAll();
            } finally {
                mLock.unlock();
            }
        }

        /**
         * Called back when a user is being created.
         *
         * @param userId The user ID to be created
         */
        @AnyThread
        void onUserCreated(@UserIdInt int userId) {
            mLock.lock();
            try {
                for (int i = mRemovedUsers.size() - 1; i >= 0; --i) {
                    if (mRemovedUsers.get(i) == userId) {
                        mRemovedUsers.remove(i);
                    }
                }
            } finally {
                mLock.unlock();
            }
        }

        /**
         * Called back when a user is being removed. Any pending task will be effectively canceled
         * if the user is removed before the task is fulfilled.
         *
         * @param userId The user ID to be removed
         */
        @AnyThread
        void onUserRemoved(@UserIdInt int userId) {
            mLock.lock();
            try {
                mRemovedUsers.add(userId);
                mPendingTasks.remove(userId);
            } finally {
                mLock.unlock();
            }
        }

        void startThread() {
            mWriterThread.start();
        }
    }

    private static final SingleThreadedBackgroundWriter sWriter =
            new SingleThreadedBackgroundWriter();

    /**
     * Not intended to be instantiated.
     */
@@ -64,9 +218,11 @@ final class AdditionalSubtypeMapRepository {
            return;
        }
        sPerUserMap.put(userId, map);
        // TODO: Offload this to a background thread.
        // TODO: Skip if the previous data is exactly the same as new one.
        AdditionalSubtypeUtils.save(map, inputMethodMap, userId);
        sWriter.scheduleWriteTask(userId, map, inputMethodMap);
    }

    static void startWriterThread() {
        sWriter.startThread();
    }

    static void initialize(@NonNull Handler handler, @NonNull Context context) {
@@ -78,6 +234,7 @@ final class AdditionalSubtypeMapRepository {
                        @Override
                        public void onUserCreated(UserInfo user, @Nullable Object token) {
                            final int userId = user.id;
                            sWriter.onUserCreated(userId);
                            handler.post(() -> {
                                synchronized (ImfLock.class) {
                                    if (!sPerUserMap.contains(userId)) {
@@ -99,6 +256,7 @@ final class AdditionalSubtypeMapRepository {
                        @Override
                        public void onUserRemoved(UserInfo user) {
                            final int userId = user.id;
                            sWriter.onUserRemoved(userId);
                            handler.post(() -> {
                                synchronized (ImfLock.class) {
                                    sPerUserMap.remove(userId);
+4 −0
Original line number Diff line number Diff line
@@ -1547,6 +1547,10 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
                InputMethodUtils.setNonSelectedSystemImesDisabledUntilUsed(
                        getPackageManagerForUser(mContext, currentUserId),
                        newSettings.getEnabledInputMethodList());

                final var unused = SystemServerInitThreadPool.submit(
                        AdditionalSubtypeMapRepository::startWriterThread,
                        "Start AdditionalSubtypeMapRepository's writer thread");
            }
        }
    }