From a74111f81c993b442c474a56e28f1b008ab61413 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 2 Jun 2020 15:10:13 -0700 Subject: [PATCH] Use a ReferenceQueue to prune weak references Currently ResourcesManager iterates over all of its weak references to Resources objects and calls Reference#get to determine if the object has been garbage collected. This method of pruning WeakReferences causes reference locks to be acquired in the native layer and causes lock contention when "references are being processed" by the garbage collector. This change uses a ReferenceQueue to determine if a reference has been garbage collected which should improve performance of ResourcesManager#getResources when the system is under memory pressure. By only removing garbage collected references when creating new Resources objects, the lists grow larger and are periodically pruned rather than attempting to prune entries from the list every getResources invocation. Bug: 157575833 Test: used debugger to observe pruning happening correctly hange-Id: I3277e80edfa441d24de165e738d33c4fac6b4121 Change-Id: I3277e80edfa441d24de165e738d33c4fac6b4121 --- core/java/android/app/ResourcesManager.java | 53 ++++++++++----------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java index 1aae04db32d05..4cba6ea749558 100644 --- a/core/java/android/app/ResourcesManager.java +++ b/core/java/android/app/ResourcesManager.java @@ -52,10 +52,13 @@ import com.android.internal.util.IndentingPrintWriter; import java.io.IOException; import java.io.PrintWriter; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.WeakHashMap; @@ -69,12 +72,6 @@ public class ResourcesManager { private static ResourcesManager sResourcesManager; - /** - * Predicate that returns true if a WeakReference is gc'ed. - */ - private static final Predicate> sEmptyReferencePredicate = - weakRef -> weakRef == null || weakRef.get() == null; - /** * The global compatibility settings. */ @@ -100,6 +97,7 @@ public class ResourcesManager { */ @UnsupportedAppUsage private final ArrayList> mResourceReferences = new ArrayList<>(); + private final ReferenceQueue mResourcesReferencesQueue = new ReferenceQueue<>(); private static class ApkKey { public final String path; @@ -155,6 +153,7 @@ public class ResourcesManager { } public final Configuration overrideConfig = new Configuration(); public final ArrayList> activityResources = new ArrayList<>(); + final ReferenceQueue activityResourcesQueue = new ReferenceQueue<>(); } /** @@ -667,12 +666,15 @@ public class ResourcesManager { @NonNull CompatibilityInfo compatInfo) { final ActivityResources activityResources = getOrCreateActivityResourcesStructLocked( activityToken); + cleanupReferences(activityResources.activityResources, + activityResources.activityResourcesQueue); Resources resources = compatInfo.needsCompatResources() ? new CompatResources(classLoader) : new Resources(classLoader); resources.setImpl(impl); resources.setCallbacks(mUpdateCallbacks); - activityResources.activityResources.add(new WeakReference<>(resources)); + activityResources.activityResources.add( + new WeakReference<>(resources, activityResources.activityResourcesQueue)); if (DEBUG) { Slog.d(TAG, "- creating new ref=" + resources); Slog.d(TAG, "- setting ref=" + resources + " with impl=" + impl); @@ -682,11 +684,13 @@ public class ResourcesManager { private @NonNull Resources createResourcesLocked(@NonNull ClassLoader classLoader, @NonNull ResourcesImpl impl, @NonNull CompatibilityInfo compatInfo) { + cleanupReferences(mResourceReferences, mResourcesReferencesQueue); + Resources resources = compatInfo.needsCompatResources() ? new CompatResources(classLoader) : new Resources(classLoader); resources.setImpl(impl); resources.setCallbacks(mUpdateCallbacks); - mResourceReferences.add(new WeakReference<>(resources)); + mResourceReferences.add(new WeakReference<>(resources, mResourcesReferencesQueue)); if (DEBUG) { Slog.d(TAG, "- creating new ref=" + resources); Slog.d(TAG, "- setting ref=" + resources + " with impl=" + impl); @@ -752,7 +756,6 @@ public class ResourcesManager { updateResourcesForActivity(token, overrideConfig, displayId, false /* movedToDifferentDisplay */); - cleanupReferences(token); rebaseKeyForActivity(token, key); synchronized (this) { @@ -778,10 +781,6 @@ public class ResourcesManager { final ActivityResources activityResources = getOrCreateActivityResourcesStructLocked(activityToken); - // Clean up any dead references so they don't pile up. - ArrayUtils.unstableRemoveIf(activityResources.activityResources, - sEmptyReferencePredicate); - // Rebase the key's override config on top of the Activity's base override. if (key.hasOverrideConfiguration() && !activityResources.overrideConfig.equals(Configuration.EMPTY)) { @@ -794,21 +793,21 @@ public class ResourcesManager { /** * Check WeakReferences and remove any dead references so they don't pile up. - * @param activityToken optional token to clean up Activity resources */ - private void cleanupReferences(IBinder activityToken) { - synchronized (this) { - if (activityToken != null) { - ActivityResources activityResources = mActivityResourceReferences.get( - activityToken); - if (activityResources != null) { - ArrayUtils.unstableRemoveIf(activityResources.activityResources, - sEmptyReferencePredicate); - } - } else { - ArrayUtils.unstableRemoveIf(mResourceReferences, sEmptyReferencePredicate); - } + private static void cleanupReferences(ArrayList> references, + ReferenceQueue referenceQueue) { + Reference enduedRef = referenceQueue.poll(); + if (enduedRef == null) { + return; } + + final HashSet> deadReferences = new HashSet<>(); + for (; enduedRef != null; enduedRef = referenceQueue.poll()) { + deadReferences.add(enduedRef); + } + + ArrayUtils.unstableRemoveIf(references, + (ref) -> ref == null || deadReferences.contains(ref)); } /** @@ -896,8 +895,6 @@ public class ResourcesManager { loaders == null ? null : loaders.toArray(new ResourcesLoader[0])); classLoader = classLoader != null ? classLoader : ClassLoader.getSystemClassLoader(); - cleanupReferences(activityToken); - if (activityToken != null) { rebaseKeyForActivity(activityToken, key); }