Fix NativeAnimatedNodesManager registering event listener multiple times

Summary:
Noticed that (LockFree)EventDispatcherImpl had NativeAnimatedNodesManager as a listener 100+ times, as listener registration can happen multiple times from Animated node creation. Since listener management on event dispatcher is thread-safe, we can avoid the thread hop for this.

Changelog: [Internal]

Reviewed By: genkikondo

Differential Revision: D36316102

fbshipit-source-id: f2f417b69885def87f88460d8b1e0b35b66726cb
This commit is contained in:
Pieter De Baets
2022-05-16 14:06:58 -07:00
committed by Facebook GitHub Bot
parent 57d3b9e2ca
commit 7c5d9ccb46
2 changed files with 17 additions and 28 deletions
@@ -397,8 +397,8 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
/**
* Given a viewTag, detect if we're running in Fabric or non-Fabric and attach an event listener
* to the correct UIManager, if necessary. This is expected to only be called from the JS thread,
* and not concurrently.
* to the correct UIManager, if necessary. This is expected to only be called from the native
* module thread, and not concurrently.
*
* @param viewTag
*/
@@ -421,8 +421,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
}
// Subscribe to UIManager (Fabric or non-Fabric) lifecycle events if we haven't yet
if ((mInitializedForFabric && mUIManagerType == UIManagerType.FABRIC)
|| (mInitializedForNonFabric && mUIManagerType == UIManagerType.DEFAULT)) {
if (mUIManagerType == UIManagerType.FABRIC ? mInitializedForFabric : mInitializedForNonFabric) {
return;
}
@@ -76,36 +76,27 @@ import java.util.Queue;
/**
* Initialize event listeners for Fabric UIManager or non-Fabric UIManager, exactly once. Once
* Fabric is the only UIManager, this logic can be simplified. This is only called on the JS
* thread.
* Fabric is the only UIManager, this logic can be simplified. This is expected to only be called
* from the native module thread.
*
* @param uiManagerType
*/
@UiThread
public void initializeEventListenerForUIManagerType(@UIManagerType final int uiManagerType) {
if ((uiManagerType == UIManagerType.FABRIC && mEventListenerInitializedForFabric)
|| (uiManagerType == UIManagerType.DEFAULT && mEventListenerInitializedForNonFabric)) {
if (uiManagerType == UIManagerType.FABRIC
? mEventListenerInitializedForFabric
: mEventListenerInitializedForNonFabric) {
return;
}
final NativeAnimatedNodesManager self = this;
mReactApplicationContext.runOnUiQueueThread(
new Runnable() {
@Override
public void run() {
UIManager uiManager =
UIManagerHelper.getUIManager(mReactApplicationContext, uiManagerType);
if (uiManager != null) {
uiManager.<EventDispatcher>getEventDispatcher().addListener(self);
if (uiManagerType == UIManagerType.FABRIC) {
mEventListenerInitializedForFabric = true;
} else {
mEventListenerInitializedForNonFabric = true;
}
}
}
});
UIManager uiManager = UIManagerHelper.getUIManager(mReactApplicationContext, uiManagerType);
if (uiManager != null) {
uiManager.<EventDispatcher>getEventDispatcher().addListener(this);
if (uiManagerType == UIManagerType.FABRIC) {
mEventListenerInitializedForFabric = true;
} else {
mEventListenerInitializedForNonFabric = true;
}
}
}
/*package*/ @Nullable
@@ -524,7 +515,6 @@ import java.util.Queue;
}
}
@UiThread
@Override
public void onEventDispatch(final Event event) {
// Events can be dispatched from any thread so we have to make sure handleEvent is run from the