Merge "Improve performance of generations in the resource cache" into udc-dev am: a0bbc28920 am: 3fc9a70e90

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/23644746

Change-Id: Ide4e887c63063dc56f94b829d36afeab11b3e505
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/java/android/animation/AnimatorInflater.java

@@ -111,20 +111,20 @@ public class AnimatorInflater {
             float pathErrorScale) throws NotFoundException {
         final ConfigurationBoundResourceCache<Animator> animatorCache = resources
                 .getAnimatorCache();
-        ConfigurationBoundResourceCache.Entry<Animator> animatorEntry =
+        Animator animator = animatorCache.getInstance(id, resources, theme);
-                animatorCache.getInstance(id, resources, theme);
+        if (animator != null) {
-        if (animatorEntry.hasValue()) {
             if (DBG_ANIMATOR_INFLATER) {
                 Log.d(TAG, "loaded animator from cache, " + resources.getResourceName(id));
             }
-            return animatorEntry.getValue();
+            return animator;
         } else if (DBG_ANIMATOR_INFLATER) {
             Log.d(TAG, "cache miss for animator " + resources.getResourceName(id));
         }
+        int cacheGeneration = animatorCache.getGeneration();
         XmlResourceParser parser = null;
         try {
             parser = resources.getAnimation(id);
-            Animator animator = createAnimatorFromXml(resources, theme, parser, pathErrorScale);
+            animator = createAnimatorFromXml(resources, theme, parser, pathErrorScale);
             if (animator != null) {
                 animator.appendChangingConfigurations(getChangingConfigs(resources, id));
                 final ConstantState<Animator> constantState = animator.createConstantState();
@@ -132,7 +132,7 @@ public class AnimatorInflater {
                     if (DBG_ANIMATOR_INFLATER) {
                         Log.d(TAG, "caching animator for res " + resources.getResourceName(id));
                     }
-                    animatorCache.put(id, theme, constantState, animatorEntry.getGeneration());
+                    animatorCache.put(id, theme, constantState, cacheGeneration);
                     // create a new animator so that cached version is never used by the user
                     animator = constantState.newInstance(resources, theme);
                 }
@@ -161,22 +161,22 @@ public class AnimatorInflater {
         final ConfigurationBoundResourceCache<StateListAnimator> cache = resources
                 .getStateListAnimatorCache();
         final Theme theme = context.getTheme();
-        ConfigurationBoundResourceCache.Entry<StateListAnimator> animatorEntry =
+        StateListAnimator animator = cache.getInstance(id, resources, theme);
-                cache.getInstance(id, resources, theme);
+        if (animator != null) {
-        if (animatorEntry.hasValue()) {
+            return animator;
-            return animatorEntry.getValue();
         }
+        int cacheGeneration = cache.getGeneration();
         XmlResourceParser parser = null;
         try {
             parser = resources.getAnimation(id);
-            StateListAnimator animator =
+            animator =
                     createStateListAnimatorFromXml(context, parser, Xml.asAttributeSet(parser));
             if (animator != null) {
                 animator.appendChangingConfigurations(getChangingConfigs(resources, id));
                 final ConstantState<StateListAnimator> constantState = animator
                         .createConstantState();
                 if (constantState != null) {
-                    cache.put(id, theme, constantState, animatorEntry.getGeneration());
+                    cache.put(id, theme, constantState, cacheGeneration);
                     // return a clone so that the animator in constant state is never used.
                     animator = constantState.newInstance(resources, theme);
                 }

core/java/android/content/res/ConfigurationBoundResourceCache.java

@@ -37,16 +37,16 @@ public class ConfigurationBoundResourceCache<T> extends ThemedResourceCache<Cons
      * @param key a key that uniquely identifies the drawable resource
      * @param resources a Resources object from which to create new instances.
      * @param theme the theme where the resource will be used
-     * @return an Entry wrapping a new instance of the resource, or {@code null} if not in
+     * @return a new instance of the resource, or {@code null} if not in
      *         the cache
      */
-    public Entry<T> getInstance(long key, Resources resources, Resources.Theme theme) {
+    public T getInstance(long key, Resources resources, Resources.Theme theme) {
-        final Entry<ConstantState<T>> e = get(key, theme);
+        final ConstantState<T> entry = get(key, theme);
-        if (e.hasValue()) {
+        if (entry != null) {
-            return new Entry<>(e.getValue().newInstance(resources, theme), e.getGeneration());
+            return entry.newInstance(resources, theme);
         }
 
-        return new Entry<>(null, e.getGeneration());
+        return null;
     }
 
     @Override

core/java/android/content/res/DrawableCache.java

@@ -40,32 +40,14 @@ class DrawableCache extends ThemedResourceCache<Drawable.ConstantState> {
      */
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
     public Drawable getInstance(long key, Resources resources, Resources.Theme theme) {
-        final Entry<Drawable.ConstantState> entry = get(key, theme);
+        final Drawable.ConstantState entry = get(key, theme);
-        if (entry.getValue() != null) {
+        if (entry != null) {
-            return entry.getValue().newDrawable(resources, theme);
+            return entry.newDrawable(resources, theme);
         }
 
         return null;
     }
 
-    /**
-     * If the resource is cached, creates and returns a new instance of it.
-     *
-     * @param key a key that uniquely identifies the drawable resource
-     * @param resources a Resources object from which to create new instances.
-     * @param theme the theme where the resource will be used
-     * @return an Entry wrapping a a new instance of the resource, or {@code null} if not in
-     *         the cache
-     */
-    public Entry<Drawable> getDrawable(long key, Resources resources, Resources.Theme theme) {
-        final Entry<Drawable.ConstantState> e = get(key, theme);
-        if (e.hasValue()) {
-            return new Entry<>(e.getValue().newDrawable(resources, theme), e.getGeneration());
-        }
-
-        return new Entry<>(null, e.getGeneration());
-    }
-
     @Override
     public boolean shouldInvalidateEntry(Drawable.ConstantState entry, int configChanges) {
         return Configuration.needNewResources(configChanges, entry.getChangingConfigurations());

core/java/android/content/res/ResourcesImpl.java

@@ -650,21 +650,16 @@ public class ResourcesImpl {
                 key = (((long) value.assetCookie) << 32) | value.data;
             }
 
-            int cacheGeneration;
+            int cacheGeneration = caches.getGeneration();
             // First, check whether we have a cached version of this drawable
             // that was inflated against the specified theme. Skip the cache if
             // we're currently preloading or we're not using the cache.
             if (!mPreloading && useCache) {
-                final ThemedResourceCache.Entry<Drawable> cachedDrawable =
+                Drawable cachedDrawable = caches.getInstance(key, wrapper, theme);
-                        caches.getDrawable(key, wrapper, theme);
+                if (cachedDrawable != null) {
-                if (cachedDrawable.hasValue()) {
+                    cachedDrawable.setChangingConfigurations(value.changingConfigurations);
-                    cachedDrawable.getValue().setChangingConfigurations(
+                    return cachedDrawable;
-                            value.changingConfigurations);
-                    return cachedDrawable.getValue();
                 }
-                cacheGeneration = cachedDrawable.getGeneration();
-            } else {
-                cacheGeneration = ThemedResourceCache.UNDEFINED_GENERATION;
             }
 
             // Next, check preloaded drawables. Preloaded drawables may contain
@@ -1009,16 +1004,15 @@ public class ResourcesImpl {
             TypedValue value, int id) {
         final long key = (((long) value.assetCookie) << 32) | value.data;
         final ConfigurationBoundResourceCache<ComplexColor> cache = mComplexColorCache;
-        ThemedResourceCache.Entry<ComplexColor> complexColorEntry =
+        ComplexColor complexColor = cache.getInstance(key, wrapper, theme);
-                cache.getInstance(key, wrapper, theme);
+        if (complexColor != null) {
-        if (complexColorEntry.hasValue()) {
+            return complexColor;
-            return complexColorEntry.getValue();
         }
+        int cacheGeneration = cache.getGeneration();
 
         final android.content.res.ConstantState<ComplexColor> factory =
                 sPreloadedComplexColors.get(key);
 
-        ComplexColor complexColor = null;
         if (factory != null) {
             complexColor = factory.newInstance(wrapper, theme);
         }
@@ -1035,8 +1029,7 @@ public class ResourcesImpl {
                     sPreloadedComplexColors.put(key, complexColor.getConstantState());
                 }
             } else {
-                cache.put(key, theme, complexColor.getConstantState(),
+                cache.put(key, theme, complexColor.getConstantState(), cacheGeneration);
-                        complexColorEntry.getGeneration());
             }
         }
         return complexColor;

core/java/android/content/res/ThemedResourceCache.java

@@ -41,29 +41,6 @@ abstract class ThemedResourceCache<T> {
 
     private int mGeneration;
 
-    public static class Entry<S> {
-        private S mValue;
-        private int mGeneration;
-
-
-        public S getValue() {
-            return mValue;
-        }
-
-        public boolean hasValue() {
-            return mValue != null;
-        }
-
-        public int getGeneration() {
-            return mGeneration;
-        }
-
-        Entry(S value, int generation) {
-            this.mValue = value;
-            this.mGeneration = generation;
-        }
-    }
-
     /**
      * Adds a new theme-dependent entry to the cache.
      *
@@ -108,6 +85,15 @@ abstract class ThemedResourceCache<T> {
         }
     }
 
+    /**
+     * Returns the current generation of the cache
+     *
+     * @return The current generation
+     */
+    public int getGeneration() {
+        return mGeneration;
+    }
+
     /**
      * Returns an entry from the cache.
      *
@@ -116,7 +102,7 @@ abstract class ThemedResourceCache<T> {
      * @return a cached entry, or {@code null} if not in the cache
      */
     @Nullable
-    public Entry get(long key, @Nullable Theme theme) {
+    public T get(long key, @Nullable Theme theme) {
         // The themed (includes null-themed) and unthemed caches are mutually
         // exclusive, so we'll give priority to whichever one we think we'll
         // hit first. Since most of the framework drawables are themed, that's
@@ -126,7 +112,7 @@ abstract class ThemedResourceCache<T> {
             if (themedEntries != null) {
                 final WeakReference<T> themedEntry = themedEntries.get(key);
                 if (themedEntry != null) {
-                    return new Entry(themedEntry.get(), mGeneration);
+                    return themedEntry.get();
                 }
             }
 
@@ -134,14 +120,15 @@ abstract class ThemedResourceCache<T> {
             if (unthemedEntries != null) {
                 final WeakReference<T> unthemedEntry = unthemedEntries.get(key);
                 if (unthemedEntry != null) {
-                    return new Entry(unthemedEntry.get(), mGeneration);
+                    return unthemedEntry.get();
                 }
             }
         }
 
-        return new Entry(null, mGeneration);
+        return null;
     }
 
+
     /**
      * Prunes cache entries that have been invalidated by a configuration
      * change.

core/tests/coretests/src/android/content/res/ConfigurationBoundResourceCacheTest.java

@@ -45,7 +45,7 @@ public class ConfigurationBoundResourceCacheTest
     @SmallTest
     public void testGetEmpty() {
         final Resources res = getActivity().getResources();
-        assertNull(mCache.getInstance(-1, res, null).getValue());
+        assertNull(mCache.getInstance(-1, res, null));
     }
 
     @SmallTest
@@ -53,9 +53,9 @@ public class ConfigurationBoundResourceCacheTest
         mCache.put(1, null, new DummyFloatConstantState(5f),
                 ThemedResourceCache.UNDEFINED_GENERATION);
         final Resources res = getActivity().getResources();
-        assertEquals(5f, mCache.getInstance(1, res, null).getValue());
+        assertEquals(5f, mCache.getInstance(1, res, null));
-        assertNotSame(5f, mCache.getInstance(1, res, null).getValue());
+        assertNotSame(5f, mCache.getInstance(1, res, null));
-        assertEquals(false, mCache.getInstance(1, res, getActivity().getTheme()).hasValue());
+        assertEquals(null, mCache.getInstance(1, res, getActivity().getTheme()));
     }
 
     @SmallTest
@@ -63,9 +63,9 @@ public class ConfigurationBoundResourceCacheTest
         mCache.put(1, getActivity().getTheme(), new DummyFloatConstantState(5f),
                 ThemedResourceCache.UNDEFINED_GENERATION);
         final Resources res = getActivity().getResources();
-        assertEquals(false, mCache.getInstance(1, res, null).hasValue());
+        assertEquals(null, mCache.getInstance(1, res, null));
-        assertEquals(5f, mCache.getInstance(1, res, getActivity().getTheme()).getValue());
+        assertEquals(5f, mCache.getInstance(1, res, getActivity().getTheme()));
-        assertNotSame(5f, mCache.getInstance(1, res, getActivity().getTheme()).getValue());
+        assertNotSame(5f, mCache.getInstance(1, res, getActivity().getTheme()));
     }
 
     @SmallTest
@@ -75,10 +75,10 @@ public class ConfigurationBoundResourceCacheTest
         mCache.put(1, null, new DummyFloatConstantState(10f),
                 ThemedResourceCache.UNDEFINED_GENERATION);
         final Resources res = getActivity().getResources();
-        assertEquals(10f, mCache.getInstance(1, res, null).getValue());
+        assertEquals(10f, mCache.getInstance(1, res, null));
-        assertNotSame(10f, mCache.getInstance(1, res, null).getValue());
+        assertNotSame(10f, mCache.getInstance(1, res, null));
-        assertEquals(5f, mCache.getInstance(1, res, getActivity().getTheme()).getValue());
+        assertEquals(5f, mCache.getInstance(1, res, getActivity().getTheme()));
-        assertNotSame(5f, mCache.getInstance(1, res, getActivity().getTheme()).getValue());
+        assertNotSame(5f, mCache.getInstance(1, res, getActivity().getTheme()));
     }
 
     @SmallTest
@@ -98,9 +98,9 @@ public class ConfigurationBoundResourceCacheTest
                 Configuration.ORIENTATION_PORTRAIT
                 : Configuration.ORIENTATION_LANDSCAPE;
         int changes = calcConfigChanges(res, newCnf);
-        assertEquals(staticDim, mCache.getInstance(key, res, getActivity().getTheme()).getValue());
+        assertEquals(staticDim, mCache.getInstance(key, res, getActivity().getTheme()));
         mCache.onConfigurationChange(changes);
-        assertEquals(staticDim, mCache.getInstance(key, res, getActivity().getTheme()).getValue());
+        assertEquals(staticDim, mCache.getInstance(key, res, getActivity().getTheme()));
     }
 
     @SmallTest
@@ -123,9 +123,9 @@ public class ConfigurationBoundResourceCacheTest
                 : Configuration.ORIENTATION_LANDSCAPE;
         int changes = calcConfigChanges(res, newCnf);
         assertEquals(changingDim,
-                mCache.getInstance(key, res, getActivity().getTheme()).getValue());
+                mCache.getInstance(key, res, getActivity().getTheme()));
         mCache.onConfigurationChange(changes);
-        assertNull(mCache.get(key, getActivity().getTheme()).getValue());
+        assertNull(mCache.get(key, getActivity().getTheme()));
     }
 
     @SmallTest
@@ -152,15 +152,15 @@ public class ConfigurationBoundResourceCacheTest
                 : Configuration.ORIENTATION_LANDSCAPE;
         int changes = calcConfigChanges(res, newCnf);
         assertEquals(staticDim, mCache.getInstance(R.dimen.resource_cache_test_generic, res,
-                getActivity().getTheme()).getValue());
+                getActivity().getTheme()));
         assertEquals(changingDim,
                 mCache.getInstance(R.dimen.resource_cache_test_orientation_dependent, res,
-                        getActivity().getTheme()).getValue());
+                        getActivity().getTheme()));
         mCache.onConfigurationChange(changes);
         assertEquals(staticDim, mCache.getInstance(R.dimen.resource_cache_test_generic, res,
-                getActivity().getTheme()).getValue());
+                getActivity().getTheme()));
         assertNull(mCache.getInstance(R.dimen.resource_cache_test_orientation_dependent, res,
-                getActivity().getTheme()).getValue());
+                getActivity().getTheme()));
     }
 
     @SmallTest
@@ -198,18 +198,18 @@ public class ConfigurationBoundResourceCacheTest
         for (int i = 0; i < 2; i++) {
             final Resources.Theme theme = i == 0 ? getActivity().getTheme() : null;
             assertEquals(staticDim,
-                    mCache.getInstance(R.dimen.resource_cache_test_generic, res, theme).getValue());
+                    mCache.getInstance(R.dimen.resource_cache_test_generic, res, theme));
             assertEquals(changingDim,
                     mCache.getInstance(R.dimen.resource_cache_test_orientation_dependent, res,
-                            theme).getValue());
+                            theme));
         }
         mCache.onConfigurationChange(changes);
         for (int i = 0; i < 2; i++) {
             final Resources.Theme theme = i == 0 ? getActivity().getTheme() : null;
             assertEquals(staticDim,
-                    mCache.getInstance(R.dimen.resource_cache_test_generic, res, theme).getValue());
+                    mCache.getInstance(R.dimen.resource_cache_test_generic, res, theme));
             assertNull(mCache.getInstance(R.dimen.resource_cache_test_orientation_dependent, res,
-                    theme).getValue());
+                    theme));
         }
     }