Skip to content

Commit 89fa597

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Avoid copying surface props when starting or updating a surface (#57813)
Summary: `SurfaceHandler` already takes a throwaway snapshot of its `Parameters` under `parametersMutex_` before handing them to the `UIManager`, but `UIManager::startSurface` and `UIManager::setSurfaceProps` took `moduleName` and `props` by const reference and then copy-captured them into the lambda posted to the `RuntimeExecutor`. That forced a second deep copy of the props tree — which for a real surface holds the initial route params and deep link data — on every surface start, prop update, and display mode change. Take both by value and move them into the lambda, and move at the `SurfaceHandler` call sites, so the snapshot is handed off instead of duplicated. The snapshot is a local that is dead after the call, so there is nothing left to observe the moved-from state. Changelog: [General][Changed] - `UIManager::startSurface` and `UIManager::setSurfaceProps` now take `moduleName` and `props` by value Reviewed By: zeyap, christophpurrer Differential Revision: D114730310
1 parent 44e590f commit 89fa597

12 files changed

Lines changed: 40 additions & 37 deletions

File tree

‎packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ void SurfaceHandler::start() const noexcept {
6464
if (!parameters.moduleName.empty()) {
6565
link_.uiManager->startSurface(
6666
std::move(shadowTree),
67-
parameters.moduleName,
68-
parameters.props,
67+
std::move(parameters.moduleName),
68+
std::move(parameters.props),
6969
parameters_.displayMode);
7070
} else {
7171
link_.uiManager->startEmptySurface(std::move(shadowTree));
@@ -118,8 +118,8 @@ void SurfaceHandler::setDisplayMode(DisplayMode displayMode) const noexcept {
118118

119119
link_.uiManager->setSurfaceProps(
120120
parameters.surfaceId,
121-
parameters.moduleName,
122-
parameters.props,
121+
std::move(parameters.moduleName),
122+
std::move(parameters.props),
123123
parameters.displayMode);
124124

125125
applyDisplayMode(displayMode);
@@ -164,8 +164,8 @@ void SurfaceHandler::setProps(const folly::dynamic& props) const noexcept {
164164
if (link_.status == Status::Running) {
165165
link_.uiManager->setSurfaceProps(
166166
parameters.surfaceId,
167-
parameters.moduleName,
168-
parameters.props,
167+
std::move(parameters.moduleName),
168+
std::move(parameters.props),
169169
parameters.displayMode);
170170
}
171171
}

‎packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,8 @@ void UIManager::setIsJSResponder(
234234

235235
void UIManager::startSurface(
236236
ShadowTree::Unique&& shadowTree,
237-
const std::string& moduleName,
238-
const folly::dynamic& props,
237+
std::string moduleName,
238+
folly::dynamic props,
239239
DisplayMode displayMode) const noexcept {
240240
TraceSection s("UIManager::startSurface");
241241

@@ -249,7 +249,10 @@ void UIManager::startSurface(
249249
}
250250
});
251251

252-
runtimeExecutor_([=](jsi::Runtime& runtime) {
252+
runtimeExecutor_([surfaceId,
253+
moduleName = std::move(moduleName),
254+
props = std::move(props),
255+
displayMode](jsi::Runtime& runtime) {
253256
TraceSection s("UIManager::startSurface::onRuntime");
254257
AppRegistryBinding::startSurface(
255258
runtime, surfaceId, moduleName, props, displayMode);
@@ -264,12 +267,15 @@ void UIManager::startEmptySurface(
264267

265268
void UIManager::setSurfaceProps(
266269
SurfaceId surfaceId,
267-
const std::string& moduleName,
268-
const folly::dynamic& props,
270+
std::string moduleName,
271+
folly::dynamic props,
269272
DisplayMode displayMode) const noexcept {
270273
TraceSection s("UIManager::setSurfaceProps");
271274

272-
runtimeExecutor_([=](jsi::Runtime& runtime) {
275+
runtimeExecutor_([surfaceId,
276+
moduleName = std::move(moduleName),
277+
props = std::move(props),
278+
displayMode](jsi::Runtime& runtime) {
273279
AppRegistryBinding::setSurfaceProps(
274280
runtime, surfaceId, moduleName, props, displayMode);
275281
});

‎packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,14 @@ class UIManager final : public ShadowTreeDelegate {
116116

117117
void startSurface(
118118
ShadowTree::Unique &&shadowTree,
119-
const std::string &moduleName,
120-
const folly::dynamic &props,
119+
std::string moduleName,
120+
folly::dynamic props,
121121
DisplayMode displayMode) const noexcept;
122122

123123
void startEmptySurface(ShadowTree::Unique &&shadowTree) const noexcept;
124124

125-
void setSurfaceProps(
126-
SurfaceId surfaceId,
127-
const std::string &moduleName,
128-
const folly::dynamic &props,
129-
DisplayMode displayMode) const noexcept;
125+
void setSurfaceProps(SurfaceId surfaceId, std::string moduleName, folly::dynamic props, DisplayMode displayMode)
126+
const noexcept;
130127

131128
ShadowTree::Unique stopSurface(SurfaceId surfaceId) const;
132129

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5295,10 +5295,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
52955295
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
52965296
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
52975297
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5298-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5298+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52995299
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
53005300
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5301-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5301+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
53025302
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
53035303
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
53045304
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5106,10 +5106,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
51065106
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
51075107
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
51085108
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5109-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5109+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
51105110
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
51115111
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5112-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5112+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
51135113
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
51145114
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
51155115
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5286,10 +5286,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
52865286
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
52875287
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
52885288
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5289-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5289+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52905290
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
52915291
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5292-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5292+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52935293
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
52945294
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
52955295
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7469,10 +7469,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
74697469
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
74707470
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
74717471
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7472-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7472+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74737473
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
74747474
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7475-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7475+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74767476
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
74777477
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
74787478
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7308,10 +7308,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
73087308
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
73097309
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
73107310
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7311-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7311+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
73127312
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
73137313
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7314-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7314+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
73157315
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
73167316
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
73177317
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7460,10 +7460,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
74607460
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
74617461
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
74627462
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7463-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7463+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74647464
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
74657465
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7466-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7466+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74677467
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
74687468
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
74697469
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

‎scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3746,10 +3746,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
37463746
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
37473747
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
37483748
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
3749-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
3749+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
37503750
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
37513751
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
3752-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
3752+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
37533753
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
37543754
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
37553755
public void unregisterCommitHook(facebook::react::UIManagerCommitHook& commitHook);

0 commit comments

Comments
 (0)