From 6f97733bb8934ce76255eceeb8d30a46a74b87ea Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 18 Jun 2019 11:23:10 -0700 Subject: [PATCH] Fabric: Changing `ComponentName` type alias from `std::string` to `char *` Summary: ComponentName is used by many core component of React Native, such as ComponentDescriptor, ShadowNode, ShadowView and so on. In all those cases this value represents the actual name of the component which came from `concreteComponentName` template parameter of ConcreteShadowNode. In all of those cases, it's raw `char const *` type. So, we don't need to use owning representation of the string (std::string) in all those places. The only exception from this is a part where we receive the name of the component from JS side. In this case, the source string comes from JS and has to be analyzed as a character sequence to find corresponding ComponentDescriptor. In my experiments, 20% of the time during diffing is spent on copying (this) `std::string`. Reviewed By: mdvacca Differential Revision: D15844407 fbshipit-source-id: a2e71505e22d09107e001bdf661d4a826bcf2dea --- ReactCommon/fabric/core/primitives/ReactPrimitives.h | 2 +- .../fabric/core/shadownode/ConcreteShadowNode.h | 2 +- .../fabric/core/tests/ComponentDescriptorTest.cpp | 12 +++++------- ReactCommon/fabric/core/tests/ShadowNodeTest.cpp | 4 ++-- .../fabric/uimanager/ComponentDescriptorRegistry.cpp | 9 +++++---- .../fabric/uimanager/ComponentDescriptorRegistry.h | 6 +++--- ReactCommon/fabric/uimanager/UIManager.cpp | 2 +- ReactCommon/fabric/uimanager/UIManager.h | 2 +- ReactCommon/fabric/uimanager/primitives.h | 2 +- 9 files changed, 20 insertions(+), 21 deletions(-) diff --git a/ReactCommon/fabric/core/primitives/ReactPrimitives.h b/ReactCommon/fabric/core/primitives/ReactPrimitives.h index 54ca67d97f5..baed263c81f 100644 --- a/ReactCommon/fabric/core/primitives/ReactPrimitives.h +++ b/ReactCommon/fabric/core/primitives/ReactPrimitives.h @@ -41,7 +41,7 @@ using ComponentHandle = int64_t; * String identifier for components used for addressing them from * JavaScript side. */ -using ComponentName = std::string; +using ComponentName = char const *; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h index b134bec8635..896569200f1 100644 --- a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h @@ -22,7 +22,7 @@ namespace react { * with many handy features. */ template < - const char *concreteComponentName, + ComponentName concreteComponentName, typename PropsT, typename EventEmitterT = EventEmitter, typename StateDataT = StateData> diff --git a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp index 3fb5575144d..362966bd623 100644 --- a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp +++ b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp @@ -16,9 +16,8 @@ TEST(ComponentDescriptorTest, createShadowNode) { std::make_shared(nullptr); ASSERT_EQ(descriptor->getComponentHandle(), TestShadowNode::Handle()); - ASSERT_STREQ( - descriptor->getComponentName().c_str(), TestShadowNode::Name().c_str()); - ASSERT_STREQ(descriptor->getComponentName().c_str(), "Test"); + ASSERT_STREQ(descriptor->getComponentName(), TestShadowNode::Name()); + ASSERT_STREQ(descriptor->getComponentName(), "Test"); const auto &raw = RawProps(folly::dynamic::object("nativeID", "abc")); SharedProps props = descriptor->cloneProps(nullptr, raw); @@ -30,9 +29,8 @@ TEST(ComponentDescriptorTest, createShadowNode) { }); ASSERT_EQ(node->getComponentHandle(), TestShadowNode::Handle()); - ASSERT_STREQ( - node->getComponentName().c_str(), TestShadowNode::Name().c_str()); - ASSERT_STREQ(node->getComponentName().c_str(), "Test"); + ASSERT_STREQ(node->getComponentName(), TestShadowNode::Name()); + ASSERT_STREQ(node->getComponentName(), "Test"); ASSERT_EQ(node->getTag(), 9); ASSERT_EQ(node->getSurfaceId(), 1); ASSERT_STREQ(node->getProps()->nativeId.c_str(), "abc"); @@ -52,7 +50,7 @@ TEST(ComponentDescriptorTest, cloneShadowNode) { }); SharedShadowNode cloned = descriptor->cloneShadowNode(*node, {}); - ASSERT_STREQ(cloned->getComponentName().c_str(), "Test"); + ASSERT_STREQ(cloned->getComponentName(), "Test"); ASSERT_EQ(cloned->getTag(), 9); ASSERT_EQ(cloned->getSurfaceId(), 1); ASSERT_STREQ(cloned->getProps()->nativeId.c_str(), "abc"); diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index 56a95cb72e8..5b87630061e 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -42,7 +42,7 @@ TEST(ShadowNodeTest, handleShadowNodeCreation) { componentDescriptor); ASSERT_FALSE(node->getSealed()); - ASSERT_STREQ(node->getComponentName().c_str(), "Test"); + ASSERT_STREQ(node->getComponentName(), "Test"); ASSERT_EQ(node->getTag(), 9); ASSERT_EQ(node->getSurfaceId(), 1); ASSERT_EQ(node->getEventEmitter(), nullptr); @@ -66,7 +66,7 @@ TEST(ShadowNodeTest, handleShadowNodeSimpleCloning) { componentDescriptor); auto node2 = std::make_shared(*node, ShadowNodeFragment{}); - ASSERT_STREQ(node->getComponentName().c_str(), "Test"); + ASSERT_STREQ(node->getComponentName(), "Test"); ASSERT_EQ(node->getTag(), 9); ASSERT_EQ(node->getSurfaceId(), 1); ASSERT_EQ(node->getEventEmitter(), nullptr); diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp index da8a830f89b..b6fd46ff537 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp @@ -34,7 +34,8 @@ void ComponentDescriptorRegistry::add( sharedComponentDescriptor; _registryByName[componentDescriptorProvider.name] = sharedComponentDescriptor; - if (componentDescriptorProvider.name == "UnimplementedNativeView") { + if (strcmp(componentDescriptorProvider.name, "UnimplementedNativeView") == + 0) { auto *self = const_cast(this); self->setFallbackComponentDescriptor(sharedComponentDescriptor); } @@ -64,7 +65,7 @@ void ComponentDescriptorRegistry::registerComponentDescriptor( _registryByName[componentName] = componentDescriptor; } -static ComponentName componentNameByReactViewName(ComponentName viewName) { +static std::string componentNameByReactViewName(std::string viewName) { // We need this function only for the transition period; // eventually, all names will be unified. @@ -114,7 +115,7 @@ static ComponentName componentNameByReactViewName(ComponentName viewName) { } ComponentDescriptor const &ComponentDescriptorRegistry::at( - ComponentName const &componentName) const { + std::string const &componentName) const { std::shared_lock lock(mutex_); auto unifiedComponentName = componentNameByReactViewName(componentName); @@ -140,7 +141,7 @@ ComponentDescriptor const &ComponentDescriptorRegistry::at( SharedShadowNode ComponentDescriptorRegistry::createNode( Tag tag, - ComponentName const &viewName, + std::string const &viewName, SurfaceId surfaceId, folly::dynamic const &props, SharedEventTarget const &eventTarget) const { diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h index 0f3254b1037..bf5a97ef11d 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h @@ -45,12 +45,12 @@ class ComponentDescriptorRegistry { void registerComponentDescriptor( SharedComponentDescriptor componentDescriptor) const; - ComponentDescriptor const &at(ComponentName const &componentName) const; + ComponentDescriptor const &at(std::string const &componentName) const; ComponentDescriptor const &at(ComponentHandle componentHandle) const; SharedShadowNode createNode( Tag tag, - ComponentName const &viewName, + std::string const &viewName, SurfaceId surfaceId, folly::dynamic const &props, SharedEventTarget const &eventTarget) const; @@ -73,7 +73,7 @@ class ComponentDescriptorRegistry { mutable better::shared_mutex mutex_; mutable better::map _registryByHandle; - mutable better::map _registryByName; + mutable better::map _registryByName; ComponentDescriptor::Shared _fallbackComponentDescriptor; ComponentDescriptorParameters parameters_{}; }; diff --git a/ReactCommon/fabric/uimanager/UIManager.cpp b/ReactCommon/fabric/uimanager/UIManager.cpp index 729661f0bd2..927b448a240 100644 --- a/ReactCommon/fabric/uimanager/UIManager.cpp +++ b/ReactCommon/fabric/uimanager/UIManager.cpp @@ -10,7 +10,7 @@ namespace react { SharedShadowNode UIManager::createNode( Tag tag, - const ComponentName &name, + std::string const &name, SurfaceId surfaceId, const RawProps &rawProps, SharedEventTarget eventTarget) const { diff --git a/ReactCommon/fabric/uimanager/UIManager.h b/ReactCommon/fabric/uimanager/UIManager.h index f8ccb65dd5b..a1c39458dc1 100644 --- a/ReactCommon/fabric/uimanager/UIManager.h +++ b/ReactCommon/fabric/uimanager/UIManager.h @@ -36,7 +36,7 @@ class UIManager { SharedShadowNode createNode( Tag tag, - const std::string &name, + std::string const &componentName, SurfaceId surfaceId, const RawProps &props, SharedEventTarget eventTarget) const; diff --git a/ReactCommon/fabric/uimanager/primitives.h b/ReactCommon/fabric/uimanager/primitives.h index c427f0428b4..e56b501da4e 100644 --- a/ReactCommon/fabric/uimanager/primitives.h +++ b/ReactCommon/fabric/uimanager/primitives.h @@ -83,7 +83,7 @@ inline static SurfaceId surfaceIdFromValue( return (SurfaceId)value.getNumber(); } -inline static ComponentName componentNameFromValue( +inline static std::string componentNameFromValue( jsi::Runtime &runtime, const jsi::Value &value) { return value.getString(runtime).utf8(runtime);