From b347c3949f40b33df300a31ab7f86df4d82a00b2 Mon Sep 17 00:00:00 2001 From: Andrei Shikov Date: Thu, 10 Mar 2022 04:30:37 -0800 Subject: [PATCH] Enabled blocking access for animated queues under feature flag Summary: Blocks on queue write/drain for Animated module under a feature flag to test whether it resolves race conditions. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D34752947 fbshipit-source-id: a1b1a286772d29a7a27b5e9c3f743cac84cc2bab --- .../java/com/facebook/react/animated/BUCK | 1 + .../react/animated/NativeAnimatedModule.java | 29 +++++++++++++++++-- .../react/config/ReactFeatureFlags.java | 3 ++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK b/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK index 2a61c4707f6135..4bb711211fcc1f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/BUCK @@ -26,6 +26,7 @@ rn_android_library( react_native_target("java/com/facebook/react/common:common"), react_native_target("java/com/facebook/react/module/annotations:annotations"), react_native_target("java/com/facebook/react/modules/core:core"), + react_native_target("java/com/facebook/react/config:config"), react_native_target("java/com/facebook/react/uimanager:uimanager"), react_native_target("java/com/facebook/react/uimanager/annotations:annotations"), react_native_target("java/com/facebook/react/views/view:view"), diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index cc7064698a3e80..89cf5c36aab7aa 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -24,6 +24,7 @@ import com.facebook.react.bridge.UIManagerListener; import com.facebook.react.bridge.WritableMap; import com.facebook.react.common.annotations.VisibleForTesting; +import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.modules.core.ReactChoreographer; @@ -108,20 +109,40 @@ public long getBatchNumber() { private class ConcurrentOperationQueue { private final Queue mQueue = new ConcurrentLinkedQueue<>(); @Nullable private UIThreadOperation mPeekedOperation = null; + private boolean mSynchronizedAccess = false; @AnyThread boolean isEmpty() { return mQueue.isEmpty(); } + void setSynchronizedAccess(boolean isSynchronizedAccess) { + mSynchronizedAccess = isSynchronizedAccess; + } + @AnyThread void add(UIThreadOperation operation) { - mQueue.add(operation); + if (mSynchronizedAccess) { + synchronized (this) { + mQueue.add(operation); + } + } else { + mQueue.add(operation); + } } @UiThread void executeBatch(long maxBatchNumber, NativeAnimatedNodesManager nodesManager) { - List operations = drainQueueIntoList(maxBatchNumber); + + List operations; + + if (mSynchronizedAccess) { + synchronized (this) { + operations = drainQueueIntoList(maxBatchNumber); + } + } else { + operations = drainQueueIntoList(maxBatchNumber); + } for (UIThreadOperation operation : operations) { operation.execute(nodesManager); } @@ -210,6 +231,10 @@ protected void doFrameGuarded(final long frameTimeNanos) { } } }; + + // If shipping this flag, make sure to migrate to non-concurrent queue for efficiency + mOperations.setSynchronizedAccess(ReactFeatureFlags.enableSynchronizationForAnimated); + mPreOperations.setSynchronizedAccess(ReactFeatureFlags.enableSynchronizationForAnimated); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 7ede3c7eb6038d..0010116c606221 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -71,6 +71,9 @@ public class ReactFeatureFlags { /** Feature flag to configure eager attachment of the root view/initialisation of the JS code */ public static boolean enableEagerRootViewAttachment = false; + /** Feature flag to configure synchronized queue access for Animated module */ + public static boolean enableSynchronizationForAnimated = false; + private static boolean mapBufferSerializationEnabled = false; /** Enables or disables MapBuffer Serialization */