From ab3cb30aa0ea166c8bcb0531a5a47ca119a000c3 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Tue, 11 Oct 2022 12:15:52 -0700 Subject: [PATCH 1/2] Ensure no duplicates in ThemeKey Some apps keep adding the same resources to their theme key objects on e.g. switching dark mode. Need to be prepared to limit the size of the arrays, otherwise theme switching gets slower with each toggle + a bit more efficient native handling Bug: 242005877 Test: manual, 10k of theme switches with no noticable slowdown Change-Id: Icf74770bd41ebeb0a31f527ae3616de00f23b0ae --- core/java/android/content/res/Resources.java | 40 +++++++++++++++----- libs/androidfw/AssetManager2.cpp | 7 ++-- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/core/java/android/content/res/Resources.java b/core/java/android/content/res/Resources.java index a03286d3ec6f6..fb1fcf8e2a063 100644 --- a/core/java/android/content/res/Resources.java +++ b/core/java/android/content/res/Resources.java @@ -173,6 +173,7 @@ public class Resources { * mThemeRefNextFlushSize is reached. */ private static final int MIN_THEME_REFS_FLUSH_SIZE = 32; + private static final int MAX_THEME_REFS_FLUSH_SIZE = 512; private int mThemeRefsNextFlushSize = MIN_THEME_REFS_FLUSH_SIZE; private int mBaseApkAssetsSize; @@ -364,10 +365,10 @@ public class Resources { // Rebase the ThemeImpls using the new ResourcesImpl. synchronized (mThemeRefs) { + cleanupThemeReferences(); final int count = mThemeRefs.size(); for (int i = 0; i < count; i++) { - WeakReference weakThemeRef = mThemeRefs.get(i); - Theme theme = weakThemeRef != null ? weakThemeRef.get() : null; + Theme theme = mThemeRefs.get(i).get(); if (theme != null) { theme.rebase(mResourcesImpl); } @@ -2001,6 +2002,15 @@ public class Resources { private int mHashCode = 0; + private boolean containsValue(int resId, boolean force) { + for (int i = 0; i < mCount; ++i) { + if (mResId[i] == resId && mForce[i] == force) { + return true; + } + } + return false; + } + public void append(int resId, boolean force) { if (mResId == null) { mResId = new int[4]; @@ -2010,6 +2020,11 @@ public class Resources { mForce = new boolean[4]; } + // Some apps tend to keep adding same resources over and over, let's protect from it. + if (containsValue(resId, force)) { + return; + } + mResId = GrowingArrayUtils.append(mResId, mCount, resId); mForce = GrowingArrayUtils.append(mForce, mCount, force); mCount++; @@ -2073,6 +2088,19 @@ public class Resources { } } + static int nextPowerOf2(int number) { + return number < 2 ? 2 : 1 >> ((int) (Math.log(number - 1) / Math.log(2)) + 1); + } + + private void cleanupThemeReferences() { + // Clean up references to garbage collected themes + if (mThemeRefs.size() > mThemeRefsNextFlushSize) { + mThemeRefs.removeIf(ref -> ref.refersTo(null)); + mThemeRefsNextFlushSize = Math.min(Math.max(MIN_THEME_REFS_FLUSH_SIZE, + nextPowerOf2(mThemeRefs.size())), MAX_THEME_REFS_FLUSH_SIZE); + } + } + /** * Generate a new Theme object for this set of Resources. It initially * starts out empty. @@ -2083,14 +2111,8 @@ public class Resources { Theme theme = new Theme(); theme.setImpl(mResourcesImpl.newThemeImpl()); synchronized (mThemeRefs) { + cleanupThemeReferences(); mThemeRefs.add(new WeakReference<>(theme)); - - // Clean up references to garbage collected themes - if (mThemeRefs.size() > mThemeRefsNextFlushSize) { - mThemeRefs.removeIf(ref -> ref.refersTo(null)); - mThemeRefsNextFlushSize = Math.max(MIN_THEME_REFS_FLUSH_SIZE, - 2 * mThemeRefs.size()); - } } return theme; } diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 235700b27c250..1381bdd6a50d9 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -1068,7 +1068,7 @@ base::expected AssetManager2::ResolveBag( base::expected AssetManager2::GetBag(uint32_t resid) const { std::vector found_resids; const auto bag = GetBag(resid, found_resids); - cached_bag_resid_stacks_.emplace(resid, found_resids); + cached_bag_resid_stacks_.emplace(resid, std::move(found_resids)); return bag; } @@ -1468,7 +1468,6 @@ base::expected Theme::ApplyStyle(uint32_t resid, continue; } - Theme::Entry new_entry{attr_res_id, it->cookie, (*bag)->type_spec_flags, it->value}; auto entry_it = std::lower_bound(entries_.begin(), entries_.end(), attr_res_id, ThemeEntryKeyComparer{}); if (entry_it != entries_.end() && entry_it->attr_res_id == attr_res_id) { @@ -1477,10 +1476,10 @@ base::expected Theme::ApplyStyle(uint32_t resid, /// true. entries_.erase(entry_it); } else if (force) { - *entry_it = new_entry; + *entry_it = Entry{attr_res_id, it->cookie, (*bag)->type_spec_flags, it->value}; } } else { - entries_.insert(entry_it, new_entry); + entries_.insert(entry_it, Entry{attr_res_id, it->cookie, (*bag)->type_spec_flags, it->value}); } } return {}; From 0776b3b410c86dd8f739097d3c53bc3ec7d5679a Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Tue, 11 Oct 2022 15:47:02 -0700 Subject: [PATCH 2/2] [sysui] Use weak reference for the listener in NavigationBarTransitions The listener is the only thing keeping objects alive after switching dark theme, bringing the number of Resources.Theme objects to thousands. Bug: 242005877 Test: manual, 10k theme switches to verify there's fewer active theme objects Change-Id: I59ff27dacd4bcad933341886c249074cf7df6dc9 --- .../NavigationBarTransitions.java | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarTransitions.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarTransitions.java index 6793f0163c43d..43cf6231aaf05 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarTransitions.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarTransitions.java @@ -36,6 +36,7 @@ import com.android.systemui.statusbar.phone.BarTransitions; import com.android.systemui.statusbar.phone.LightBarTransitionsController; import java.io.PrintWriter; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.List; @@ -75,15 +76,27 @@ public final class NavigationBarTransitions extends BarTransitions implements private List mDarkIntensityListeners; private final Handler mHandler = Handler.getMain(); - private final IWallpaperVisibilityListener mWallpaperVisibilityListener = - new IWallpaperVisibilityListener.Stub() { + + static final class WallpaperVisibilityListener extends IWallpaperVisibilityListener.Stub { + private final WeakReference mSelf; + + WallpaperVisibilityListener(NavigationBarTransitions self) { + mSelf = new WeakReference<>(self); + } + @Override public void onWallpaperVisibilityChanged(boolean newVisibility, - int displayId) throws RemoteException { - mWallpaperVisible = newVisibility; - mHandler.post(() -> applyLightsOut(true, false)); + int displayId) throws RemoteException { + NavigationBarTransitions self = mSelf.get(); + if (self == null) { + return; + } + self.mWallpaperVisible = newVisibility; + self.mHandler.post(() -> self.applyLightsOut(true, false)); } - }; + } + + private final IWallpaperVisibilityListener mWallpaperVisibilityListener; @Inject public NavigationBarTransitions( @@ -91,6 +104,7 @@ public final class NavigationBarTransitions extends BarTransitions implements IWindowManager windowManagerService, LightBarTransitionsController.Factory lightBarTransitionsControllerFactory) { super(view, R.drawable.nav_background); + mView = view; mWindowManagerService = windowManagerService; mLightTransitionsController = lightBarTransitionsControllerFactory.create(this); @@ -98,6 +112,7 @@ public final class NavigationBarTransitions extends BarTransitions implements .getBoolean(R.bool.config_navigation_bar_enable_auto_dim_no_visible_wallpaper); mDarkIntensityListeners = new ArrayList(); + mWallpaperVisibilityListener = new WallpaperVisibilityListener(this); try { mWallpaperVisible = mWindowManagerService.registerWallpaperVisibilityListener( mWallpaperVisibilityListener, Display.DEFAULT_DISPLAY);