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

Commit 824ab708 authored by Martijn Coenen's avatar Martijn Coenen
Browse files

Cache StorageManger#getVolumeList.

Use a PropertyInvalidatedCache to cache results of getVolumeList(),
which is an expensive operation that is frequently called by some apps.

Bug: 323574186
Test: atest StorageManagerTest
Flag: EXEMPT refactor
Change-Id: I071487e30fb3312abdf29163d411a9f34c9370cd
parent 90243296
Loading
Loading
Loading
Loading
+160 −106
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static android.app.AppOpsManager.OP_LEGACY_STORAGE;
import static android.app.AppOpsManager.OP_MANAGE_EXTERNAL_STORAGE;
import static android.app.AppOpsManager.OP_READ_EXTERNAL_STORAGE;
import static android.app.AppOpsManager.OP_READ_MEDIA_IMAGES;
import static android.app.PropertyInvalidatedCache.MODULE_SYSTEM;
import static android.content.ContentResolver.DEPRECATE_DATA_PREFIX;
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
import static android.os.UserHandle.PER_USER_RANGE;
@@ -44,6 +45,7 @@ import android.app.ActivityThread;
import android.app.AppGlobals;
import android.app.AppOpsManager;
import android.app.PendingIntent;
import android.app.PropertyInvalidatedCache;
import android.compat.annotation.UnsupportedAppUsage;
import android.content.ContentResolver;
import android.content.Context;
@@ -276,7 +278,8 @@ public class StorageManager {
            FLAG_STORAGE_SDK,
    })
    @Retention(RetentionPolicy.SOURCE)
    public @interface StorageFlags {}
    public @interface StorageFlags {
    }

    /** {@hide} */
    public static final int FLAG_FOR_WRITE = 1 << 8;
@@ -309,6 +312,44 @@ public class StorageManager {
    @GuardedBy("mDelegates")
    private final ArrayList<StorageEventListenerDelegate> mDelegates = new ArrayList<>();

    static record VolumeListQuery(int mUserId, String mPackageName, int mFlags) {
    }

    private static final PropertyInvalidatedCache.QueryHandler<VolumeListQuery, StorageVolume[]>
            sVolumeListQuery = new PropertyInvalidatedCache.QueryHandler<>() {
                @androidx.annotation.Nullable
                @Override
                public StorageVolume[] apply(@androidx.annotation.NonNull VolumeListQuery query) {
                    final IStorageManager storageManager = IStorageManager.Stub.asInterface(
                            ServiceManager.getService("mount"));
                    if (storageManager == null) {
                        // negative results won't be cached, so we will just try again next time
                        return null;
                    }
                    try {
                        return storageManager.getVolumeList(
                                query.mUserId, query.mPackageName, query.mFlags);
                    } catch (RemoteException e) {
                        throw e.rethrowFromSystemServer();
                    }
                }
            };

    // Generally, the userId and packageName parameters stay pretty constant, but flags may change
    // regularly; we have observed some processes hitting 10+ variations.
    private static final int VOLUME_LIST_CACHE_MAX = 16;

    private static final PropertyInvalidatedCache<VolumeListQuery, StorageVolume[]>
            sVolumeListCache = new PropertyInvalidatedCache<>(
                    new PropertyInvalidatedCache.Args(MODULE_SYSTEM).cacheNulls(false)
                    .api("getVolumeList").maxEntries(VOLUME_LIST_CACHE_MAX), "getVolumeList",
                    sVolumeListQuery);

    /** {@hide} */
    public static void invalidateVolumeListCache() {
        sVolumeListCache.invalidateCache();
    }

    private class StorageEventListenerDelegate extends IStorageEventListener.Stub {
        final Executor mExecutor;
        final StorageEventListener mListener;
@@ -395,7 +436,8 @@ public class StorageManager {

    private class ObbActionListener extends IObbActionListener.Stub {
        @SuppressWarnings("hiding")
        private SparseArray<ObbListenerDelegate> mListeners = new SparseArray<ObbListenerDelegate>();
        private SparseArray<ObbListenerDelegate> mListeners =
                new SparseArray<ObbListenerDelegate>();

        @Override
        public void onObbResult(String filename, int nonce, int status) {
@@ -478,9 +520,9 @@ public class StorageManager {
     * @param looper The {@link android.os.Looper} which events will be received on.
     *
     *               <p>Applications can get instance of this class by calling
     * {@link android.content.Context#getSystemService(java.lang.String)} with an argument
     *               {@link android.content.Context#getSystemService(java.lang.String)} with an
     *               argument
     *               of {@link android.content.Context#STORAGE_SERVICE}.
     *
     * @hide
     */
    @UnsupportedAppUsage
@@ -488,15 +530,16 @@ public class StorageManager {
        mContext = context;
        mResolver = context.getContentResolver();
        mLooper = looper;
        mStorageManager = IStorageManager.Stub.asInterface(ServiceManager.getServiceOrThrow("mount"));
        mStorageManager = IStorageManager.Stub.asInterface(
                ServiceManager.getServiceOrThrow("mount"));
        mAppOps = mContext.getSystemService(AppOpsManager.class);
    }

    /**
     * Registers a {@link android.os.storage.StorageEventListener StorageEventListener}.
     *
     * @param listener A {@link android.os.storage.StorageEventListener StorageEventListener} object.
     *
     * @param listener A {@link android.os.storage.StorageEventListener StorageEventListener}
     *                 object.
     * @hide
     */
    @UnsupportedAppUsage
@@ -516,8 +559,8 @@ public class StorageManager {
    /**
     * Unregisters a {@link android.os.storage.StorageEventListener StorageEventListener}.
     *
     * @param listener A {@link android.os.storage.StorageEventListener StorageEventListener} object.
     *
     * @param listener A {@link android.os.storage.StorageEventListener StorageEventListener}
     *                 object.
     * @hide
     */
    @UnsupportedAppUsage
@@ -558,7 +601,8 @@ public class StorageManager {
         * {@link StorageManager#getStorageVolumes()} to observe the latest
         * value.
         */
        public void onStateChanged(@NonNull StorageVolume volume) { }
        public void onStateChanged(@NonNull StorageVolume volume) {
        }
    }

    /**
@@ -628,8 +672,8 @@ public class StorageManager {

    /**
     * Query if a USB Mass Storage (UMS) host is connected.
     * @return true if UMS host is connected.
     *
     * @return true if UMS host is connected.
     * @hide
     */
    @Deprecated
@@ -640,8 +684,8 @@ public class StorageManager {

    /**
     * Query if a USB Mass Storage (UMS) is enabled on the device.
     * @return true if UMS host is enabled.
     *
     * @return true if UMS host is enabled.
     * @hide
     */
    @Deprecated
@@ -1172,8 +1216,8 @@ public class StorageManager {
    /**
     * This is not the API you're looking for.
     *
     * @see PackageManager#getPrimaryStorageCurrentVolume()
     * @hide
     * @see PackageManager#getPrimaryStorageCurrentVolume()
     */
    public String getPrimaryStorageUuid() {
        try {
@@ -1186,8 +1230,8 @@ public class StorageManager {
    /**
     * This is not the API you're looking for.
     *
     * @see PackageManager#movePrimaryStorage(VolumeInfo)
     * @hide
     * @see PackageManager#movePrimaryStorage(VolumeInfo)
     */
    public void setPrimaryStorageUuid(String volumeUuid, IPackageMoveObserver callback) {
        try {
@@ -1275,6 +1319,7 @@ public class StorageManager {

    /**
     * Gets the state of a volume via its mountpoint.
     *
     * @hide
     */
    @Deprecated
@@ -1389,8 +1434,6 @@ public class StorageManager {
    /** {@hide} */
    @UnsupportedAppUsage
    public static @NonNull StorageVolume[] getVolumeList(int userId, int flags) {
        final IStorageManager storageManager = IStorageManager.Stub.asInterface(
                ServiceManager.getService("mount"));
        try {
            String packageName = ActivityThread.currentOpPackageName();
            if (packageName == null) {
@@ -1406,7 +1449,7 @@ public class StorageManager {
                }
                packageName = packageNames[0];
            }
            return storageManager.getVolumeList(userId, packageName, flags);
            return sVolumeListCache.query(new VolumeListQuery(userId, packageName, flags));
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
@@ -1414,6 +1457,7 @@ public class StorageManager {

    /**
     * Returns list of paths for all mountable volumes.
     *
     * @hide
     */
    @Deprecated
@@ -1711,7 +1755,8 @@ public class StorageManager {
        return false;
    }

    /** {@hide}
    /**
     * {@hide}
     * Is this device encrypted?
     * <p>
     * Note: all devices launching with Android 10 (API level 29) or later are
@@ -1724,8 +1769,10 @@ public class StorageManager {
        return RoSystemProperties.CRYPTO_ENCRYPTED;
    }

    /** {@hide}
    /**
     * {@hide}
     * Does this device have file-based encryption (FBE) enabled?
     *
     * @return true if the device has file-based encryption enabled.
     */
    public static boolean isFileEncrypted() {
@@ -1759,8 +1806,8 @@ public class StorageManager {
    }

    /**
     * @deprecated disabled now that FUSE has been replaced by sdcardfs
     * @hide
     * @deprecated disabled now that FUSE has been replaced by sdcardfs
     */
    @Deprecated
    public static File maybeTranslateEmulatedPathToInternal(File path) {
@@ -1790,6 +1837,7 @@ public class StorageManager {

    /**
     * Check that given app holds both permission and appop.
     *
     * @hide
     */
    public static boolean checkPermissionAndAppOp(Context context, boolean enforce, int pid,
@@ -1800,6 +1848,7 @@ public class StorageManager {

    /**
     * Check that given app holds both permission and appop but do not noteOp.
     *
     * @hide
     */
    public static boolean checkPermissionAndCheckOp(Context context, boolean enforce,
@@ -1810,6 +1859,7 @@ public class StorageManager {

    /**
     * Check that given app holds both permission and appop.
     *
     * @hide
     */
    private static boolean checkPermissionAndAppOp(Context context, boolean enforce, int pid,
@@ -1877,7 +1927,9 @@ public class StorageManager {
                // Legacy apps technically have the access granted by this op,
                // even when the op is denied
                if ((mAppOps.checkOpNoThrow(OP_LEGACY_STORAGE, uid,
                        packageName) == AppOpsManager.MODE_ALLOWED)) return true;
                        packageName) == AppOpsManager.MODE_ALLOWED)) {
                    return true;
                }

                if (enforce) {
                    throw new SecurityException("Op " + AppOpsManager.opToName(op) + " "
@@ -2014,7 +2066,6 @@ public class StorageManager {
     *                 returned file descriptor.
     * @param handler  Handler that invokes callback methods.
     * @return Seekable ParcelFileDescriptor.
     * @throws IOException
     */
    public @NonNull ParcelFileDescriptor openProxyFileDescriptor(
            int mode, ProxyFileDescriptorCallback callback, Handler handler)
@@ -2115,16 +2166,19 @@ public class StorageManager {
    })
    @Retention(RetentionPolicy.SOURCE)
    /** @hide */
    public @interface MountMode {}
    public @interface MountMode {
    }

    /**
     * No external storage should be mounted.
     *
     * @hide
     */
    @SystemApi
    public static final int MOUNT_MODE_EXTERNAL_NONE = IVold.REMOUNT_MODE_NONE;
    /**
     * Default external storage should be mounted.
     *
     * @hide
     */
    @SystemApi
@@ -2132,12 +2186,14 @@ public class StorageManager {
    /**
     * Mount mode for package installers which should give them access to
     * all obb dirs in addition to their package sandboxes
     *
     * @hide
     */
    @SystemApi
    public static final int MOUNT_MODE_EXTERNAL_INSTALLER = IVold.REMOUNT_MODE_INSTALLER;
    /**
     * The lower file system should be bind mounted directly on external storage
     *
     * @hide
     */
    @SystemApi
@@ -2146,6 +2202,7 @@ public class StorageManager {
    /**
     * Use the regular scoped storage filesystem, but Android/ should be writable.
     * Used to support the applications hosting DownloadManager and the MTP server.
     *
     * @hide
     */
    @SystemApi
@@ -2164,10 +2221,10 @@ public class StorageManager {
     * this flag to take effect.
     * </p>
     *
     * @hide
     * @see #getAllocatableBytes(UUID, int)
     * @see #allocateBytes(UUID, long, int)
     * @see #allocateBytes(FileDescriptor, long, int)
     * @hide
     */
    @RequiresPermission(android.Manifest.permission.ALLOCATE_AGGRESSIVE)
    @SystemApi
@@ -2194,6 +2251,7 @@ public class StorageManager {
     * freeable cached space when determining allocatable space.
     *
     * Intended for use with {@link #getAllocatableBytes()}.
     *
     * @hide
     */
    public static final int FLAG_ALLOCATE_NON_CACHE_ONLY = 1 << 3;
@@ -2203,6 +2261,7 @@ public class StorageManager {
     * cached space when determining allocatable space.
     *
     * Intended for use with {@link #getAllocatableBytes()}.
     *
     * @hide
     */
    public static final int FLAG_ALLOCATE_CACHE_ONLY = 1 << 4;
@@ -2216,7 +2275,8 @@ public class StorageManager {
            FLAG_ALLOCATE_CACHE_ONLY,
    })
    @Retention(RetentionPolicy.SOURCE)
    public @interface AllocateFlags {}
    public @interface AllocateFlags {
    }

    /**
     * Return the maximum number of new bytes that your app can allocate for
@@ -2332,10 +2392,9 @@ public class StorageManager {
     * These mount modes specify different views and access levels for
     * different apps on external storage.
     *
     * @return {@code MountMode} for the given uid and packageName.
     * @params uid UID of the application
     * @params packageName name of the package
     * @return {@code MountMode} for the given uid and packageName.
     *
     * @hide
     */
    @RequiresPermission(android.Manifest.permission.WRITE_MEDIA_STORAGE)
@@ -2505,7 +2564,8 @@ public class StorageManager {
            QUOTA_TYPE_MEDIA_VIDEO,
            QUOTA_TYPE_MEDIA_IMAGE,
    })
    public @interface QuotaType {}
    public @interface QuotaType {
    }

    private static native boolean setQuotaProjectId(String path, long projectId);

@@ -2536,11 +2596,9 @@ public class StorageManager {
     * @param quotaType the quota type of the file; this is based on the
     *                  {@code QuotaType} constants, eg
     *                  {@code StorageManager.QUOTA_TYPE_MEDIA_AUDIO}
     *
     * @throws IllegalArgumentException if {@code quotaType} does not correspond to a valid
     *                                  quota type.
     * @throws IOException              if the quota type could not be updated.
     *
     * @hide
     */
    @SystemApi
@@ -2616,7 +2674,6 @@ public class StorageManager {
     * permissions of a directory to what they should anyway be.
     *
     * @param path the path for which we should fix up the permissions
     *
     * @hide
     */
    public void fixupAppDir(@NonNull File path) {
@@ -2826,7 +2883,8 @@ public class StorageManager {
            APP_IO_BLOCKED_REASON_TRANSCODING,
            APP_IO_BLOCKED_REASON_UNKNOWN,
    })
    public @interface AppIoBlockedReason {}
    public @interface AppIoBlockedReason {
    }

    /**
     * Notify the system that an app with {@code uid} and {@code tid} is blocked on an IO request on
@@ -2842,7 +2900,6 @@ public class StorageManager {
     * @param uid        the UID of the app blocked on IO
     * @param tid        the tid of the app blocked on IO
     * @param reason     the reason the app is blocked on IO
     *
     * @hide
     */
    @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
@@ -2869,7 +2926,6 @@ public class StorageManager {
     * @param uid        the UID of the app resuming IO
     * @param tid        the tid of the app resuming IO
     * @param reason     the reason the app is resuming IO
     *
     * @hide
     */
    @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES)
@@ -2893,7 +2949,6 @@ public class StorageManager {
     * @param uid        the UID of the app to check IO blocked status
     * @param tid        the tid of the app to check IO blocked status
     * @param reason     the reason to check IO blocked status for
     *
     * @hide
     */
    @TestApi
@@ -2962,7 +3017,6 @@ public class StorageManager {
     * information is available, -1 is returned.
     *
     * @return Percentage of the remaining useful lifetime of the internal storage device.
     *
     * @hide
     */
    @FlaggedApi(Flags.FLAG_STORAGE_LIFETIME_API)
+9 −0
Original line number Diff line number Diff line
@@ -161,7 +161,9 @@ import com.android.server.storage.AppFuseBridge;
import com.android.server.storage.StorageSessionController;
import com.android.server.storage.StorageSessionController.ExternalStorageServiceException;
import com.android.server.storage.WatchedVolumeInfo;
import com.android.server.utils.Watchable;
import com.android.server.utils.WatchedArrayMap;
import com.android.server.utils.Watcher;
import com.android.server.wm.ActivityTaskManagerInternal;
import com.android.server.wm.ActivityTaskManagerInternal.ScreenObserver;

@@ -1964,6 +1966,13 @@ class StorageManagerService extends IStorageManager.Stub
        mContext = context;
        mCallbacks = new Callbacks(FgThread.get().getLooper());

        mVolumes.registerObserver(new Watcher() {
            @Override
            public void onChange(Watchable what) {
                // When we change the list or any volume contained in it, invalidate the cache
                StorageManager.invalidateVolumeListCache();
            }
        });
        HandlerThread hthread = new HandlerThread(TAG);
        hthread.start();
        mHandler = new StorageManagerServiceHandler(hthread.getLooper());
+3 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import static com.google.common.truth.Truth.assertWithMessage;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.spy;

import android.app.PropertyInvalidatedCache;
import android.content.Context;
import android.multiuser.Flags;
import android.os.UserManager;
@@ -75,6 +76,8 @@ public class StorageManagerServiceTest {

    @Before
    public void setFixtures() {
        PropertyInvalidatedCache.disableForTestMode();

        // Called when WatchedUserStates is constructed
        doNothing().when(() -> UserManager.invalidateIsUserUnlockedCache());