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 642dd331dbaae0..d42402272631c1 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 void onFabricCommitEnd(DevToolsReactPerfLogger.FabricCommitPoint commitPo @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 FabricUIManager( mShouldDeallocateEventDispatcher = false; mEventBeatManager = eventBeatManager; mReactApplicationContext.addLifecycleEventListener(this); + + mViewManagerRegistry = viewManagerRegistry; + mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry); } public FabricUIManager( @@ -244,6 +248,9 @@ public FabricUIManager( 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 void onCatalystInstanceDestroy() { 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 8df93b57578476..96437946c552a7 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 @@ protected T prepareToRecycleView(@NonNull ThemedReactContext reactContext, T vie view.setOutlineAmbientShadowColor(Color.BLACK); view.setOutlineSpotShadowColor(Color.BLACK); + // Focus IDs + // Also see in AOSP source: + // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493 + view.setNextFocusDownId(View.NO_ID); + view.setNextFocusForwardId(View.NO_ID); + view.setNextFocusRightId(View.NO_ID); + view.setNextFocusUpId(View.NO_ID); + + // 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: + view.setFocusable(false); + view.setFocusableInTouchMode(false); + + // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491 + view.setElevation(0); + + // Predictably, 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 + view.setAlpha(1); + + // setPadding is a noop for most View types, but it is not for Text + setPadding(view, 0, 0, 0, 0); + + // Other stuff + view.setForeground(null); + return view; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 57f383b81a6c5a..2c21f9064ed509 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -207,6 +207,7 @@ public Map getConstants() { @Override public void initialize() { getReactApplicationContext().registerComponentCallbacks(mMemoryTrimCallback); + getReactApplicationContext().registerComponentCallbacks(mViewManagerRegistry); mEventDispatcher.registerEventEmitter( DEFAULT, getReactApplicationContext().getJSModule(RCTEventEmitter.class)); } @@ -234,6 +235,7 @@ public void onCatalystInstanceDestroy() { ReactApplicationContext reactApplicationContext = getReactApplicationContext(); reactApplicationContext.unregisterComponentCallbacks(mMemoryTrimCallback); + reactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry); YogaNodePool.get().clear(); ViewManagerPropertyUpdater.clear(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java index d0cae313b072c7..0f08b9b7115e6f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java @@ -41,15 +41,23 @@ public abstract class ViewManager * 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 C createShadowNodeInstance() { * {@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 2fe412b4e437d7..ec107e83b667da 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 @@ * 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 ViewManagerRegistry(Map viewManagerMap) { 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 void run() { 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 6920301046f864..fcc81f8db88a75 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 @@ private void initView() { 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 @@ private void initView() { // 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 @@ private void initView() { 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 29edd7a68d48ba..de60d769c094f6 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 e0ca386f36f8bc..93afe79dc7f519 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 @@ private void initView() { // 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 void setOverflowInset(int left, int top, int right, int bottom) { 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 59098e792bc765..7765a3a828c15f 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