Skip to content

Commit cbcdaae

Browse files
Andrei Shikovfacebook-github-bot
Andrei Shikov
authored andcommitted
Use mapbuffer for ReactViewGroup
Summary: Provides an alternative way of diffing props on Android side. Instead of passing dynamic JSON values from JS, the new approach diff C++ props and serializes difference into a `MapBuffer`. Current implementation does this only for `RCTView` component to test performance, correctness and memory impact (as MapBuffers are supposed to be more compact and performant). This way of passing props allows for additional alignment between platforms, as C++ props are now source of truth for the view state, similar to iOS. At the same time, changing serialization method requires reimplementing `ViewManager` codegen (done manually for now) to account for `MapBuffer`s. Changelog: [Added][Android] - Added an experimental prop serialization path based on MapBuffer Reviewed By: mdvacca Differential Revision: D33735245 fbshipit-source-id: 1515b5fe92f6557ae55abe215ce48277c83974a6
1 parent b1a7793 commit cbcdaae

14 files changed

+777
-68
lines changed

ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java

+4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ public static JavaOnlyMap of(Object... keysAndValues) {
3131
return new JavaOnlyMap(keysAndValues);
3232
}
3333

34+
public static JavaOnlyMap from(Map<String, Object> map) {
35+
return new JavaOnlyMap(map);
36+
}
37+
3438
public static JavaOnlyMap deepClone(ReadableMap map) {
3539
JavaOnlyMap res = new JavaOnlyMap();
3640
ReadableMapKeySetIterator iter = map.keySetIterator();

ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.java

+32-2
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,36 @@ public boolean equals(@Nullable Object obj) {
301301
return thisByteBuffer.equals(otherByteBuffer);
302302
}
303303

304+
@Override
305+
public String toString() {
306+
StringBuilder builder = new StringBuilder("{");
307+
for (MapBufferEntry entry : this) {
308+
int key = entry.getKey();
309+
builder.append(key);
310+
builder.append('=');
311+
switch (entry.getType()) {
312+
case BOOL:
313+
builder.append(entry.getBoolean());
314+
break;
315+
case INT:
316+
builder.append(entry.getInt());
317+
break;
318+
case DOUBLE:
319+
builder.append(entry.getDouble());
320+
break;
321+
case STRING:
322+
builder.append(entry.getString());
323+
break;
324+
case MAP:
325+
builder.append(entry.getReadableMapBuffer().toString());
326+
break;
327+
}
328+
builder.append(',');
329+
}
330+
builder.append('}');
331+
return builder.toString();
332+
}
333+
304334
/** @return an {@link Iterator<MapBufferEntry>} for the entries of this MapBuffer. */
305335
@Override
306336
public Iterator<MapBufferEntry> iterator() {
@@ -372,15 +402,15 @@ public boolean getBoolean() {
372402
}
373403

374404
/** @return the String value that is stored in this {@link MapBufferEntry}. */
375-
public @Nullable String getString() {
405+
public String getString() {
376406
assertType(DataType.STRING);
377407
return readStringValue(mBucketOffset + VALUE_OFFSET);
378408
}
379409

380410
/**
381411
* @return the {@link ReadableMapBuffer} value that is stored in this {@link MapBufferEntry}.
382412
*/
383-
public @Nullable ReadableMapBuffer getReadableMapBuffer() {
413+
public ReadableMapBuffer getReadableMapBuffer() {
384414
assertType(DataType.MAP);
385415
return readMapBufferValue(mBucketOffset + VALUE_OFFSET);
386416
}

ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ private void preallocateView(
704704
int rootTag,
705705
int reactTag,
706706
final String componentName,
707-
@Nullable ReadableMap props,
707+
@Nullable Object props,
708708
@Nullable Object stateWrapper,
709709
@Nullable Object eventEmitterWrapper,
710710
boolean isLayoutable) {

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@ CppMountItem CppMountItem::RemoveMountItem(
2828
int index) {
2929
return {CppMountItem::Type::Remove, parentView, shadowView, {}, index};
3030
}
31-
CppMountItem CppMountItem::UpdatePropsMountItem(ShadowView const &shadowView) {
32-
return {CppMountItem::Type::UpdateProps, {}, {}, shadowView, -1};
31+
CppMountItem CppMountItem::UpdatePropsMountItem(
32+
ShadowView const &oldShadowView,
33+
ShadowView const &newShadowView) {
34+
return {
35+
CppMountItem::Type::UpdateProps, {}, oldShadowView, newShadowView, -1};
3336
}
3437
CppMountItem CppMountItem::UpdateStateMountItem(ShadowView const &shadowView) {
3538
return {CppMountItem::Type::UpdateState, {}, {}, shadowView, -1};

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ struct CppMountItem final {
3535
ShadowView const &shadowView,
3636
int index);
3737

38-
static CppMountItem UpdatePropsMountItem(ShadowView const &shadowView);
38+
static CppMountItem UpdatePropsMountItem(
39+
ShadowView const &oldShadowView,
40+
ShadowView const &newShadowView);
3941

4042
static CppMountItem UpdateStateMountItem(ShadowView const &shadowView);
4143

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp

+33-28
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "FabricMountingManager.h"
99
#include "EventEmitterWrapper.h"
1010
#include "StateWrapperImpl.h"
11+
#include "viewPropConversions.h"
1112

1213
#include <react/jni/ReadableNativeMap.h>
1314
#include <react/renderer/components/scrollview/ScrollViewProps.h>
@@ -177,11 +178,6 @@ static inline void writeIntBufferTypePreamble(
177178
}
178179
}
179180

180-
inline local_ref<ReadableMap::javaobject> castReadableMap(
181-
local_ref<ReadableNativeMap::javaobject> const &nativeMap) {
182-
return make_local(reinterpret_cast<ReadableMap::javaobject>(nativeMap.get()));
183-
}
184-
185181
inline local_ref<ReadableArray::javaobject> castReadableArray(
186182
local_ref<ReadableNativeArray::javaobject> const &nativeArray) {
187183
return make_local(
@@ -222,6 +218,22 @@ static inline float scale(Float value, Float pointScaleFactor) {
222218
return result;
223219
}
224220

221+
local_ref<jobject> FabricMountingManager::getProps(
222+
ShadowView const &oldShadowView,
223+
ShadowView const &newShadowView) {
224+
if (useMapBufferForViewProps_ &&
225+
newShadowView.traits.check(ShadowNodeTraits::Trait::View)) {
226+
auto oldProps = oldShadowView.props != nullptr
227+
? static_cast<ViewProps const &>(*oldShadowView.props)
228+
: ViewProps{};
229+
auto newProps = static_cast<ViewProps const &>(*newShadowView.props);
230+
return ReadableMapBuffer::createWithContents(
231+
viewPropsDiff(oldProps, newProps));
232+
} else {
233+
return ReadableNativeMap::newObjectCxxArgs(newShadowView.props->rawProps);
234+
}
235+
}
236+
225237
void FabricMountingManager::executeMount(
226238
MountingCoordinator::Shared const &mountingCoordinator) {
227239
std::lock_guard<std::recursive_mutex> lock(commitMutex_);
@@ -308,7 +320,8 @@ void FabricMountingManager::executeMount(
308320
if (!isVirtual) {
309321
if (oldChildShadowView.props != newChildShadowView.props) {
310322
cppUpdatePropsMountItems.push_back(
311-
CppMountItem::UpdatePropsMountItem(newChildShadowView));
323+
CppMountItem::UpdatePropsMountItem(
324+
oldChildShadowView, newChildShadowView));
312325
}
313326
if (oldChildShadowView.state != newChildShadowView.state) {
314327
cppUpdateStateMountItems.push_back(
@@ -367,7 +380,7 @@ void FabricMountingManager::executeMount(
367380
shouldRememberAllocatedViews_ ? allocationCheck : revisionCheck;
368381
if (shouldCreateView) {
369382
cppUpdatePropsMountItems.push_back(
370-
CppMountItem::UpdatePropsMountItem(newChildShadowView));
383+
CppMountItem::UpdatePropsMountItem({}, newChildShadowView));
371384
}
372385

373386
// State
@@ -484,7 +497,6 @@ void FabricMountingManager::executeMount(
484497

485498
// Allocate the intBuffer and object array, now that we know exact sizes
486499
// necessary
487-
// TODO: don't allocate at all if size is zero
488500
jintArray intBufferArray = env->NewIntArray(batchMountItemIntsSize);
489501
local_ref<JArrayClass<jobject>> objBufferArray =
490502
JArrayClass<jobject>::newArray(batchMountItemObjectsSize);
@@ -525,10 +537,8 @@ void FabricMountingManager::executeMount(
525537
int isLayoutable =
526538
mountItem.newChildShadowView.layoutMetrics != EmptyLayoutMetrics ? 1
527539
: 0;
528-
529-
local_ref<ReadableMap::javaobject> props =
530-
castReadableMap(ReadableNativeMap::newObjectCxxArgs(
531-
mountItem.newChildShadowView.props->rawProps));
540+
local_ref<JObject> props =
541+
getProps(mountItem.oldChildShadowView, mountItem.newChildShadowView);
532542

533543
// Do not hold onto Java object from C
534544
// We DO want to hold onto C object from Java, since we don't know the
@@ -584,11 +594,8 @@ void FabricMountingManager::executeMount(
584594
temp[0] = mountItem.newChildShadowView.tag;
585595
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 1, temp);
586596
intBufferPosition += 1;
587-
588-
auto newProps = mountItem.newChildShadowView.props->rawProps;
589-
local_ref<ReadableMap::javaobject> newPropsReadableMap =
590-
castReadableMap(ReadableNativeMap::newObjectCxxArgs(newProps));
591-
(*objBufferArray)[objBufferPosition++] = newPropsReadableMap.get();
597+
(*objBufferArray)[objBufferPosition++] =
598+
getProps(mountItem.oldChildShadowView, mountItem.newChildShadowView);
592599
}
593600
}
594601
if (!cppUpdateStateMountItems.empty()) {
@@ -795,15 +802,11 @@ void FabricMountingManager::preallocateShadowView(
795802

796803
bool isLayoutableShadowNode = shadowView.layoutMetrics != EmptyLayoutMetrics;
797804

798-
static auto preallocateView = jni::findClassStatic(UIManagerJavaDescriptor)
799-
->getMethod<void(
800-
jint,
801-
jint,
802-
jstring,
803-
ReadableMap::javaobject,
804-
jobject,
805-
jobject,
806-
jboolean)>("preallocateView");
805+
static auto preallocateView =
806+
jni::findClassStatic(UIManagerJavaDescriptor)
807+
->getMethod<void(
808+
jint, jint, jstring, jobject, jobject, jobject, jboolean)>(
809+
"preallocateView");
807810

808811
// Do not hold onto Java object from C
809812
// We DO want to hold onto C object from Java, since we don't know the
@@ -826,8 +829,8 @@ void FabricMountingManager::preallocateShadowView(
826829
}
827830
}
828831

829-
local_ref<ReadableMap::javaobject> props = castReadableMap(
830-
ReadableNativeMap::newObjectCxxArgs(shadowView.props->rawProps));
832+
local_ref<JObject> props = getProps({}, shadowView);
833+
831834
auto component = getPlatformComponentName(shadowView);
832835

833836
preallocateView(
@@ -942,6 +945,8 @@ FabricMountingManager::FabricMountingManager(
942945
useOverflowInset_ = doesUseOverflowInset();
943946
shouldRememberAllocatedViews_ = config->getBool(
944947
"react_native_new_architecture:remember_views_on_mount_android");
948+
useMapBufferForViewProps_ = config->getBool(
949+
"react_native_new_architecture:use_mapbuffer_for_viewprops");
945950
}
946951

947952
} // namespace react

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.h

+5
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ class FabricMountingManager {
7676
bool disableRevisionCheckForPreallocation_{false};
7777
bool useOverflowInset_{false};
7878
bool shouldRememberAllocatedViews_{false};
79+
bool useMapBufferForViewProps_{false};
80+
81+
jni::local_ref<jobject> getProps(
82+
ShadowView const &oldShadowView,
83+
ShadowView const &newShadowView);
7984
};
8085

8186
} // namespace react

0 commit comments

Comments
 (0)