Make RawPropsParser::iterateOverValues more performant

Summary: This diff should make iterator-style prop setting more performant. - It removes some layers of indirection. Now `ConcreteComponentDescriptor` calls into `setProp` directly. - On both platforms, we will use `folly::dynamic` parser, it seems it is slightly faster. - On Android, we will reuse `props->rawProps` parsed as a `folly::dynamic` representation, instead of parsing stuff twice. Changelog: [Internal] - This hasn't been rolled out to OSS yet. Reviewed By: sammy-SC Differential Revision: D58593492
dmytrorykun · Jun 20, 2024 · 0646f58 · 0646f58
1 parent 75451d5
commit 0646f58
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 106 deletions.
diff --git a/packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h b/packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h
@@ -7,7 +7,6 @@
 
 #pragma once
 
-#include <functional>
 #include <memory>
 
 #include <react/debug/react_native_assert.h>
@@ -119,11 +118,16 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
     // Note that we just check if `Props` has this flag set, no matter
     // the type of ShadowNode; it acts as the single global flag.
     if (CoreFeatures::enablePropIteratorSetter) {
-      rawProps.iterateOverValues([&](RawPropsPropNameHash hash,
-                                     const char* propName,
-                                     const RawValue& fn) {
-        shadowNodeProps.get()->setProp(context, hash, propName, fn);
-      });
+#ifdef ANDROID
+      const auto& dynamic = shadowNodeProps->rawProps;
+#else
+      const auto& dynamic = static_cast<folly::dynamic>(rawProps);
+#endif
+      for (const auto& key : dynamic.keys()) {
+        auto& name = key.getString();
+        shadowNodeProps->setProp(
+            context, RAW_PROPS_KEY_HASH(name), name.c_str(), dynamic[key]);
+      }
     }
 
     return shadowNodeProps;

diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp
@@ -209,10 +209,4 @@ const RawValue* RawProps::at(
   return parser_->at(*this, RawPropsKey{prefix, name, suffix});
 }
 
-void RawProps::iterateOverValues(
-    const std::function<
-        void(RawPropsPropNameHash, const char*, const RawValue&)>& fn) const {
-  return parser_->iterateOverValues(*this, fn);
-}
-
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawProps.h b/packages/react-native/ReactCommon/react/renderer/core/RawProps.h
@@ -95,14 +95,6 @@ class RawProps final {
   const RawValue* at(const char* name, const char* prefix, const char* suffix)
       const noexcept;
 
-  /**
-   * Iterator functions: for when you want to iterate over values in-order
-   * instead of using `at` to access values randomly.
-   */
-  void iterateOverValues(
-      const std::function<
-          void(RawPropsPropNameHash, const char*, const RawValue&)>& fn) const;
-
  private:
   friend class RawPropsParser;
 

diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp
@@ -45,23 +45,23 @@ const RawValue* RawPropsParser::at(
     return nullptr;
   }
 
-  // Normally, keys are looked up in-order. For performance we can simply
-  // increment this key counter, and if the key is equal to the key at the next
-  // index, there's no need to do any lookups. However, it's possible for keys
-  // to be accessed out-of-order or multiple times, in which case we start
-  // searching again from index 0.
-  // To prevent infinite loops (which can occur if you look up a key that
-  // doesn't exist) we keep track of whether or not we've already looped around,
-  // and log and return nullptr if so. However, we ONLY do this in debug mode,
-  // where you're more likely to look up a nonexistent key as part of debugging.
-  // You can (and must) ensure infinite loops are not possible in production by:
-  // (1) constructing all props objects without conditionals, or (2) if there
-  // are conditionals, ensure that in the parsing setup case, the Props
-  // constructor will access _all_ possible props. To ensure this performance
-  // optimization is utilized, always access props in the same order every time.
-  // This is trivial if you have a simple Props constructor, but difficult or
-  // impossible if you have a shared sub-prop Struct that is used by multiple
-  // parent Props.
+// Normally, keys are looked up in-order. For performance we can simply
+// increment this key counter, and if the key is equal to the key at the next
+// index, there's no need to do any lookups. However, it's possible for keys
+// to be accessed out-of-order or multiple times, in which case we start
+// searching again from index 0.
+// To prevent infinite loops (which can occur if you look up a key that
+// doesn't exist) we keep track of whether or not we've already looped around,
+// and log and return nullptr if so. However, we ONLY do this in debug mode,
+// where you're more likely to look up a nonexistent key as part of debugging.
+// You can (and must) ensure infinite loops are not possible in production by:
+// (1) constructing all props objects without conditionals, or (2) if there
+// are conditionals, ensure that in the parsing setup case, the Props
+// constructor will access _all_ possible props. To ensure this performance
+// optimization is utilized, always access props in the same order every time.
+// This is trivial if you have a simple Props constructor, but difficult or
+// impossible if you have a shared sub-prop Struct that is used by multiple
+// parent Props.
 #ifdef REACT_NATIVE_DEBUG
   bool resetLoop = false;
 #endif
@@ -168,57 +168,4 @@ void RawPropsParser::preparse(const RawProps& rawProps) const noexcept {
   }
 }
 
-/**
- * To be used by RawProps only. Value iterator functions.
- */
-void RawPropsParser::iterateOverValues(
-    const RawProps& rawProps,
-    const std::function<
-        void(RawPropsPropNameHash, const char*, const RawValue&)>& visit)
-    const {
-  switch (rawProps.mode_) {
-    case RawProps::Mode::Empty:
-      return;
-
-    case RawProps::Mode::JSI: {
-      auto& runtime = *rawProps.runtime_;
-      if (!rawProps.value_.isObject()) {
-        LOG(ERROR) << "Preparse props: rawProps value is not object";
-      }
-      react_native_assert(rawProps.value_.isObject());
-      auto object = rawProps.value_.asObject(runtime);
-
-      auto names = object.getPropertyNames(runtime);
-      auto count = names.size(runtime);
-
-      for (size_t i = 0; i < count; i++) {
-        auto nameValue = names.getValueAtIndex(runtime, i).getString(runtime);
-        auto value = object.getProperty(runtime, nameValue);
-
-        auto name = nameValue.utf8(runtime);
-
-        auto nameHash = RAW_PROPS_KEY_HASH(name);
-        auto rawValue = RawValue(jsi::dynamicFromValue(runtime, value));
-
-        visit(nameHash, name.c_str(), rawValue);
-      }
-
-      break;
-    }
-
-    case RawProps::Mode::Dynamic: {
-      const auto& dynamic = rawProps.dynamic_;
-
-      for (const auto& pair : dynamic.items()) {
-        auto name = pair.first.getString();
-
-        auto nameHash = RAW_PROPS_KEY_HASH(name);
-        auto rawValue = RawValue{pair.second};
-        visit(nameHash, name.c_str(), rawValue);
-      }
-      break;
-    }
-  }
-}
-
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h b/packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.h
@@ -73,15 +73,6 @@ class RawPropsParser final {
   const RawValue* at(const RawProps& rawProps, const RawPropsKey& key)
       const noexcept;
 
-  /**
-   * To be used by RawProps only. Value iterator functions.
-   */
-  void iterateOverValues(
-      const RawProps& rawProps,
-      const std::function<
-          void(RawPropsPropNameHash, const char*, const RawValue&)>& visit)
-      const;
-
   mutable std::vector<RawPropsKey> keys_{};
   mutable RawPropsKeyMap nameToIndex_{};
   mutable bool ready_{false};

diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawValue.h b/packages/react-native/ReactCommon/react/renderer/core/RawValue.h
@@ -65,18 +65,15 @@ class RawValue {
     return *this;
   }
 
+  RawValue(const folly::dynamic& dynamic) noexcept : dynamic_(dynamic) {}
+
+  RawValue(folly::dynamic&& dynamic) noexcept : dynamic_(std::move(dynamic)) {}
+
  private:
   friend class RawProps;
   friend class RawPropsParser;
   friend class UIManagerBinding;
 
-  /*
-   * Arbitrary constructors are private only for RawProps and internal usage.
-   */
-  RawValue(const folly::dynamic& dynamic) noexcept : dynamic_(dynamic) {}
-
-  RawValue(folly::dynamic&& dynamic) noexcept : dynamic_(std::move(dynamic)) {}
-
   /*
    * Copy constructor and copy assignment operator would be private and only for
    * internal use, but it's needed for user-code that does `auto val =