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

Commit 9cb1e9a7 authored by Pinyao Ting's avatar Pinyao Ting
Browse files

Use more fine-grained lock when persisting shortcuts to disk 

Currently we are holding service-level lock whenever a process talks to
ShortcutService via any of the public apis, which unnecessarily
decreases the overall throughput.

At the time ShortcutService was implemented, all shortcuts from various
packages were persisted into a single xml file on a write-through basis.
This was the main reason a single synchoronization lock was required
since two parallel updates might corrupt the xml file.

Since that shortcuts from different packages are moved into separate xml
files, in this CL we:
1. Only write the shortcuts from individual packages into corresponding
   xml file after there's a change to the shortcuts in that package, as
   opposed to writing every shortcuts from every packages under that
   user into corresponding xml files, which was rather inefficient.
2. The write operation is still scheduled as a background job, but it no
   longer holds the service-level lock. Instead it would only hold
   package level lock since it is only writing into the xml from that
   specific package.

Given that file I/O is the most time-consuming task in ShortcutService,
refrain from holding service-level lock when performing these type of
tasks should help overall system throughput.

Bug: 186011943
Test: atest CtsShortcutManagerTestCases
Change-Id: I773f4c1d7a39e26030a78ee57f29bf0f33848614
parent b45b8bdc

services/core/java/com/android/server/pm/ShortcutLauncher.java

+10 −0
Original line number Diff line number Diff line
@@ -420,4 +420,14 @@ class ShortcutLauncher extends ShortcutPackageItem { 
    ArraySet<String> getAllPinnedShortcutsForTest(String packageName, int packageUserId) {

        return new ArraySet<>(mPinnedShortcuts.get(PackageWithUser.of(packageUserId, packageName)));

    }



    @Override

    protected File getShortcutPackageItemFile() {

        final File path = new File(mShortcutUser.mService.injectUserDataPath(

                mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_LUANCHERS);

        // Package user id and owner id can have different values for ShortcutLaunchers. Adding

        // user Id to the file name to create a unique path. Owner id is used in the root path.

        final String fileName = getPackageName() + getPackageUserId() + ".xml";

        return new File(path, fileName);

    }

}

services/core/java/com/android/server/pm/ShortcutPackage.java

+20 −18
Original line number Diff line number Diff line
@@ -160,8 +160,6 @@ class ShortcutPackage extends ShortcutPackageItem { 
    private static final String KEY_BITMAPS = "bitmaps";

    private static final String KEY_BITMAP_BYTES = "bitmapBytes";



    private final Object mLock = new Object();



    private final Executor mExecutor;



    /**
@@ -779,7 +777,7 @@ class ShortcutPackage extends ShortcutPackageItem { 
            return false;

        }

        mApiCallCount++;

        s.scheduleSaveUser(getOwnerUserId());

        scheduleSave();

        return true;

    }
@@ -789,7 +787,7 @@ class ShortcutPackage extends ShortcutPackageItem { 
        }

        if (mApiCallCount > 0) {

            mApiCallCount = 0;

            mShortcutUser.mService.scheduleSaveUser(getOwnerUserId());

            scheduleSave();

        }

    }
@@ -1886,15 +1884,12 @@ class ShortcutPackage extends ShortcutPackageItem { 


        final ShortcutPackage ret = new ShortcutPackage(shortcutUser,

                shortcutUser.getUserId(), packageName);



        synchronized (ret.mLock) {

            ret.mIsAppSearchSchemaUpToDate = ShortcutService.parseIntAttribute(

                    parser, ATTR_SCHEMA_VERSON, 0) == AppSearchShortcutInfo.SCHEMA_VERSION;

        }

        ret.mApiCallCount =

                ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT);

        ret.mLastResetTime =

                ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET);

        ret.mApiCallCount = ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT);

        ret.mLastResetTime = ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET);





        final int outerDepth = parser.getDepth();
@@ -2436,9 +2431,9 @@ class ShortcutPackage extends ShortcutPackageItem { 
                        })));

    }



    void persistsAllShortcutsAsync() {

        synchronized (mLock) {

            final Map<String, ShortcutInfo> copy = mShortcuts;

    @Override

    void scheduleSaveToAppSearchLocked() {

        final Map<String, ShortcutInfo> copy = new ArrayMap<>(mShortcuts);

        if (!mTransientShortcuts.isEmpty()) {

            copy.putAll(mTransientShortcuts);

            mTransientShortcuts.clear();
@@ -2446,7 +2441,6 @@ class ShortcutPackage extends ShortcutPackageItem { 
        saveShortcutsAsync(copy.values().stream().filter(ShortcutInfo::usesQuota).collect(

                Collectors.toList()));

    }

    }



    private void saveShortcutsAsync(

            @NonNull final Collection<ShortcutInfo> shortcuts) {
@@ -2544,4 +2538,12 @@ class ShortcutPackage extends ShortcutPackageItem { 
            Binder.restoreCallingIdentity(callingIdentity);

        }

    }



    @Override

    protected File getShortcutPackageItemFile() {

        final File path = new File(mShortcutUser.mService.injectUserDataPath(

                mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_PACKAGES);

        final String fileName = getPackageName() + ".xml";

        return new File(path, fileName);

    }

}

services/core/java/com/android/server/pm/ShortcutPackageItem.java

+45 −4
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.util.Slog; 
import android.util.TypedXmlSerializer;

import android.util.Xml;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.util.Preconditions;



import org.json.JSONException;
@@ -36,7 +37,7 @@ import java.nio.charset.StandardCharsets; 
import java.util.Objects;



/**

 * All methods should be guarded by {@code #mShortcutUser.mService.mLock}.

 * All methods should be either guarded by {@code #mShortcutUser.mService.mLock} or {@code #mLock}.

 */

abstract class ShortcutPackageItem {

    private static final String TAG = ShortcutService.TAG;
@@ -49,6 +50,8 @@ abstract class ShortcutPackageItem { 


    protected ShortcutUser mShortcutUser;



    protected final Object mLock = new Object();



    protected ShortcutPackageItem(@NonNull ShortcutUser shortcutUser,

            int packageUserId, @NonNull String packageName,

            @NonNull ShortcutPackageInfo packageInfo) {
@@ -98,7 +101,7 @@ abstract class ShortcutPackageItem { 
        }

        final ShortcutService s = mShortcutUser.mService;

        mPackageInfo.refreshSignature(s, this);

        s.scheduleSaveUser(getOwnerUserId());

        scheduleSave();

    }



    public void attemptToRestoreIfNeededAndSave() {
@@ -138,7 +141,7 @@ abstract class ShortcutPackageItem { 
        // Either way, it's no longer a shadow.

        mPackageInfo.setShadow(false);



        s.scheduleSaveUser(mPackageUserId);

        scheduleSave();

    }



    protected abstract boolean canRestoreAnyVersion();
@@ -148,7 +151,8 @@ abstract class ShortcutPackageItem { 
    public abstract void saveToXml(@NonNull TypedXmlSerializer out, boolean forBackup)

            throws IOException, XmlPullParserException;



    public void saveToFile(File path, boolean forBackup) {

    @GuardedBy("mLock")

    public void saveToFileLocked(File path, boolean forBackup) {

        final AtomicFile file = new AtomicFile(path);

        FileOutputStream os = null;

        try {
@@ -176,6 +180,11 @@ abstract class ShortcutPackageItem { 
        }

    }



    @GuardedBy("mLock")

    void scheduleSaveToAppSearchLocked() {



    }



    public JSONObject dumpCheckin(boolean clear) throws JSONException {

        final JSONObject result = new JSONObject();

        result.put(KEY_NAME, mPackageName);
@@ -187,4 +196,36 @@ abstract class ShortcutPackageItem { 
     */

    public void verifyStates() {

    }



    public void scheduleSave() {

        mShortcutUser.mService.injectPostToHandlerDebounced(

                mSaveShortcutPackageRunner, mSaveShortcutPackageRunner);

    }



    private final Runnable mSaveShortcutPackageRunner = this::saveShortcutPackageItem;



    void saveShortcutPackageItem() {

        // Wait for bitmap saves to conclude before proceeding to saving shortcuts.

        mShortcutUser.mService.waitForBitmapSaves();

        // Save each ShortcutPackageItem in a separate Xml file.

        final File path = getShortcutPackageItemFile();

        if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) {

            Slog.d(TAG, "Saving package item " + getPackageName() + " to " + path);

        }

        synchronized (mLock) {

            path.getParentFile().mkdirs();

            // TODO: Since we are persisting shortcuts into AppSearch, we should read from/write to

            //  AppSearch as opposed to maintaining a separate XML file.

            saveToFileLocked(path, false /*forBackup*/);

            scheduleSaveToAppSearchLocked();

        }

    }



    void removeShortcutPackageItem() {

        synchronized (mLock) {

            getShortcutPackageItemFile().delete();

        }

    }



    protected abstract File getShortcutPackageItemFile();

}

services/core/java/com/android/server/pm/ShortcutService.java

+13 −19
Original line number Diff line number Diff line
@@ -748,7 +748,7 @@ public class ShortcutService extends IShortcutService.Stub { 
        getUserShortcutsLocked(userId).cancelAllInFlightTasks();



        // Save all dirty information.

        saveDirtyInfo(false);

        saveDirtyInfo();



        // Unload

        mUsers.delete(userId);
@@ -1203,10 +1203,6 @@ public class ShortcutService extends IShortcutService.Stub { 


    @VisibleForTesting

    void saveDirtyInfo() {

        saveDirtyInfo(true);

    }



    private void saveDirtyInfo(boolean saveShortcutsInAppSearch) {

        if (DEBUG || DEBUG_REBOOT) {

            Slog.d(TAG, "saveDirtyInfo");

        }
@@ -1221,10 +1217,6 @@ public class ShortcutService extends IShortcutService.Stub { 
                    if (userId == UserHandle.USER_NULL) { // USER_NULL for base state.

                        saveBaseStateLocked();

                    } else {

                        if (saveShortcutsInAppSearch) {

                            getUserShortcutsLocked(userId).forAllPackages(

                                    ShortcutPackage::persistsAllShortcutsAsync);

                        }

                        saveUserLocked(userId);

                    }

                }
@@ -1816,7 +1808,7 @@ public class ShortcutService extends IShortcutService.Stub { 
        }

        injectPostToHandlerDebounced(sp, notifyListenerRunnable(packageName, userId));

        notifyShortcutChangeCallbacks(packageName, userId, changedShortcuts, removedShortcuts);

        scheduleSaveUser(userId);

        sp.scheduleSave();

    }



    private void notifyListeners(@NonNull final String packageName, @UserIdInt final int userId) {
@@ -2878,13 +2870,12 @@ public class ShortcutService extends IShortcutService.Stub { 


        final ShortcutUser user = getUserShortcutsLocked(owningUserId);

        boolean doNotify = false;



        // First, remove the package from the package list (if the package is a publisher).

        if (packageUserId == owningUserId) {

            if (user.removePackage(packageName) != null) {

        final ShortcutPackage sp = (packageUserId == owningUserId)

                ? user.removePackage(packageName) : null;

        if (sp != null) {

            doNotify = true;

        }

        }



        // Also remove from the launcher list (if the package is a launcher).

        user.removeLauncher(packageUserId, packageName);
@@ -2906,6 +2897,10 @@ public class ShortcutService extends IShortcutService.Stub { 
            // notifyListeners.

            user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true);

        }

        if (!appStillExists && (packageUserId == owningUserId) && sp != null) {

            // If the app is removed altogether, we can get rid of the xml as well

            injectPostToHandler(() -> sp.removeShortcutPackageItem());

        }



        if (!wasUserLoaded) {

            // Note this will execute the scheduled save.
@@ -3788,7 +3783,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                if (mHandler.hasCallbacks(mSaveDirtyInfoRunner)) {

                    mHandler.removeCallbacks(mSaveDirtyInfoRunner);

                    forEachLoadedUserLocked(ShortcutUser::cancelAllInFlightTasks);

                    saveDirtyInfo(false);

                    saveDirtyInfo();

                }

                mShutdown.set(true);

            }
@@ -4457,7 +4452,7 @@ public class ShortcutService extends IShortcutService.Stub { 


            // Save to the filesystem.

            scheduleSaveUser(userId);

            saveDirtyInfo(false);

            saveDirtyInfo();



            // Note, in case of backup, we don't have to wait on bitmap saving, because we don't

            // back up bitmaps anyway.
@@ -5352,8 +5347,7 @@ public class ShortcutService extends IShortcutService.Stub { 
        }

    }



    @VisibleForTesting

    void waitForBitmapSavesForTest() {

    void waitForBitmapSaves() {

        synchronized (mLock) {

            mShortcutBitmapSaver.waitForAllSavesLocked();

        }

services/core/java/com/android/server/pm/ShortcutUser.java

+1 −26
Original line number Diff line number Diff line
@@ -407,33 +407,8 @@ class ShortcutUser { 
            }

            spi.saveToXml(out, forBackup);

        } else {

            // Save each ShortcutPackageItem in a separate Xml file.

            final File path = getShortcutPackageItemFile(spi);

            if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) {

                Slog.d(TAG, "Saving package item " + spi.getPackageName() + " to " + path);

            spi.saveShortcutPackageItem();

        }



            path.getParentFile().mkdirs();

            spi.saveToFile(path, forBackup);

        }

    }



    private File getShortcutPackageItemFile(ShortcutPackageItem spi) {

        boolean isShortcutLauncher = spi instanceof ShortcutLauncher;



        final File path = new File(mService.injectUserDataPath(mUserId),

                isShortcutLauncher ? DIRECTORY_LUANCHERS : DIRECTORY_PACKAGES);



        final String fileName;

        if (isShortcutLauncher) {

            // Package user id and owner id can have different values for ShortcutLaunchers. Adding

            // user Id to the file name to create a unique path. Owner id is used in the root path.

            fileName = spi.getPackageName() + spi.getPackageUserId() + ".xml";

        } else {

            fileName = spi.getPackageName() + ".xml";

        }



        return new File(path, fileName);

    }



    public static ShortcutUser loadFromXml(ShortcutService s, TypedXmlPullParser parser, int userId,