Skip to content

Commit 723908a

Browse files
javachemeta-codesync[bot]
authored andcommitted
Remove useOptimizedViewRegistryOnAndroid feature flag (#57228)
Summary: Pull Request resolved: #57228 The flag gated a more memory-efficient view registry implementation in `SurfaceMountingManager` (`MutableIntObjectMap` with a `ReadWriteLock` instead of `ConcurrentHashMap`). Removing the gate, keeping the optimized path, and inlining the helper methods that previously dispatched between the two implementations. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D108415790 fbshipit-source-id: b21cbb1923cf7d15e375df8d3c3c51d187eecee3
1 parent a160cac commit 723908a

21 files changed

Lines changed: 75 additions & 250 deletions

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt

Lines changed: 48 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ import com.facebook.systrace.Systrace
5555
import java.util.ArrayDeque
5656
import java.util.LinkedList
5757
import java.util.Queue
58-
import java.util.concurrent.ConcurrentHashMap
5958
import java.util.concurrent.locks.ReentrantReadWriteLock
6059
import kotlin.concurrent.Volatile
6160
import kotlin.concurrent.read
@@ -87,20 +86,9 @@ internal constructor(
8786
public var context: ThemedReactContext? = reactContext
8887
private set
8988

90-
private val tagToViewState: ConcurrentHashMap<Int, ViewState>?
91-
private val optimizedTagToViewState: MutableIntObjectMap<ViewState>?
89+
private val tagToViewState: MutableIntObjectMap<ViewState> = MutableIntObjectMap()
9290
private val registryLock = ReentrantReadWriteLock()
9391

94-
init {
95-
if (ReactNativeFeatureFlags.useOptimizedViewRegistryOnAndroid()) {
96-
tagToViewState = null
97-
optimizedTagToViewState = MutableIntObjectMap()
98-
} else {
99-
tagToViewState = ConcurrentHashMap()
100-
optimizedTagToViewState = null
101-
}
102-
}
103-
10492
private val onViewAttachMountItems: Queue<MountItem> = ArrayDeque()
10593

10694
// These are all non-null, until StopSurface is called
@@ -142,7 +130,9 @@ internal constructor(
142130
return
143131
}
144132

145-
registryPut(surfaceId, ViewState(surfaceId, rootView, rootViewManager, true))
133+
registryLock.write {
134+
tagToViewState[surfaceId] = ViewState(surfaceId, rootView, rootViewManager, true)
135+
}
146136

147137
val runnable: Runnable =
148138
object : GuardedRunnable(checkNotNull(context)) {
@@ -206,7 +196,7 @@ internal constructor(
206196
if (tagSetForStoppedSurface?.containsKey(tag) == true) {
207197
return true
208198
}
209-
return registryContains(tag)
199+
return registryLock.read { tagToViewState.containsKey(tag) }
210200
}
211201

212202
@UiThread
@@ -252,42 +242,38 @@ internal constructor(
252242
// Reset all StateWrapper objects
253243
// Since this can happen on any thread, is it possible to race between StateWrapper destruction
254244
// and some accesses from View classes in the UI thread?
255-
registryForEachValue { viewState ->
256-
viewState.stateWrapper?.destroyState()
257-
viewState.stateWrapper = null
245+
registryLock.read {
246+
tagToViewState.forEachValue { viewState ->
247+
viewState.stateWrapper?.destroyState()
248+
viewState.stateWrapper = null
258249

259-
viewState.eventEmitter?.destroy()
260-
viewState.eventEmitter = null
250+
viewState.eventEmitter?.destroy()
251+
viewState.eventEmitter = null
252+
}
261253
}
262254

263255
val runnable = Runnable {
264256
if (ReactNativeFeatureFlags.enableViewRecycling()) {
265257
viewManagerRegistry?.onSurfaceStopped(surfaceId)
266258
}
267259

268-
if (optimizedTagToViewState != null) {
269-
val viewStatesToDelete: ArrayList<ViewState>
270-
registryLock.write {
271-
val tagSetForStoppedSurface =
272-
SparseArrayCompat<Any>().also { this.tagSetForStoppedSurface = it }
273-
viewStatesToDelete = ArrayList(optimizedTagToViewState.size)
274-
optimizedTagToViewState.forEach { key, value ->
275-
tagSetForStoppedSurface[key] = this@SurfaceMountingManager
276-
viewStatesToDelete.add(value)
277-
}
278-
optimizedTagToViewState.clear()
279-
}
280-
for (viewState in viewStatesToDelete) {
281-
onViewStateDeleted(viewState)
260+
// Using this as a placeholder value in the map. We're using SparseArrayCompat
261+
// since it can efficiently represent the list of pending tags
262+
val tagSetForStoppedSurface =
263+
SparseArrayCompat<Any>().also { this.tagSetForStoppedSurface = it }
264+
265+
val viewStatesToDelete: ArrayList<ViewState>
266+
registryLock.write {
267+
viewStatesToDelete = ArrayList(tagToViewState.size)
268+
tagToViewState.forEach { key, value ->
269+
tagSetForStoppedSurface[key] = this@SurfaceMountingManager
270+
viewStatesToDelete.add(value)
282271
}
283-
} else {
284-
val tagSetForStoppedSurface =
285-
SparseArrayCompat<Any>().also { this.tagSetForStoppedSurface = it }
286-
for ((key, value) in tagToViewState!!) {
287-
tagSetForStoppedSurface[key] = this
288-
onViewStateDeleted(value)
289-
}
290-
tagToViewState!!.clear()
272+
tagToViewState.clear()
273+
}
274+
for (viewState in viewStatesToDelete) {
275+
// We must call `onDropViewInstance` on all remaining Views
276+
onViewStateDeleted(viewState)
291277
}
292278

293279
// Evict all views from cache and memory
@@ -602,7 +588,7 @@ internal constructor(
602588
this.stateWrapper = stateWrapper
603589
this.eventEmitter = eventEmitterWrapper
604590
}
605-
registryPut(reactTag, viewState)
591+
registryLock.write { tagToViewState[reactTag] = viewState }
606592

607593
if (isLayoutable) {
608594
@Suppress("UNCHECKED_CAST")
@@ -977,17 +963,9 @@ internal constructor(
977963

978964
// TODO T62717437 - Use a flag to determine that these event emitters belong to virtual nodes
979965
// only.
980-
val viewState: ViewState =
981-
if (optimizedTagToViewState != null) {
982-
registryLock.write { optimizedTagToViewState.getOrPut(reactTag) { ViewState(reactTag) } }
983-
} else {
984-
var vs = tagToViewState!![reactTag]
985-
if (vs == null) {
986-
vs = ViewState(reactTag)
987-
tagToViewState!![reactTag] = vs
988-
}
989-
vs
990-
}
966+
val viewState: ViewState = registryLock.write {
967+
tagToViewState.getOrPut(reactTag) { ViewState(reactTag) }
968+
}
991969

992970
val previousEventEmitterWrapper = viewState.eventEmitter
993971
synchronized(viewState) {
@@ -1096,7 +1074,7 @@ internal constructor(
10961074
// To delete we simply remove the tag from the registry.
10971075
// We want to rely on the correct set of MountInstructions being sent to the platform,
10981076
// or StopSurface being called, so we do not handle deleting descendants of the View.
1099-
registryRemove(reactTag)
1077+
registryLock.write { tagToViewState.remove(reactTag) }
11001078

11011079
onViewStateDeleted(viewState)
11021080
}
@@ -1141,51 +1119,13 @@ internal constructor(
11411119
}
11421120

11431121
private fun getViewState(reactTag: Int): ViewState =
1144-
registryGet(reactTag)
1122+
getNullableViewState(reactTag)
11451123
?: throw RetryableMountingLayerException(
11461124
"Unable to find viewState for tag $reactTag. Surface stopped: $isStopped"
11471125
)
11481126

1149-
private fun getNullableViewState(reactTag: Int): ViewState? = registryGet(reactTag)
1150-
1151-
private fun registryGet(tag: Int): ViewState? {
1152-
return if (optimizedTagToViewState != null) {
1153-
registryLock.read { optimizedTagToViewState[tag] }
1154-
} else {
1155-
tagToViewState!![tag]
1156-
}
1157-
}
1158-
1159-
private fun registryPut(tag: Int, state: ViewState) {
1160-
if (optimizedTagToViewState != null) {
1161-
registryLock.write { optimizedTagToViewState[tag] = state }
1162-
} else {
1163-
tagToViewState!![tag] = state
1164-
}
1165-
}
1166-
1167-
private fun registryRemove(tag: Int) {
1168-
if (optimizedTagToViewState != null) {
1169-
registryLock.write { optimizedTagToViewState.remove(tag) }
1170-
} else {
1171-
tagToViewState!!.remove(tag)
1172-
}
1173-
}
1174-
1175-
private fun registryContains(tag: Int): Boolean {
1176-
return if (optimizedTagToViewState != null) {
1177-
registryLock.read { optimizedTagToViewState.containsKey(tag) }
1178-
} else {
1179-
tagToViewState!!.containsKey(tag)
1180-
}
1181-
}
1182-
1183-
private inline fun registryForEachValue(action: (ViewState) -> Unit) {
1184-
if (optimizedTagToViewState != null) {
1185-
registryLock.read { optimizedTagToViewState.forEachValue(action) }
1186-
} else {
1187-
tagToViewState!!.values.forEach(action)
1188-
}
1127+
private inline fun getNullableViewState(reactTag: Int): ViewState? = registryLock.read {
1128+
tagToViewState[reactTag]
11891129
}
11901130

11911131
/** Applies a bitmap as the background of the view with the given tag, if it exists. */
@@ -1197,16 +1137,18 @@ internal constructor(
11971137

11981138
public fun printSurfaceState(): Unit {
11991139
FLog.e(TAG, "Views created for surface $surfaceId:")
1200-
registryForEachValue { viewState ->
1201-
val viewManagerName = viewState.viewManager?.name
1202-
val view = viewState.view
1203-
val parent = if (view != null) view.parent as View? else null
1204-
val parentTag = parent?.id
1140+
registryLock.read {
1141+
tagToViewState.forEachValue { viewState ->
1142+
val viewManagerName = viewState.viewManager?.name
1143+
val view = viewState.view
1144+
val parent = if (view != null) view.parent as View? else null
1145+
val parentTag = parent?.id
12051146

1206-
FLog.e(
1207-
TAG,
1208-
"<$viewManagerName id=${viewState.reactTag} parentTag=$parentTag isRoot=${viewState.isRoot} />",
1209-
)
1147+
FLog.e(
1148+
TAG,
1149+
"<$viewManagerName id=${viewState.reactTag} parentTag=$parentTag isRoot=${viewState.isRoot} />",
1150+
)
1151+
}
12101152
}
12111153
}
12121154

@@ -1219,7 +1161,7 @@ internal constructor(
12191161
@EventCategoryDef eventCategory: Int,
12201162
eventTimestamp: Long,
12211163
) {
1222-
val viewState = registryGet(reactTag)
1164+
val viewState = getNullableViewState(reactTag)
12231165
if (viewState == null) {
12241166
FLog.i(TAG, "Unable to invoke event: %s for reactTag: %d", eventName, reactTag)
12251167
return

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<17567d3adfba54ec2f888b50dae9efb8>>
7+
* @generated SignedSource<<e5d648cb1649c713cac9ffaac905fc80>>
88
*/
99

1010
/**
@@ -498,12 +498,6 @@ public object ReactNativeFeatureFlags {
498498
@JvmStatic
499499
public fun useNestedScrollViewAndroid(): Boolean = accessor.useNestedScrollViewAndroid()
500500

501-
/**
502-
* Use MutableIntObjectMap with ReadWriteLock instead of ConcurrentHashMap for the view registry in SurfaceMountingManager to reduce memory overhead and GC pressure.
503-
*/
504-
@JvmStatic
505-
public fun useOptimizedViewRegistryOnAndroid(): Boolean = accessor.useOptimizedViewRegistryOnAndroid()
506-
507501
/**
508502
* Use shared animation backend in C++ Animated
509503
*/

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<337b652caa9d443c8d83915e22210198>>
7+
* @generated SignedSource<<5bf52aa57fc011858db9f632930bb8fb>>
88
*/
99

1010
/**
@@ -98,7 +98,6 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
9898
private var useFabricInteropCache: Boolean? = null
9999
private var useNativeViewConfigsInBridgelessModeCache: Boolean? = null
100100
private var useNestedScrollViewAndroidCache: Boolean? = null
101-
private var useOptimizedViewRegistryOnAndroidCache: Boolean? = null
102101
private var useSharedAnimatedBackendCache: Boolean? = null
103102
private var useTraitHiddenOnAndroidCache: Boolean? = null
104103
private var useTurboModuleInteropCache: Boolean? = null
@@ -809,15 +808,6 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
809808
return cached
810809
}
811810

812-
override fun useOptimizedViewRegistryOnAndroid(): Boolean {
813-
var cached = useOptimizedViewRegistryOnAndroidCache
814-
if (cached == null) {
815-
cached = ReactNativeFeatureFlagsCxxInterop.useOptimizedViewRegistryOnAndroid()
816-
useOptimizedViewRegistryOnAndroidCache = cached
817-
}
818-
return cached
819-
}
820-
821811
override fun useSharedAnimatedBackend(): Boolean {
822812
var cached = useSharedAnimatedBackendCache
823813
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<b74d90991a4808bd1f3abfa75f3adcde>>
7+
* @generated SignedSource<<4c03e1b03360e7703ffd1d7aa0afc277>>
88
*/
99

1010
/**
@@ -184,8 +184,6 @@ public object ReactNativeFeatureFlagsCxxInterop {
184184

185185
@DoNotStrip @JvmStatic public external fun useNestedScrollViewAndroid(): Boolean
186186

187-
@DoNotStrip @JvmStatic public external fun useOptimizedViewRegistryOnAndroid(): Boolean
188-
189187
@DoNotStrip @JvmStatic public external fun useSharedAnimatedBackend(): Boolean
190188

191189
@DoNotStrip @JvmStatic public external fun useTraitHiddenOnAndroid(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<efb2b99bb1697ec28c59e4d2244717ad>>
7+
* @generated SignedSource<<98ed9f6f027f919cd31cd1e890750684>>
88
*/
99

1010
/**
@@ -179,8 +179,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
179179

180180
override fun useNestedScrollViewAndroid(): Boolean = false
181181

182-
override fun useOptimizedViewRegistryOnAndroid(): Boolean = false
183-
184182
override fun useSharedAnimatedBackend(): Boolean = false
185183

186184
override fun useTraitHiddenOnAndroid(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<164c9c2751f660ee098c12b8cf2c8b66>>
7+
* @generated SignedSource<<d17ebed58238939efc1b3d4a2d7fd3c7>>
88
*/
99

1010
/**
@@ -102,7 +102,6 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
102102
private var useFabricInteropCache: Boolean? = null
103103
private var useNativeViewConfigsInBridgelessModeCache: Boolean? = null
104104
private var useNestedScrollViewAndroidCache: Boolean? = null
105-
private var useOptimizedViewRegistryOnAndroidCache: Boolean? = null
106105
private var useSharedAnimatedBackendCache: Boolean? = null
107106
private var useTraitHiddenOnAndroidCache: Boolean? = null
108107
private var useTurboModuleInteropCache: Boolean? = null
@@ -891,16 +890,6 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
891890
return cached
892891
}
893892

894-
override fun useOptimizedViewRegistryOnAndroid(): Boolean {
895-
var cached = useOptimizedViewRegistryOnAndroidCache
896-
if (cached == null) {
897-
cached = currentProvider.useOptimizedViewRegistryOnAndroid()
898-
accessedFeatureFlags.add("useOptimizedViewRegistryOnAndroid")
899-
useOptimizedViewRegistryOnAndroidCache = cached
900-
}
901-
return cached
902-
}
903-
904893
override fun useSharedAnimatedBackend(): Boolean {
905894
var cached = useSharedAnimatedBackendCache
906895
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<b9cfc6790e36d7f7ff0ce9b87edca1e8>>
7+
* @generated SignedSource<<4f18bde7c0680691cd6fceb63c41ad65>>
88
*/
99

1010
/**
@@ -179,8 +179,6 @@ public interface ReactNativeFeatureFlagsProvider {
179179

180180
@DoNotStrip public fun useNestedScrollViewAndroid(): Boolean
181181

182-
@DoNotStrip public fun useOptimizedViewRegistryOnAndroid(): Boolean
183-
184182
@DoNotStrip public fun useSharedAnimatedBackend(): Boolean
185183

186184
@DoNotStrip public fun useTraitHiddenOnAndroid(): Boolean

0 commit comments

Comments
 (0)