chore: Replace view descriptors set with view descriptors map #8324

MatiPl01 · 2025-10-02T12:57:36Z

Summary

This PR replaces the old ViewDescriptorsSet implementation with the ViewDescriptorsMap. The main difference is that it uses a Map object to store view descriptors instead of storing them in the array. Thanks to this, now the add and remove methods have O(1) time complexity compared to the previous O(n) time complexity.

Because the iteration over a Map via the map .values() iterator is slower than the iteration over an array (because an array uses a continuous memory, thus the iteration over its items is well-optimized compared to the Map's iterator iteration that uses pointers to subsequent elements), I decided to add the toArray method on the ShareableViewDescriptors object, which is basically an extension of the SharedValue type. This conversion happens only when the previous array was invalidated because of the .add() or .remove() method call on the ViewDescriptorsMap. In all other cases, the already cached array is used.

MatiPl01 · 2025-10-02T13:25:06Z

apps/fabric-example/ios/Podfile.lock

  RNScreens: 6ced6ae8a526512a6eef6e28c2286e1fc2d378c3
  RNSVG: 287504b73fa0e90a605225aa9f852a86d5461e84
-  RNWorklets: 7119ae08263033c456c80d90794a312f2f88c956
+  RNWorklets: 991f94e4fa31fc20853e74d5d987426f8580cb0d


These changed when I installed pods. Not sure if it'd better to add these changes in a separate PR instead.

MatiPl01 · 2025-10-02T13:25:24Z

packages/react-native-reanimated/android/CMakeLists.txt

 string(APPEND CMAKE_CXX_FLAGS " -fno-omit-frame-pointer -fstack-protector-all")
 # flags to optimize the binary size
-string(APPEND CMAKE_CXX_FLAGS " -fvisibility=hidden -ffunction-sections -fdata-sections")
+string(APPEND CMAKE_CXX_FLAGS


Here as well, I ran the yarn format command and this has changed.

MatiPl01 · 2025-10-02T13:26:26Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+        if (!cachedArray.value) {
+          cachedArray.value = Array.from(sharedDescriptors.value.values());
+        }
+        return cachedArray.value;


Not sure if I should use the .modify method here. I think I don't have to as I am never modifying the already stored value (I always assign a new one), so the .value assignment seems to be fine.

MatiPl01 · 2025-10-02T13:28:37Z

packages/react-native-reanimated/src/updateProps/updateProps.ts

  updateProps = (viewDescriptors, updates, isAnimatedProps) => {
    'worklet';
-    viewDescriptors.value?.forEach((viewDescriptor) => {
+    for (const viewDescriptor of viewDescriptors.toArray()) {


for ... of loops are faster than forEach so I decided to change them as well as a part of the view descriptors optimization.

tjzel · 2025-10-02T16:09:51Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+
+  return {
+    shareableViewDescriptors: {
+      ...(sharedDescriptors as SharedValue<ReadonlyMap<ViewTag, Descriptor>>),


Why do you spread sharedDescriptors mutable here?

Because I want to add an additional field to it - the toArray method. It allowed me to get rid of type casting in the packages/react-native-reanimated/src/screenTransition/styleUpdater.ts file and to easily get the array (usually cached) from the SharedValue via viewDescriptors.toArray() in the packages/react-native-reanimated/src/updateProps/updateProps.ts.

And, since the Mutable is a JS object, extending it in this way seems to be fine.

I don't think it's safe. Mutables use the reference from makeMutable so spreading it might lead to undefined behavior. It'd be better to call Object.defineProperty instead.

Both implementations of makeMutable (i.e. makeMutableNative and makeMutableWeb) create a mutable object, which is a plain JS object so I think my approach is correct. We don't reference the mutable itself via this in the mutable object methods and every method is just a function which should still be able to use values from its closure after being destructed.

I can change it to Object.defineProperty if you prefer but it shouldn't change the behavior (at least I can't see how it may change it).

We call serializableMappingCache.set(mutable, handle);. I'd rather use Object.defineProperty for safety.

I agree with Tomek, it is safer to avoid a spread operator in that case.

piaskowyk · 2025-10-06T14:19:40Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+
+  return {
+    shareableViewDescriptors: {
+      ...(sharedDescriptors as SharedValue<ReadonlyMap<ViewTag, Descriptor>>),


I agree with Tomek, it is safer to avoid a spread operator in that case.

piaskowyk · 2025-10-06T14:24:33Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+
+export function makeViewDescriptorsMap(): ViewDescriptorsMap {
+  const sharedDescriptors = makeMutable<Map<ViewTag, Descriptor>>(new Map());
+  const cachedArray = makeMutable<Descriptor[] | null>(null);


I'm curious about the performance of this new solution with a cacheable array. Usually, the view descriptors container doesn't contain more than 3 elements (mostly just one) So, finding the index with linear complexity isn't too bad in this context.

What if we have a FlatList with hundreds of elements and each of them share the same animated style? It would register/unregister the style quite often and doing it in O(n) for each of elements doesn't seem to be performant.

I agree, it could be problematic in this cases, but it's not the primary use case. I just want to ensure that this doesn't introduce a performance regression when we have many components, each with its own style (not shared).

Ok, I see. The map is used only during renders, so the performance difference might be noticeable only during renders, not on every animations frame, so I think it is not that bad. Animations still use the cached array, which is invalidated only when new animated styles are added or old are removed (very rare cases).

Wrapping up, it shouldn't affect the performance of animations but may affect the performance of renders if we have multiple views with separate animated styles each (still I feel that this performance loss shouldn't be that much noticeable but I haven't measured the impact).

There's another case to consider. Let's imagine a long FlatList where every component uses the same animated style. The animation is playing (for example skeleton animation). As we scroll down, we'll create new animated components, and each time we have to recreate the whole array. Maybe we can replace the recreation of an array with simply pushing new descriptor to it?

I think that it usually works in such a way that new elements are rendered but old ones are removed at the same time when we scroll the list. In this case, pushing won't give us any benefit as we won't be able to perform a fast removal of elements, so the array would have to still be re-created.

The old implementation re-created it on each .remove() call made to the ViewDescriptorsSet, so for n elements removed, its time complexity was O(n^2). Now, all updates are batched until the next animation frame is executed. Thanks to this, we will perform all removals before the array is re-created so it is still better than it was.

If we consider only additions, then yes, we could optimize it a bit by pushing new descriptors to the array but I think it is a very rare case to have more elements added without old ones being removed.

piaskowyk · 2025-10-07T08:24:39Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+      sharedDescriptors.modify((descriptors) => {
+        'worklet';
+        descriptors.set(item.tag, item);
+        cachedArray.value = null;


Why we can just push to an array instead of create new one?

piaskowyk · 2025-10-07T08:28:09Z

packages/react-native-reanimated/src/ViewDescriptorsMap.ts

+
+export function makeViewDescriptorsMap(): ViewDescriptorsMap {
+  const sharedDescriptors = makeMutable<Map<ViewTag, Descriptor>>(new Map());
+  const cachedArray = makeMutable<Descriptor[] | null>(null);


There's another case to consider. Let's imagine a long FlatList where every component uses the same animated style. The animation is playing (for example skeleton animation). As we scroll down, we'll create new animated components, and each time we have to recreate the whole array. Maybe we can replace the recreation of an array with simply pushing new descriptor to it?

Optimize view descriptors set

d7553ae

MatiPl01 self-assigned this Oct 2, 2025

github-actions bot added the lib: Reanimated label Oct 2, 2025

Cleanup

97cd7b3

MatiPl01 marked this pull request as ready for review October 2, 2025 13:24

MatiPl01 commented Oct 2, 2025

View reviewed changes

MatiPl01 requested review from piaskowyk and tjzel October 2, 2025 13:28

tjzel reviewed Oct 2, 2025

View reviewed changes

MatiPl01 requested review from tjzel and tomekzaw October 6, 2025 09:01

piaskowyk reviewed Oct 6, 2025

View reviewed changes

piaskowyk reviewed Oct 7, 2025

View reviewed changes

software-mansion deleted a comment Oct 7, 2025

Uh oh!

chore: Replace view descriptors set with view descriptors map #8324

Are you sure you want to change the base?

chore: Replace view descriptors set with view descriptors map #8324

Conversation

MatiPl01 commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piaskowyk Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatiPl01 Oct 6, 2025 • edited by tjzel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MatiPl01 commented Oct 2, 2025 •

edited

Loading

piaskowyk Oct 6, 2025 •

edited

Loading

MatiPl01 Oct 6, 2025 •

edited by tjzel

Loading