From a68dca3c469530c2d9a6dc54cad96dd9aa299ef8 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 31 May 2022 14:34:33 -0700 Subject: [PATCH] Clean up View Recycling Summary: Followups to View Recycling diffs to improve things / clean up things a bit. This also fixes memory warnings which were not hooked up before. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D36707792 fbshipit-source-id: 410e70bf0eeec5569566138af547e1601394d0e6 --- .../react/fabric/FabricUIManager.java | 9 +++ .../react/uimanager/BaseViewManager.java | 30 +++++++++ .../react/uimanager/UIManagerModule.java | 2 + .../facebook/react/uimanager/ViewManager.java | 28 +++++--- .../react/uimanager/ViewManagerRegistry.java | 66 +++++++++---------- .../react/views/text/ReactTextView.java | 26 ++------ .../views/text/ReactTextViewManager.java | 12 +--- .../react/views/view/ReactViewGroup.java | 31 --------- .../react/views/view/ReactViewManager.java | 4 +- 9 files changed, 101 insertions(+), 107 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 642dd331dba..d4240227263 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -166,6 +166,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @NonNull private final MountingManager mMountingManager; @NonNull private final EventDispatcher mEventDispatcher; @NonNull private final MountItemDispatcher mMountItemDispatcher; + @NonNull private final ViewManagerRegistry mViewManagerRegistry; @NonNull private final EventBeatManager mEventBeatManager; @@ -226,6 +227,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mShouldDeallocateEventDispatcher = false; mEventBeatManager = eventBeatManager; mReactApplicationContext.addLifecycleEventListener(this); + + mViewManagerRegistry = viewManagerRegistry; + mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry); } public FabricUIManager( @@ -244,6 +248,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mShouldDeallocateEventDispatcher = true; mEventBeatManager = eventBeatManager; mReactApplicationContext.addLifecycleEventListener(this); + + mViewManagerRegistry = viewManagerRegistry; + mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry); } // TODO (T47819352): Rename this to startSurface for consistency with xplat/iOS @@ -451,6 +458,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager); mEventDispatcher.unregisterEventEmitter(FABRIC); + mReactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry); + // Remove lifecycle listeners (onHostResume, onHostPause) since the FabricUIManager is going // away. Then stop the mDispatchUIFrameCallback false will cause the choreographer // callbacks to stop firing. diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java index 8df93b57578..96437946c55 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java @@ -105,6 +105,36 @@ public abstract class BaseViewManager * null signals that View Recycling is disabled. `enableViewRecycling` must be explicitly called * in a concrete constructor to enable View Recycling per ViewManager. */ - private HashMap> mRecyclableViews = null; + @Nullable private HashMap> mRecyclableViews = null; + + private int mRecyclableViewsBufferSize = 1024; /** Call in constructor of concrete ViewManager class to enable. */ - protected void enableViewRecycling() { + protected void setupViewRecycling() { if (ReactFeatureFlags.enableViewRecycling) { mRecyclableViews = new HashMap<>(); } } + /** Call in constructor of concrete ViewManager class to enable. */ + protected void setupViewRecycling(int bufferSize) { + mRecyclableViewsBufferSize = bufferSize; + setupViewRecycling(); + } + private @Nullable Stack getRecyclableViewStack(int surfaceId) { if (mRecyclableViews == null) { return null; @@ -191,12 +199,16 @@ public abstract class ViewManager * {@link ViewManager} subclass. */ public void onDropViewInstance(@NonNull T view) { - @Nullable - Stack recyclableViews = - getRecyclableViewStack(((ThemedReactContext) view.getContext()).getSurfaceId()); - // By default we treat views as recyclable - if (recyclableViews != null) { - recyclableViews.push(prepareToRecycleView((ThemedReactContext) view.getContext(), view)); + // View recycling + ThemedReactContext themedReactContext = (ThemedReactContext) view.getContext(); + int surfaceId = themedReactContext.getSurfaceId(); + @Nullable Stack recyclableViews = getRecyclableViewStack(surfaceId); + + // Any max buffer size <0 results in an infinite buffer size + if (recyclableViews != null + && (mRecyclableViewsBufferSize < 0 + || recyclableViews.size() < mRecyclableViewsBufferSize)) { + recyclableViews.push(prepareToRecycleView(themedReactContext, view)); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java index 2fe412b4e43..ec107e83b66 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java @@ -19,11 +19,10 @@ import java.util.Map; * Class that stores the mapping between native view name used in JS and the corresponding instance * of {@link ViewManager}. */ -public final class ViewManagerRegistry { +public final class ViewManagerRegistry implements ComponentCallbacks2 { private final Map mViewManagers; private final @Nullable ViewManagerResolver mViewManagerResolver; - private final MemoryTrimCallback mMemoryTrimCallback = new MemoryTrimCallback(); public ViewManagerRegistry(ViewManagerResolver viewManagerResolver) { mViewManagers = MapBuilder.newHashMap(); @@ -46,40 +45,6 @@ public final class ViewManagerRegistry { mViewManagerResolver = null; } - /** - * Trim View Recycling memory aggressively. Whenever the system is running even slightly low on - * memory or is backgrounded, we immediately flush all recyclable Views. GC and memory swaps cause - * intense CPU pressure, so we always favor low memory usage over View recycling, even if there is - * only "moderate" pressure. - */ - private class MemoryTrimCallback implements ComponentCallbacks2 { - @Override - public void onTrimMemory(int level) { - Runnable runnable = - new Runnable() { - @Override - public void run() { - for (Map.Entry entry : mViewManagers.entrySet()) { - entry.getValue().trimMemory(); - } - } - }; - if (UiThreadUtil.isOnUiThread()) { - runnable.run(); - } else { - UiThreadUtil.runOnUiThread(runnable); - } - } - - @Override - public void onConfigurationChanged(Configuration newConfig) {} - - @Override - public void onLowMemory() { - this.onTrimMemory(0); - } - } - /** * @param className {@link String} that identifies the {@link ViewManager} inside the {@link * ViewManagerRegistry}. This methods {@throws IllegalViewOperationException} if there is no @@ -147,4 +112,33 @@ public final class ViewManagerRegistry { UiThreadUtil.runOnUiThread(runnable); } } + + /** ComponentCallbacks2 method. */ + @Override + public void onTrimMemory(int level) { + Runnable runnable = + new Runnable() { + @Override + public void run() { + for (Map.Entry entry : mViewManagers.entrySet()) { + entry.getValue().trimMemory(); + } + } + }; + if (UiThreadUtil.isOnUiThread()) { + runnable.run(); + } else { + UiThreadUtil.runOnUiThread(runnable); + } + } + + /** ComponentCallbacks2 method. */ + @Override + public void onConfigurationChanged(Configuration newConfig) {} + + /** ComponentCallbacks2 method. */ + @Override + public void onLowMemory() { + this.onTrimMemory(0); + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index 6920301046f..fcc81f8db88 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -96,16 +96,18 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie mSpanned = null; } - /* package */ void recycleView(ReactTextView defaultView) { + /* package */ void recycleView() { // Set default field values initView(); - setForeground(null); + // Defaults for these fields: + // https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/widget/TextView.java#L1061 + setBreakStrategy(Layout.BREAK_STRATEGY_SIMPLE); + setMovementMethod(getDefaultMovementMethod()); + setJustificationMode(Layout.JUSTIFICATION_MODE_NONE); // reset text setLayoutParams(EMPTY_LAYOUT_PARAMS); - setMovementMethod(defaultView.getMovementMethod()); - setBreakStrategy(defaultView.getBreakStrategy()); super.setText(null); // Call setters to ensure that any super setters are called @@ -124,21 +126,8 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie // reset data detectors setLinkifyMask(0); - setJustificationMode(defaultView.getJustificationMode()); - setEllipsizeLocation(mEllipsizeLocation); - // Focus IDs - // Also see in AOSP source: - // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493 - setNextFocusDownId(View.NO_ID); - setNextFocusForwardId(View.NO_ID); - setNextFocusRightId(View.NO_ID); - setNextFocusUpId(View.NO_ID); - - // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491 - setElevation(0); - // View flags - defaults are here: // https://android.googlesource.com/platform/frameworks/base/+/98e54bb941cb6feb07127b75da37833281951d52/core/java/android/view/View.java#5311 // mViewFlags = SOUND_EFFECTS_ENABLED | HAPTIC_FEEDBACK_ENABLED | @@ -146,8 +135,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie setEnabled(true); setFocusable(View.FOCUSABLE_AUTO); - // Things that could be set as a result of updateText/setText - setPadding(0, 0, 0, 0); + setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NONE); updateView(); // call after changing ellipsizeLocation in particular } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java index 29edd7a68d4..de60d769c09 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java @@ -45,33 +45,25 @@ public class ReactTextViewManager @VisibleForTesting public static final String REACT_CLASS = "RCTText"; - private ReactTextView mDefaultViewForRecycling = null; - protected @Nullable ReactTextViewManagerCallback mReactTextViewManagerCallback; public ReactTextViewManager() { super(); - enableViewRecycling(); + setupViewRecycling(); } @Override protected ReactTextView prepareToRecycleView( @NonNull ThemedReactContext reactContext, ReactTextView view) { - // TODO: use context as key - if (mDefaultViewForRecycling == null) { - mDefaultViewForRecycling = createViewInstance(reactContext); - } - // BaseViewManager super.prepareToRecycleView(reactContext, view); // Resets background and borders - view.recycleView(mDefaultViewForRecycling); + view.recycleView(); // Defaults from ReactTextAnchorViewManager setSelectionColor(view, null); - setAndroidHyphenationFrequency(view, null); return view; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index e0ca386f36f..93afe79dc7f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -176,32 +176,6 @@ public class ReactViewGroup extends ViewGroup // Reset background, borders updateBackgroundDrawable(null); - setForeground(null); - - // This is possibly subject to change and overrideable per-platform, but these - // are the default view flags in View.java: - // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2712 - // `mViewFlags = SOUND_EFFECTS_ENABLED | HAPTIC_FEEDBACK_ENABLED | LAYOUT_DIRECTION_INHERIT` - // Therefore we set the following options as such: - setFocusable(false); - setFocusableInTouchMode(false); - - // Focus IDs - // Also see in AOSP source: - // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493 - setNextFocusDownId(View.NO_ID); - setNextFocusForwardId(View.NO_ID); - setNextFocusRightId(View.NO_ID); - setNextFocusUpId(View.NO_ID); - - // Predictable, alpha defaults to 1: - // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2186 - // This accounts for resetting mBackfaceOpacity and mBackfaceVisibility - setAlpha(1); - - // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491 - setElevation(0); - resetPointerEvents(); } @@ -813,11 +787,6 @@ public class ReactViewGroup extends ViewGroup mOverflowInset.set(left, top, right, bottom); } - public void setOverflowInset(Rect overflowInset) { - mOverflowInset.set( - overflowInset.left, overflowInset.top, overflowInset.right, overflowInset.bottom); - } - @Override public Rect getOverflowInset() { return mOverflowInset; diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java index 59098e792bc..7765a3a828c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java @@ -52,12 +52,10 @@ public class ReactViewManager extends ReactClippingViewManager { private static final int CMD_SET_PRESSED = 2; private static final String HOTSPOT_UPDATE_KEY = "hotspotUpdate"; - private ReactViewGroup mDefaultViewForRecycling = null; - public ReactViewManager() { super(); - enableViewRecycling(); + setupViewRecycling(); } @Override