From 2cdea6871c275a864f6051b6f56965bdcf0b3fce Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Thu, 1 Dec 2022 14:11:48 -0800 Subject: Split out the apk assets cache lock The apk assets cache is separate from the rest of the class and can be protected separately, reducing the lock contention + shorten the lock scope for nearby simple functions Bug: 259941466 Test: build + boot Change-Id: I8e58dbae443f1b22bc7c3f486e53db430c0f4270 --- core/java/android/app/ResourcesManager.java | 93 +++++++++++++++-------------- 1 file changed, 49 insertions(+), 44 deletions(-) diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java index 322176100e92..7e6386e47494 100644 --- a/core/java/android/app/ResourcesManager.java +++ b/core/java/android/app/ResourcesManager.java @@ -41,6 +41,7 @@ import android.os.Trace; import android.util.ArrayMap; import android.util.ArraySet; import android.util.DisplayMetrics; +import android.util.IndentingPrintWriter; import android.util.Log; import android.util.Pair; import android.util.Slog; @@ -51,7 +52,6 @@ import android.window.WindowContext; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; -import com.android.internal.util.IndentingPrintWriter; import java.io.IOException; import java.io.PrintWriter; @@ -173,6 +173,7 @@ public class ResourcesManager { /** * The ApkAssets that are being referenced in the wild that we can reuse. + * Used as a lock for itself as well. */ private final ArrayMap> mCachedApkAssets = new ArrayMap<>(); @@ -286,42 +287,43 @@ public class ResourcesManager { * try as hard as possible to release any open FDs. */ public void invalidatePath(String path) { + final List implsToFlush = new ArrayList<>(); synchronized (mLock) { - int count = 0; - for (int i = mResourceImpls.size() - 1; i >= 0; i--) { final ResourcesKey key = mResourceImpls.keyAt(i); if (key.isPathReferenced(path)) { - ResourcesImpl impl = mResourceImpls.removeAt(i).get(); - if (impl != null) { - impl.flushLayoutCache(); + ResourcesImpl resImpl = mResourceImpls.removeAt(i).get(); + if (resImpl != null) { + implsToFlush.add(resImpl); } - count++; } } - - Log.i(TAG, "Invalidated " + count + " asset managers that referenced " + path); - + } + for (int i = 0; i < implsToFlush.size(); i++) { + implsToFlush.get(i).flushLayoutCache(); + } + final List assetsToClose = new ArrayList<>(); + synchronized (mCachedApkAssets) { for (int i = mCachedApkAssets.size() - 1; i >= 0; i--) { final ApkKey key = mCachedApkAssets.keyAt(i); if (key.path.equals(path)) { final WeakReference apkAssetsRef = mCachedApkAssets.removeAt(i); - if (apkAssetsRef == null) { - continue; - } - final ApkAssets apkAssets = apkAssetsRef.get(); + final ApkAssets apkAssets = apkAssetsRef != null ? apkAssetsRef.get() : null; if (apkAssets != null) { - apkAssets.close(); + assetsToClose.add(apkAssets); } } } } + for (int i = 0; i < assetsToClose.size(); i++) { + assetsToClose.get(i).close(); + } + Log.i(TAG, + "Invalidated " + implsToFlush.size() + " asset managers that referenced " + path); } public Configuration getConfiguration() { - synchronized (mLock) { - return mResConfiguration; - } + return mResConfiguration; } @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) @@ -406,14 +408,12 @@ public class ResourcesManager { * @param resources The {@link Resources} backing the display adjustments. */ public Display getAdjustedDisplay(final int displayId, Resources resources) { - synchronized (mLock) { - final DisplayManagerGlobal dm = DisplayManagerGlobal.getInstance(); - if (dm == null) { - // may be null early in system startup - return null; - } - return dm.getCompatibleDisplay(displayId, resources); + final DisplayManagerGlobal dm = DisplayManagerGlobal.getInstance(); + if (dm == null) { + // may be null early in system startup + return null; } + return dm.getCompatibleDisplay(displayId, resources); } /** @@ -451,7 +451,7 @@ public class ResourcesManager { // Optimistically check if this ApkAssets exists somewhere else. final WeakReference apkAssetsRef; - synchronized (mLock) { + synchronized (mCachedApkAssets) { apkAssetsRef = mCachedApkAssets.get(key); } if (apkAssetsRef != null) { @@ -474,7 +474,7 @@ public class ResourcesManager { apkAssets = ApkAssets.loadFromPath(key.path, flags); } - synchronized (mLock) { + synchronized (mCachedApkAssets) { mCachedApkAssets.put(key, new WeakReference<>(apkAssets)); } @@ -586,28 +586,33 @@ public class ResourcesManager { * @hide */ public void dump(String prefix, PrintWriter printWriter) { + final int references; + final int resImpls; synchronized (mLock) { - IndentingPrintWriter pw = new IndentingPrintWriter(printWriter, " "); - for (int i = 0; i < prefix.length() / 2; i++) { - pw.increaseIndent(); - } - - pw.println("ResourcesManager:"); - pw.increaseIndent(); - pw.print("total apks: "); - pw.println(countLiveReferences(mCachedApkAssets.values())); - - pw.print("resources: "); - - int references = countLiveReferences(mResourceReferences); + int refs = countLiveReferences(mResourceReferences); for (ActivityResources activityResources : mActivityResourceReferences.values()) { - references += activityResources.countLiveReferences(); + refs += activityResources.countLiveReferences(); } - pw.println(references); + references = refs; + resImpls = countLiveReferences(mResourceImpls.values()); + } + final int liveAssets; + synchronized (mCachedApkAssets) { + liveAssets = countLiveReferences(mCachedApkAssets.values()); + } - pw.print("resource impls: "); - pw.println(countLiveReferences(mResourceImpls.values())); + final var pw = new IndentingPrintWriter(printWriter, " "); + for (int i = 0; i < prefix.length() / 2; i++) { + pw.increaseIndent(); } + pw.println("ResourcesManager:"); + pw.increaseIndent(); + pw.print("total apks: "); + pw.println(liveAssets); + pw.print("resources: "); + pw.println(references); + pw.print("resource impls: "); + pw.println(resImpls); } private Configuration generateConfig(@NonNull ResourcesKey key) { -- cgit v1.2.3-59-g8ed1b