Skip to content

Commit

Permalink
Make RawPropsParser::iterateOverValues more performant (facebook#45088)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#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
  • Loading branch information
dmytrorykun authored and facebook-github-bot committed Jun 25, 2024
1 parent 4a8f0ee commit 193b7dd
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#pragma once

#include <functional>
#include <memory>

#include <react/debug/react_native_assert.h>
Expand Down Expand Up @@ -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<folly::dynamic>(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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
13 changes: 6 additions & 7 deletions packages/react-native/ReactCommon/react/renderer/core/RawValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down

0 comments on commit 193b7dd

Please sign in to comment.