From e97e3499c32366db00ff084ad37e32feb6b552d3 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 8 Mar 2022 04:28:51 -0800 Subject: [PATCH] Avoid string copy in event dispatching Summary: Changelog: [internal] To avoid unnecessary string copy in event pipeline, use move semantics. Event pipeline has ownership of event type. Passing it by reference ends up in a copy when `RawEvent` object is constructed. To avoid this, pass string by value through each layer and use move semantics to avoid extra copies. Reviewed By: javache Differential Revision: D34392608 fbshipit-source-id: c11d221be345665e165d9edbc360ba5a057e3890 --- .../react/fabric/jni/EventEmitterWrapper.cpp | 4 +-- .../react/fabric/jni/EventEmitterWrapper.h | 5 ++-- .../scrollview/ScrollViewEventEmitter.cpp | 4 +-- .../scrollview/ScrollViewEventEmitter.h | 2 +- .../components/view/TouchEventEmitter.cpp | 4 +-- .../components/view/TouchEventEmitter.h | 2 +- .../components/view/ViewEventEmitter.cpp | 2 +- .../components/view/ViewEventEmitter.h | 2 +- .../react/renderer/core/EventEmitter.cpp | 25 +++++++++++-------- .../react/renderer/core/EventEmitter.h | 11 ++++---- 10 files changed, 31 insertions(+), 30 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.cpp index 19a003793d3..5b2169770c1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.cpp @@ -19,7 +19,7 @@ EventEmitterWrapper::initHybrid(jni::alias_ref) { } void EventEmitterWrapper::invokeEvent( - std::string const &eventName, + std::string eventName, NativeMap *payload, int category) { // It is marginal, but possible for this to be constructed without a valid @@ -35,7 +35,7 @@ void EventEmitterWrapper::invokeEvent( } void EventEmitterWrapper::invokeUniqueEvent( - std::string const &eventName, + std::string eventName, NativeMap *payload, int customCoalesceKey) { // TODO: customCoalesceKey currently unused diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.h index 4f4d68fb5ce..c533f36be10 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.h @@ -25,10 +25,9 @@ class EventEmitterWrapper : public jni::HybridClass { SharedEventEmitter eventEmitter; - void - invokeEvent(std::string const &eventName, NativeMap *params, int category); + void invokeEvent(std::string eventName, NativeMap *params, int category); void invokeUniqueEvent( - std::string const &eventName, + std::string eventName, NativeMap *params, int customCoalesceKey); diff --git a/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.cpp b/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.cpp index 10810ad2ea3..c009fa42775 100644 --- a/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.cpp +++ b/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.cpp @@ -86,11 +86,11 @@ void ScrollViewEventEmitter::onMomentumScrollEnd( } void ScrollViewEventEmitter::dispatchScrollViewEvent( - const std::string &name, + std::string name, const ScrollViewMetrics &scrollViewMetrics, EventPriority priority) const { dispatchEvent( - name, + std::move(name), [scrollViewMetrics](jsi::Runtime &runtime) { return scrollViewMetricsPayload(runtime, scrollViewMetrics); }, diff --git a/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.h b/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.h index ca15a26940c..39274ad2daa 100644 --- a/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.h +++ b/ReactCommon/react/renderer/components/scrollview/ScrollViewEventEmitter.h @@ -38,7 +38,7 @@ class ScrollViewEventEmitter : public ViewEventEmitter { private: void dispatchScrollViewEvent( - const std::string &name, + std::string name, const ScrollViewMetrics &scrollViewMetrics, EventPriority priority = EventPriority::AsynchronousBatched) const; }; diff --git a/ReactCommon/react/renderer/components/view/TouchEventEmitter.cpp b/ReactCommon/react/renderer/components/view/TouchEventEmitter.cpp index d94144c8715..31184c167ed 100644 --- a/ReactCommon/react/renderer/components/view/TouchEventEmitter.cpp +++ b/ReactCommon/react/renderer/components/view/TouchEventEmitter.cpp @@ -60,12 +60,12 @@ static jsi::Value touchEventPayload( } void TouchEventEmitter::dispatchTouchEvent( - std::string const &type, + std::string type, TouchEvent const &event, EventPriority priority, RawEvent::Category category) const { dispatchEvent( - type, + std::move(type), [event](jsi::Runtime &runtime) { return touchEventPayload(runtime, event); }, diff --git a/ReactCommon/react/renderer/components/view/TouchEventEmitter.h b/ReactCommon/react/renderer/components/view/TouchEventEmitter.h index e652dc6e3bb..3978c1e979b 100644 --- a/ReactCommon/react/renderer/components/view/TouchEventEmitter.h +++ b/ReactCommon/react/renderer/components/view/TouchEventEmitter.h @@ -31,7 +31,7 @@ class TouchEventEmitter : public EventEmitter { private: void dispatchTouchEvent( - std::string const &type, + std::string type, TouchEvent const &event, EventPriority priority, RawEvent::Category category) const; diff --git a/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp b/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp index 23f17caacbf..4e98daa58fe 100644 --- a/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp +++ b/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp @@ -12,7 +12,7 @@ namespace react { #pragma mark - Accessibility -void ViewEventEmitter::onAccessibilityAction(const std::string &name) const { +void ViewEventEmitter::onAccessibilityAction(std::string const &name) const { dispatchEvent("accessibilityAction", [name](jsi::Runtime &runtime) { auto payload = jsi::Object(runtime); payload.setProperty(runtime, "actionName", name); diff --git a/ReactCommon/react/renderer/components/view/ViewEventEmitter.h b/ReactCommon/react/renderer/components/view/ViewEventEmitter.h index 5685a3e94ad..5ff128454b1 100644 --- a/ReactCommon/react/renderer/components/view/ViewEventEmitter.h +++ b/ReactCommon/react/renderer/components/view/ViewEventEmitter.h @@ -28,7 +28,7 @@ class ViewEventEmitter : public TouchEventEmitter { #pragma mark - Accessibility - void onAccessibilityAction(const std::string &name) const; + void onAccessibilityAction(std::string const &name) const; void onAccessibilityTap() const; void onAccessibilityMagicTap() const; void onAccessibilityEscape() const; diff --git a/ReactCommon/react/renderer/core/EventEmitter.cpp b/ReactCommon/react/renderer/core/EventEmitter.cpp index b397392f98c..eabc9463caf 100644 --- a/ReactCommon/react/renderer/core/EventEmitter.cpp +++ b/ReactCommon/react/renderer/core/EventEmitter.cpp @@ -22,9 +22,9 @@ namespace react { * Capitalizes the first letter of the event type and adds "top" prefix if * necessary (e.g. "layout" becames "topLayout"). */ -static std::string normalizeEventType(const std::string &type) { - auto prefixedType = type; - if (type.find("top", 0) != 0) { +static std::string normalizeEventType(std::string type) { + auto prefixedType = std::move(type); + if (prefixedType.find("top", 0) != 0) { prefixedType.insert(0, "top"); prefixedType[3] = static_cast(toupper(prefixedType[3])); } @@ -50,12 +50,12 @@ EventEmitter::EventEmitter( eventDispatcher_(std::move(eventDispatcher)) {} void EventEmitter::dispatchEvent( - const std::string &type, + std::string type, const folly::dynamic &payload, EventPriority priority, RawEvent::Category category) const { dispatchEvent( - type, + std::move(type), [payload](jsi::Runtime &runtime) { return valueFromDynamic(runtime, payload); }, @@ -64,15 +64,15 @@ void EventEmitter::dispatchEvent( } void EventEmitter::dispatchUniqueEvent( - const std::string &type, + std::string type, const folly::dynamic &payload) const { - dispatchUniqueEvent(type, [payload](jsi::Runtime &runtime) { + dispatchUniqueEvent(std::move(type), [payload](jsi::Runtime &runtime) { return valueFromDynamic(runtime, payload); }); } void EventEmitter::dispatchEvent( - const std::string &type, + std::string type, const ValueFactory &payloadFactory, EventPriority priority, RawEvent::Category category) const { @@ -85,12 +85,15 @@ void EventEmitter::dispatchEvent( eventDispatcher->dispatchEvent( RawEvent( - normalizeEventType(type), payloadFactory, eventTarget_, category), + normalizeEventType(std::move(type)), + payloadFactory, + eventTarget_, + category), priority); } void EventEmitter::dispatchUniqueEvent( - const std::string &type, + std::string type, const ValueFactory &payloadFactory) const { SystraceSection s("EventEmitter::dispatchUniqueEvent"); @@ -100,7 +103,7 @@ void EventEmitter::dispatchUniqueEvent( } eventDispatcher->dispatchUniqueEvent(RawEvent( - normalizeEventType(type), + normalizeEventType(std::move(type)), payloadFactory, eventTarget_, RawEvent::Category::Continuous)); diff --git a/ReactCommon/react/renderer/core/EventEmitter.h b/ReactCommon/react/renderer/core/EventEmitter.h index 0ebb31a9df4..1f5afceab2f 100644 --- a/ReactCommon/react/renderer/core/EventEmitter.h +++ b/ReactCommon/react/renderer/core/EventEmitter.h @@ -68,24 +68,23 @@ class EventEmitter { * Is used by particular subclasses only. */ void dispatchEvent( - const std::string &type, + std::string type, const ValueFactory &payloadFactory = EventEmitter::defaultPayloadFactory(), EventPriority priority = EventPriority::AsynchronousBatched, RawEvent::Category category = RawEvent::Category::Unspecified) const; void dispatchEvent( - const std::string &type, + std::string type, const folly::dynamic &payload, EventPriority priority = EventPriority::AsynchronousBatched, RawEvent::Category category = RawEvent::Category::Unspecified) const; - void dispatchUniqueEvent( - const std::string &type, - const folly::dynamic &payload) const; + void dispatchUniqueEvent(std::string type, const folly::dynamic &payload) + const; void dispatchUniqueEvent( - const std::string &type, + std::string type, const ValueFactory &payloadFactory = EventEmitter::defaultPayloadFactory()) const;