Exclude raw props from view shadow nodes

Summary: With the `MapBuffer`-based props calculated from C++ props, there's no need to keep `rawProps` around for Android views. This change makes sure that the `rawProps` field is only initialized under the feature flag that is responsible for enabling `MapBuffer` for prop diffing, potentially decreasing memory footprint and speeding up node initialization as JS props don't have to be converted to `folly::dynamic` anymore. For layout animations, props rely on C++ values, so there's no need to update `rawProps` values either. Changelog: [Internal][Android] - Do not init `rawProps` when mapbuffer serialization is used for ViewProps. Reviewed By: mdvacca Differential Revision: D33793044 fbshipit-source-id: 35873b10d3ca8b152b25344ef2c27aff9641846f
facebook · Feb 23, 2022 · 1953f6f · 1953f6f
1 parent 5928105
commit 1953f6f
Show file tree

Hide file tree

Showing 17 changed files with 96 additions and 40 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp
@@ -223,6 +223,9 @@ local_ref<jobject> FabricMountingManager::getProps(
     ShadowView const &newShadowView) {
   if (useMapBufferForViewProps_ &&
       newShadowView.traits.check(ShadowNodeTraits::Trait::View)) {
+    react_native_assert(
+        newShadowView.props->rawProps.empty() &&
+        "Raw props must be empty when views are using mapbuffer");
     auto oldProps = oldShadowView.props != nullptr
         ? static_cast<ViewProps const &>(*oldShadowView.props)
         : ViewProps{};

diff --git a/ReactCommon/react/renderer/components/view/BUCK b/ReactCommon/react/renderer/components/view/BUCK
@@ -60,6 +60,7 @@ rn_xplat_cxx_library(
         react_native_xplat_target("react/renderer/core:core"),
         react_native_xplat_target("react/renderer/debug:debug"),
         react_native_xplat_target("react/renderer/graphics:graphics"),
+        react_native_xplat_target("react/config:config"),
         react_native_xplat_target("logger:logger"),
     ],
 )

diff --git a/ReactCommon/react/renderer/components/view/ViewProps.cpp b/ReactCommon/react/renderer/components/view/ViewProps.cpp
@@ -21,8 +21,9 @@ namespace react {
 ViewProps::ViewProps(
     const PropsParserContext &context,
     ViewProps const &sourceProps,
-    RawProps const &rawProps)
-    : YogaStylableProps(context, sourceProps, rawProps),
+    RawProps const &rawProps,
+    bool shouldSetRawProps)
+    : YogaStylableProps(context, sourceProps, rawProps, shouldSetRawProps),
       AccessibilityProps(context, sourceProps, rawProps),
       opacity(convertRawProp(
           context,

diff --git a/ReactCommon/react/renderer/components/view/ViewProps.h b/ReactCommon/react/renderer/components/view/ViewProps.h
@@ -30,7 +30,8 @@ class ViewProps : public YogaStylableProps, public AccessibilityProps {
   ViewProps(
       const PropsParserContext &context,
       ViewProps const &sourceProps,
-      RawProps const &rawProps);
+      RawProps const &rawProps,
+      bool shouldSetRawProps = true);
 
 #pragma mark - Props
 

diff --git a/ReactCommon/react/renderer/components/view/ViewPropsInterpolation.h b/ReactCommon/react/renderer/components/view/ViewPropsInterpolation.h
@@ -44,10 +44,12 @@ static inline void interpolateViewProps(
   // mounting layer. Once we can remove this, we should change `rawProps` to
   // be const again.
 #ifdef ANDROID
-  interpolatedProps->rawProps["opacity"] = interpolatedProps->opacity;
+  if (!interpolatedProps->rawProps.isNull()) {
+    interpolatedProps->rawProps["opacity"] = interpolatedProps->opacity;
 
-  interpolatedProps->rawProps["transform"] =
-      (folly::dynamic)interpolatedProps->transform;
+    interpolatedProps->rawProps["transform"] =
+        (folly::dynamic)interpolatedProps->transform;
+  }
 #endif
 }
 

diff --git a/ReactCommon/react/renderer/components/view/ViewShadowNode.cpp b/ReactCommon/react/renderer/components/view/ViewShadowNode.cpp
@@ -6,13 +6,45 @@
  */
 
 #include "ViewShadowNode.h"
+#include <react/config/ReactNativeConfig.h>
 #include <react/renderer/components/view/primitives.h>
 
 namespace facebook {
 namespace react {
 
 char const ViewComponentName[] = "View";
 
+static inline bool keepRawValuesInViewProps(PropsParserContext const &context) {
+  static bool shouldUseRawProps = true;
+
+#ifdef ANDROID
+  static bool initialized = false;
+
+  if (!initialized) {
+    auto config =
+        context.contextContainer.find<std::shared_ptr<const ReactNativeConfig>>(
+            "ReactNativeConfig");
+    if (config.has_value()) {
+      initialized = true;
+      shouldUseRawProps = !config.value()->getBool(
+          "react_native_new_architecture:use_mapbuffer_for_viewprops");
+    }
+  }
+#endif
+
+  return shouldUseRawProps;
+}
+
+ViewShadowNodeProps::ViewShadowNodeProps(
+    PropsParserContext const &context,
+    ViewShadowNodeProps const &sourceProps,
+    RawProps const &rawProps)
+    : ViewProps(
+          context,
+          sourceProps,
+          rawProps,
+          keepRawValuesInViewProps(context)){};
+
 ViewShadowNode::ViewShadowNode(
     ShadowNodeFragment const &fragment,
     ShadowNodeFamily::Shared const &family,

diff --git a/ReactCommon/react/renderer/components/view/ViewShadowNode.h b/ReactCommon/react/renderer/components/view/ViewShadowNode.h
@@ -15,12 +15,24 @@ namespace react {
 
 extern const char ViewComponentName[];
 
+/**
+ * Implementation of the ViewProps that propagates feature flag.
+ */
+class ViewShadowNodeProps final : public ViewProps {
+ public:
+  ViewShadowNodeProps() = default;
+  ViewShadowNodeProps(
+      const PropsParserContext &context,
+      ViewShadowNodeProps const &sourceProps,
+      RawProps const &rawProps);
+};
+
 /*
  * `ShadowNode` for <View> component.
  */
 class ViewShadowNode final : public ConcreteViewShadowNode<
                                  ViewComponentName,
-                                 ViewProps,
+                                 ViewShadowNodeProps,
                                  ViewEventEmitter> {
  public:
   static ShadowNodeTraits BaseTraits() {

diff --git a/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp b/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp
@@ -22,8 +22,9 @@ namespace react {
 YogaStylableProps::YogaStylableProps(
     const PropsParserContext &context,
     YogaStylableProps const &sourceProps,
-    RawProps const &rawProps)
-    : Props(context, sourceProps, rawProps),
+    RawProps const &rawProps,
+    bool shouldSetRawProps)
+    : Props(context, sourceProps, rawProps, shouldSetRawProps),
       yogaStyle(convertRawProp(context, rawProps, sourceProps.yogaStyle)){};
 
 #pragma mark - DebugStringConvertible

diff --git a/ReactCommon/react/renderer/components/view/YogaStylableProps.h b/ReactCommon/react/renderer/components/view/YogaStylableProps.h
@@ -22,7 +22,8 @@ class YogaStylableProps : public Props {
   YogaStylableProps(
       const PropsParserContext &context,
       YogaStylableProps const &sourceProps,
-      RawProps const &rawProps);
+      RawProps const &rawProps,
+      bool shouldSetRawProps = true);
 
 #pragma mark - Props
 

diff --git a/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp b/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp
@@ -74,7 +74,7 @@ class LayoutTest : public ::testing::Test {
             Element<ViewShadowNode>()
               .reference(viewShadowNodeA_)
               .props([] {
-                auto sharedProps = std::make_shared<ViewProps>();
+                auto sharedProps = std::make_shared<ViewShadowNodeProps>();
                 auto &props = *sharedProps;
                 auto &yogaStyle = props.yogaStyle;
                 yogaStyle.positionType() = YGPositionTypeAbsolute;
@@ -86,7 +86,7 @@ class LayoutTest : public ::testing::Test {
                 Element<ViewShadowNode>()
                   .reference(viewShadowNodeAB_)
                   .props([] {
-                    auto sharedProps = std::make_shared<ViewProps>();
+                    auto sharedProps = std::make_shared<ViewShadowNodeProps>();
                     auto &props = *sharedProps;
                     auto &yogaStyle = props.yogaStyle;
                     yogaStyle.positionType() = YGPositionTypeAbsolute;
@@ -100,7 +100,7 @@ class LayoutTest : public ::testing::Test {
                     Element<ViewShadowNode>()
                       .reference(viewShadowNodeABC_)
                       .props([=] {
-                        auto sharedProps = std::make_shared<ViewProps>();
+                        auto sharedProps = std::make_shared<ViewShadowNodeProps>();
                         auto &props = *sharedProps;
                         auto &yogaStyle = props.yogaStyle;
 
@@ -119,7 +119,7 @@ class LayoutTest : public ::testing::Test {
                         Element<ViewShadowNode>()
                           .reference(viewShadowNodeABCD_)
                           .props([] {
-                            auto sharedProps = std::make_shared<ViewProps>();
+                            auto sharedProps = std::make_shared<ViewShadowNodeProps>();
                             auto &props = *sharedProps;
                             auto &yogaStyle = props.yogaStyle;
                             yogaStyle.positionType() = YGPositionTypeAbsolute;
@@ -133,7 +133,7 @@ class LayoutTest : public ::testing::Test {
                     Element<ViewShadowNode>()
                       .reference(viewShadowNodeABE_)
                       .props([] {
-                        auto sharedProps = std::make_shared<ViewProps>();
+                        auto sharedProps = std::make_shared<ViewShadowNodeProps>();
                         auto &props = *sharedProps;
                         auto &yogaStyle = props.yogaStyle;
                         yogaStyle.positionType() = YGPositionTypeAbsolute;

diff --git a/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp b/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp
@@ -49,7 +49,7 @@ class YogaDirtyFlagTest : public ::testing::Test {
                     /*
                      * Some non-default props.
                      */
-                    auto mutableViewProps = std::make_shared<ViewProps>();
+                    auto mutableViewProps = std::make_shared<ViewShadowNodeProps>();
                     auto &props = *mutableViewProps;
                     props.nativeId = "native Id";
                     props.opacity = 0.5;
@@ -111,7 +111,7 @@ TEST_F(YogaDirtyFlagTest, changingNonLayoutSubPropsMustNotDirtyYogaNode) {
    */
   auto newRootShadowNode = rootShadowNode_->cloneTree(
       innerShadowNode_->getFamily(), [](ShadowNode const &oldShadowNode) {
-        auto viewProps = std::make_shared<ViewProps>();
+        auto viewProps = std::make_shared<ViewShadowNodeProps>();
         auto &props = *viewProps;
 
         props.nativeId = "some new native Id";
@@ -135,7 +135,7 @@ TEST_F(YogaDirtyFlagTest, changingLayoutSubPropsMustDirtyYogaNode) {
    */
   auto newRootShadowNode = rootShadowNode_->cloneTree(
       innerShadowNode_->getFamily(), [](ShadowNode const &oldShadowNode) {
-        auto viewProps = std::make_shared<ViewProps>();
+        auto viewProps = std::make_shared<ViewShadowNodeProps>();
         auto &props = *viewProps;
 
         props.yogaStyle.alignContent() = YGAlignBaseline;

diff --git a/ReactCommon/react/renderer/core/Props.cpp b/ReactCommon/react/renderer/core/Props.cpp
@@ -16,7 +16,8 @@ namespace react {
 Props::Props(
     const PropsParserContext &context,
     const Props &sourceProps,
-    const RawProps &rawProps)
+    const RawProps &rawProps,
+    const bool shouldSetRawProps)
     : nativeId(convertRawProp(
           context,
           rawProps,
@@ -26,7 +27,9 @@ Props::Props(
       revision(sourceProps.revision + 1)
 #ifdef ANDROID
       ,
-      rawProps((folly::dynamic)rawProps)
+      rawProps(
+          shouldSetRawProps ? (folly::dynamic)rawProps
+                            : /* null */ folly::dynamic())
 #endif
           {};
 

diff --git a/ReactCommon/react/renderer/core/Props.h b/ReactCommon/react/renderer/core/Props.h
@@ -33,7 +33,8 @@ class Props : public virtual Sealable, public virtual DebugStringConvertible {
   Props(
       const PropsParserContext &context,
       const Props &sourceProps,
-      RawProps const &rawProps);
+      RawProps const &rawProps,
+      bool shouldSetRawProps = true);
   virtual ~Props() = default;
 
   std::string nativeId;

diff --git a/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp b/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp
@@ -45,9 +45,9 @@ TEST(FindNodeAtPointTest, withoutTransform) {
           })
         })
     });
-  
+
   auto parentShadowNode = builder.build(element);
-  
+
   EXPECT_EQ(
             LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {115, 115})->getTag(), 3);
   EXPECT_EQ(LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {105, 105})->getTag(), 2);
@@ -91,7 +91,7 @@ TEST(FindNodeAtPointTest, viewIsTranslated) {
           })
         })
     });
-  
+
   auto parentShadowNode = builder.build(element);
 
   EXPECT_EQ(
@@ -125,7 +125,7 @@ TEST(FindNodeAtPointTest, viewIsScaled) {
           Element<ViewShadowNode>()
           .tag(3)
           .props([] {
-            auto sharedProps = std::make_shared<ViewProps>();
+            auto sharedProps = std::make_shared<ViewShadowNodeProps>();
             sharedProps->transform = Transform::Scale(0.5, 0.5, 0);
             return sharedProps;
           })
@@ -137,7 +137,7 @@ TEST(FindNodeAtPointTest, viewIsScaled) {
           })
         })
     });
-  
+
   auto parentShadowNode = builder.build(element);
 
   EXPECT_EQ(
@@ -175,9 +175,9 @@ TEST(FindNodeAtPointTest, overlappingViews) {
           shadowNode.setLayoutMetrics(layoutMetrics);
         })
     });
-  
+
   auto parentShadowNode = builder.build(element);
-  
+
   EXPECT_EQ(
             LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {50, 50})->getTag(), 3);
 }
@@ -198,7 +198,7 @@ TEST(FindNodeAtPointTest, overlappingViewsWithZIndex) {
         Element<ViewShadowNode>()
         .tag(2)
         .props([] {
-          auto sharedProps = std::make_shared<ViewProps>();
+          auto sharedProps = std::make_shared<ViewShadowNodeProps>();
           sharedProps->zIndex = 1;
           auto &yogaStyle = sharedProps->yogaStyle;
           yogaStyle.positionType() = YGPositionTypeAbsolute;
@@ -219,11 +219,9 @@ TEST(FindNodeAtPointTest, overlappingViewsWithZIndex) {
           shadowNode.setLayoutMetrics(layoutMetrics);
         })
     });
-  
+
   auto parentShadowNode = builder.build(element);
-  
+
   EXPECT_EQ(
             LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {50, 50})->getTag(), 2);
 }
-
-
diff --git a/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp b/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp
@@ -148,7 +148,7 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnTransformedNode) {
           shadowNode.setLayoutMetrics(layoutMetrics);
         })
         .props([] {
-          auto sharedProps = std::make_shared<ViewProps>();
+          auto sharedProps = std::make_shared<ViewShadowNodeProps>();
           sharedProps->transform = Transform::Scale(0.5, 0.5, 1);
           return sharedProps;
         })
@@ -199,7 +199,7 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnTransformedParent) {
       .children({
         Element<ViewShadowNode>()
         .props([] {
-          auto sharedProps = std::make_shared<ViewProps>();
+          auto sharedProps = std::make_shared<ViewShadowNodeProps>();
           sharedProps->transform = Transform::Scale(0.5, 0.5, 1);
           return sharedProps;
         })
@@ -280,7 +280,7 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnSameTransformedNode) {
   auto element =
     Element<ViewShadowNode>()
       .props([] {
-        auto sharedProps = std::make_shared<ViewProps>();
+        auto sharedProps = std::make_shared<ViewShadowNodeProps>();
         sharedProps->transform = Transform::Scale(2, 2, 1);
         return sharedProps;
       })

diff --git a/ReactCommon/react/renderer/element/tests/ElementTest.cpp b/ReactCommon/react/renderer/element/tests/ElementTest.cpp
@@ -26,7 +26,7 @@ TEST(ElementTest, testNormalCases) {
   auto shadowNodeAB = std::shared_ptr<ViewShadowNode>{};
   auto shadowNodeABA = std::shared_ptr<ViewShadowNode>{};
 
-  auto propsAA = std::make_shared<ViewProps>();
+  auto propsAA = std::make_shared<ViewShadowNodeProps>();
   propsAA->nativeId = "node AA";
 
   // clang-format off
@@ -51,7 +51,7 @@ TEST(ElementTest, testNormalCases) {
             .reference(shadowNodeAB)
             .tag(3)
             .props([]() {
-               auto props = std::make_shared<ViewProps>();
+               auto props = std::make_shared<ViewShadowNodeProps>();
                props->nativeId = "node AB";
                return props;
             })
@@ -60,7 +60,7 @@ TEST(ElementTest, testNormalCases) {
                 .reference(shadowNodeABA)
                 .tag(4)
                 .props([]() {
-                  auto props = std::make_shared<ViewProps>();
+                  auto props = std::make_shared<ViewShadowNodeProps>();
                   props->nativeId = "node ABA";
                   return props;
                 })

diff --git a/ReactCommon/react/renderer/mounting/tests/StackingContextTest.cpp b/ReactCommon/react/renderer/mounting/tests/StackingContextTest.cpp
@@ -158,7 +158,7 @@ class StackingContextTest : public ::testing::Test {
     rootShadowNode_ =
         std::static_pointer_cast<RootShadowNode>(rootShadowNode_->cloneTree(
             node->getFamily(), [&](ShadowNode const &oldShadowNode) {
-              auto viewProps = std::make_shared<ViewProps>();
+              auto viewProps = std::make_shared<ViewShadowNodeProps>();
               callback(*viewProps);
               return oldShadowNode.clone(ShadowNodeFragment{viewProps});
             }));