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
diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 1aae04d..4cba6ea 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -52,10 +52,13 @@
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;
@@ -70,12 +73,6 @@
private static ResourcesManager sResourcesManager;
/**
- * Predicate that returns true if a WeakReference is gc'ed.
- */
- private static final Predicate<WeakReference<Resources>> sEmptyReferencePredicate =
- weakRef -> weakRef == null || weakRef.get() == null;
-
- /**
* The global compatibility settings.
*/
private CompatibilityInfo mResCompatibilityInfo;
@@ -100,6 +97,7 @@
*/
@UnsupportedAppUsage
private final ArrayList<WeakReference<Resources>> mResourceReferences = new ArrayList<>();
+ private final ReferenceQueue<Resources> mResourcesReferencesQueue = new ReferenceQueue<>();
private static class ApkKey {
public final String path;
@@ -155,6 +153,7 @@
}
public final Configuration overrideConfig = new Configuration();
public final ArrayList<WeakReference<Resources>> activityResources = new ArrayList<>();
+ final ReferenceQueue<Resources> activityResourcesQueue = new ReferenceQueue<>();
}
/**
@@ -667,12 +666,15 @@
@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 @@
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 @@
updateResourcesForActivity(token, overrideConfig, displayId,
false /* movedToDifferentDisplay */);
- cleanupReferences(token);
rebaseKeyForActivity(token, key);
synchronized (this) {
@@ -778,10 +781,6 @@
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 @@
/**
* 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 <T> void cleanupReferences(ArrayList<WeakReference<T>> references,
+ ReferenceQueue<T> referenceQueue) {
+ Reference<? extends T> enduedRef = referenceQueue.poll();
+ if (enduedRef == null) {
+ return;
}
+
+ final HashSet<Reference<? extends T>> 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 @@
loaders == null ? null : loaders.toArray(new ResourcesLoader[0]));
classLoader = classLoader != null ? classLoader : ClassLoader.getSystemClassLoader();
- cleanupReferences(activityToken);
-
if (activityToken != null) {
rebaseKeyForActivity(activityToken, key);
}