From 193b7dd1d60e54b7badf802e6cb571dcd83e4a11 Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Tue, 25 Jun 2024 02:15:58 -0700 Subject: [PATCH] Make RawPropsParser::iterateOverValues more performant (#45088) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45088 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 --- .../core/ConcreteComponentDescriptor.h | 19 ++-- .../react/renderer/core/RawProps.cpp | 6 -- .../react/renderer/core/RawProps.h | 8 -- .../react/renderer/core/RawPropsParser.cpp | 87 ++++--------------- .../react/renderer/core/RawPropsParser.h | 9 -- .../react/renderer/core/RawValue.h | 13 ++- 6 files changed, 36 insertions(+), 106 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h b/packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h index 9e42b9b524ad8e..945cebd4b6d737 100644 --- 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 #include #include @@ -119,11 +118,19 @@ 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(rawProps); +#endif + for (const auto& key : dynamic.keys()) { + const auto name = key.getString(); + shadowNodeProps->setProp( + context, + RAW_PROPS_KEY_HASH(name), + name.c_str(), + RawValue{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 index 05535e3cc138b7..49f288e358d878 100644 --- 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 index ab628513002e73..b7716be5a97aaa 100644 --- 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 index 04cf1d91e5a529..435d21ff87ffca 100644 --- 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 index 6ffb38bf546464..6016c393538e15 100644 --- 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 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 index 32d5277e0b2fe6..ef136889c2f3cb 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawValue.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawValue.h @@ -65,18 +65,17 @@ class RawValue { return *this; } + explicit RawValue(const folly::dynamic& dynamic) noexcept + : dynamic_(dynamic) {} + + explicit 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 =