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

Commit 34df8bb9 authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Make *Repository#get(int userId) lock-free" into main

parents bf13f5ba d1e9ca2c
Loading
Loading
Loading
Loading
+20 −30
Original line number Diff line number Diff line
@@ -38,10 +38,11 @@ import java.util.concurrent.locks.ReentrantLock;
final class AdditionalSubtypeMapRepository {
    private static final String TAG = "AdditionalSubtypeMapRepository";

    // TODO(b/352594784): Should we user other lock primitives?
    @GuardedBy("sPerUserMap")
    private static final Object sMutationLock = new Object();

    @NonNull
    private static final SparseArray<AdditionalSubtypeMap> sPerUserMap = new SparseArray<>();
    private static volatile ImmutableSparseArray<AdditionalSubtypeMap> sPerUserMap =
            ImmutableSparseArray.empty();

    record WriteTask(@UserIdInt int userId, @NonNull AdditionalSubtypeMap subtypeMap,
                     @NonNull InputMethodMap inputMethodMap) {
@@ -198,7 +199,7 @@ final class AdditionalSubtypeMapRepository {
    /**
     * Returns {@link AdditionalSubtypeMap} for the given user.
     *
     * <p>This method is expected be called after {@link #ensureInitializedAndGet(int)}. Otherwise
     * <p>This method is expected be called after {@link #initializeIfNecessary(int)}. Otherwise
     * {@link AdditionalSubtypeMap#EMPTY_MAP} will be returned.</p>
     *
     * @param userId the user to be queried about
@@ -207,10 +208,7 @@ final class AdditionalSubtypeMapRepository {
    @AnyThread
    @NonNull
    static AdditionalSubtypeMap get(@UserIdInt int userId) {
        final AdditionalSubtypeMap map;
        synchronized (sPerUserMap) {
            map = sPerUserMap.get(userId);
        }
        final AdditionalSubtypeMap map = sPerUserMap.get(userId);
        if (map == null) {
            Slog.e(TAG, "get(userId=" + userId + ") is called before loadInitialDataAndGet()."
                    + " Returning an empty map");
@@ -220,28 +218,24 @@ final class AdditionalSubtypeMapRepository {
    }

    /**
     * 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.
     * Ensures that {@link AdditionalSubtypeMap} is initialized for the given user.
     *
     * @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) {
    static void initializeIfNecessary(@UserIdInt int userId) {
        if (sPerUserMap.contains(userId)) {
            // Fast-pass. If putAndSave() is already called, then do nothing.
            return;
        }
        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;
        synchronized (sMutationLock) {
            // Check the condition again.
            if (!sPerUserMap.contains(userId)) {
                sPerUserMap = sPerUserMap.cloneWithPutOrSelf(userId, map);
            }
            sPerUserMap.put(userId, map);
        }
        return map;
    }

    /**
@@ -255,12 +249,8 @@ final class AdditionalSubtypeMapRepository {
    @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;
            }
            sPerUserMap.put(userId, map);
        synchronized (sMutationLock) {
            sPerUserMap = sPerUserMap.cloneWithPutOrSelf(userId, map);
            sWriter.scheduleWriteTask(userId, map, inputMethodMap);
        }
    }
@@ -277,9 +267,9 @@ final class AdditionalSubtypeMapRepository {

    @AnyThread
    static void remove(@UserIdInt int userId) {
        synchronized (sPerUserMap) {
        synchronized (sMutationLock) {
            sWriter.onUserRemoved(userId);
            sPerUserMap.remove(userId);
            sPerUserMap = sPerUserMap.cloneWithRemoveOrSelf(userId);
        }
    }
}
+183 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.server.inputmethod;


import android.annotation.AnyThread;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.util.SparseArray;

import java.util.function.Consumer;

/**
 * A holder object to expose {@link SparseArray} to multiple threads in a thread-safe manner through
 * "final Field Semantics" defined in JLS 17.5, with only exposing thread-safe methods such as
 * {@link SparseArray#get(int)} and {@link SparseArray#size()} from {@link SparseArray}, and with
 * adding clone-with-update style methods {@link #cloneWithPutOrSelf(int, Object)} and
 * {@link #cloneWithRemoveOrSelf(int)} instead of exposing mutation methods.
 *
 * @param <E> Type of the element
 */
final class ImmutableSparseArray<E> {
    @NonNull
    private final SparseArray<E> mArray;

    private static final ImmutableSparseArray<Object> EMPTY =
            new ImmutableSparseArray<>(new SparseArray<>());

    /**
     * Returns an empty {@link ImmutableSparseArray} instance.
     *
     * @return An empty {@link ImmutableSparseArray} instance.
     * @param <T> Type of the element
     */
    @SuppressWarnings("unchecked")
    @AnyThread
    @NonNull
    static <T> ImmutableSparseArray<T> empty() {
        return (ImmutableSparseArray<T>) EMPTY;
    }

    private ImmutableSparseArray(@NonNull SparseArray<E> array) {
        mArray = array;
    }

    /**
     * @return the size of this array
     */
    @AnyThread
    int size() {
        return mArray.size();
    }

    /**
     * Returns the key of the specified index.
     *
     * @return the key of the specified index
     * @throws ArrayIndexOutOfBoundsException when the index is out of range
     */
    @AnyThread
    int keyAt(int index) {
        return mArray.keyAt(index);
    }

    /**
     * Returns the value of the specified index.
     *
     * @return the value of the specified index
     * @throws ArrayIndexOutOfBoundsException when the index is out of range
     */
    @AnyThread
    @Nullable
    public E valueAt(int index) {
        return mArray.valueAt(index);
    }

    /**
     * Returns the index of the specified key.
     *
     * @return the index of the specified key if exists. Otherwise {@code -1}
     */
    @AnyThread
    int indexOfKey(int key) {
        return mArray.indexOfKey(key);
    }

    /**
     * Returns {@code true} if the given {@code key} exists.
     *
     * @param key the key to be queried
     * @return    {@code true} if the given {@code key} exists
     */
    @AnyThread
    boolean contains(int key) {
        return mArray.contains(key);
    }

    /**
     * Returns the value associated with the {@code key}.
     *
     * @param key the key to be queried
     * @return    the value associated with the {@code key} if exists. Otherwise {@code null}
     */
    @AnyThread
    @Nullable
    E get(int key) {
        return mArray.get(key);
    }

    /**
     * Run {@link Consumer} for each value.
     *
     * @param consumer {@link Consumer} to be called back
     */
    @AnyThread
    void forEach(@NonNull Consumer<E> consumer) {
        final int size = mArray.size();
        for (int i = 0; i < size; ++i) {
            consumer.accept(mArray.valueAt(i));
        }
    }

    /**
     * Returns an instance of {@link ImmutableSparseArray} that has the given key and value on top
     * of items cloned from this instance.
     *
     * @param key   the key to be added
     * @param value the value to be added
     * @return      the same {@link ImmutableSparseArray} instance if there is actually no update.
     *              Otherwise, a new instance of {@link ImmutableSparseArray}
     */
    @AnyThread
    @NonNull
    ImmutableSparseArray<E> cloneWithPutOrSelf(int key, @Nullable E value) {
        final var prevKeyIndex = mArray.indexOfKey(key);
        if (prevKeyIndex >= 0) {
            final var prevValue = mArray.valueAt(prevKeyIndex);
            if (prevValue == value) {
                return this;
            }
        }
        final var clone = mArray.clone();
        clone.put(key, value);
        return new ImmutableSparseArray<>(clone);
    }

    /**
     * Returns an instance of {@link ImmutableSparseArray} that does not have the given key on top
     * of items cloned from this instance.
     *
     * @param key the key to be removed
     * @return    the same {@link ImmutableSparseArray} instance if there is actually no update.
     *            Otherwise, a new instance of {@link ImmutableSparseArray}
     */
    @AnyThread
    @NonNull
    ImmutableSparseArray<E> cloneWithRemoveOrSelf(int key) {
        final int index = indexOfKey(key);
        if (index < 0) {
            return this;
        }
        if (mArray.size() == 1) {
            return empty();
        }
        final var clone = mArray.clone();
        clone.remove(key);
        return new ImmutableSparseArray<>(clone);
    }
}
+2 −2
Original line number Diff line number Diff line
@@ -1063,8 +1063,8 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.

                for (int userId : userIds) {
                    Slog.d(TAG, "Start initialization for user=" + userId);
                    final var additionalSubtypeMap =
                            AdditionalSubtypeMapRepository.ensureInitializedAndGet(userId);
                    AdditionalSubtypeMapRepository.initializeIfNecessary(userId);
                    final var additionalSubtypeMap = AdditionalSubtypeMapRepository.get(userId);
                    final var settings = InputMethodManagerService.queryInputMethodServicesInternal(
                            context, userId, additionalSubtypeMap,
                            DirectBootAwareness.AUTO).getMethodMap();
+9 −14
Original line number Diff line number Diff line
@@ -19,15 +19,13 @@ package com.android.server.inputmethod;
import android.annotation.AnyThread;
import android.annotation.NonNull;
import android.annotation.UserIdInt;
import android.util.SparseArray;

import com.android.internal.annotations.GuardedBy;

final class InputMethodSettingsRepository {
    // TODO(b/352594784): Should we user other lock primitives?
    @GuardedBy("sPerUserMap")
    private static final Object sMutationLock = new Object();

    @NonNull
    private static final SparseArray<InputMethodSettings> sPerUserMap = new SparseArray<>();
    private static volatile ImmutableSparseArray<InputMethodSettings> sPerUserMap =
            ImmutableSparseArray.empty();

    /**
     * Not intended to be instantiated.
@@ -38,10 +36,7 @@ final class InputMethodSettingsRepository {
    @NonNull
    @AnyThread
    static InputMethodSettings get(@UserIdInt int userId) {
        final InputMethodSettings obj;
        synchronized (sPerUserMap) {
            obj = sPerUserMap.get(userId);
        }
        final InputMethodSettings obj = sPerUserMap.get(userId);
        if (obj != null) {
            return obj;
        }
@@ -50,15 +45,15 @@ final class InputMethodSettingsRepository {

    @AnyThread
    static void put(@UserIdInt int userId, @NonNull InputMethodSettings obj) {
        synchronized (sPerUserMap) {
            sPerUserMap.put(userId, obj);
        synchronized (sMutationLock) {
            sPerUserMap = sPerUserMap.cloneWithPutOrSelf(userId, obj);
        }
    }

    @AnyThread
    static void remove(@UserIdInt int userId) {
        synchronized (sPerUserMap) {
            sPerUserMap.remove(userId);
        synchronized (sMutationLock) {
            sPerUserMap = sPerUserMap.cloneWithRemoveOrSelf(userId);
        }
    }
}
+17 −17
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ import android.content.ContentResolver;
import android.provider.Settings;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.SparseArray;

import com.android.internal.annotations.GuardedBy;
import com.android.server.LocalServices;
@@ -38,6 +37,13 @@ import com.android.server.pm.UserManagerInternal;
 * to the persistent value when the user storage is unlocked.</p>
 */
final class SecureSettingsWrapper {

    private static final Object sMutationLock = new Object();

    @NonNull
    private static volatile ImmutableSparseArray<ReaderWriter> sUserMap =
            ImmutableSparseArray.empty();

    @Nullable
    private static volatile ContentResolver sContentResolver = null;

@@ -61,8 +67,8 @@ final class SecureSettingsWrapper {
     */
    @AnyThread
    static void endTestMode() {
        synchronized (sUserMap) {
            sUserMap.clear();
        synchronized (sMutationLock) {
            sUserMap = ImmutableSparseArray.empty();
        }
        sTestMode = false;
    }
@@ -243,10 +249,6 @@ final class SecureSettingsWrapper {
        }
    }

    @GuardedBy("sUserMap")
    @NonNull
    private static final SparseArray<ReaderWriter> sUserMap = new SparseArray<>();

    private static final ReaderWriter NOOP = new ReaderWriter() {
        @Override
        public void putString(String key, String str) {
@@ -282,15 +284,15 @@ final class SecureSettingsWrapper {
    private static ReaderWriter putOrGet(@UserIdInt int userId,
            @NonNull ReaderWriter readerWriter) {
        final boolean isUnlockedUserImpl = readerWriter instanceof UnlockedUserImpl;
        synchronized (sUserMap) {
        synchronized (sMutationLock) {
            final ReaderWriter current = sUserMap.get(userId);
            if (current == null) {
                sUserMap.put(userId, readerWriter);
                sUserMap = sUserMap.cloneWithPutOrSelf(userId, readerWriter);
                return readerWriter;
            }
            // Upgrading from CopyOnWriteImpl to DirectImpl is allowed.
            if (current instanceof LockedUserImpl && isUnlockedUserImpl) {
                sUserMap.put(userId, readerWriter);
                sUserMap = sUserMap.cloneWithPutOrSelf(userId, readerWriter);
                return readerWriter;
            }
            return current;
@@ -300,12 +302,10 @@ final class SecureSettingsWrapper {
    @NonNull
    @AnyThread
    private static ReaderWriter get(@UserIdInt int userId) {
        synchronized (sUserMap) {
        final ReaderWriter readerWriter = sUserMap.get(userId);
        if (readerWriter != null) {
            return readerWriter;
        }
        }
        if (sTestMode) {
            return putOrGet(userId, new FakeReaderWriterImpl());
        }
@@ -363,8 +363,8 @@ final class SecureSettingsWrapper {
     */
    @AnyThread
    static void onUserRemoved(@UserIdInt int userId) {
        synchronized (sUserMap) {
            sUserMap.remove(userId);
        synchronized (sMutationLock) {
            sUserMap = sUserMap.cloneWithRemoveOrSelf(userId);
        }
    }

Loading