Skip to content

Commit 2d92d63

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Avoid copying surface props when starting or updating a surface
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 Differential Revision: D114730310
1 parent 909f326 commit 2d92d63

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
@@ -5288,10 +5288,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
52885288
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
52895289
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
52905290
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5291-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5291+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52925292
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
52935293
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5294-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5294+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52955295
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
52965296
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
52975297
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
@@ -5099,10 +5099,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
50995099
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
51005100
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
51015101
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5102-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5102+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
51035103
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
51045104
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5105-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5105+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
51065106
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
51075107
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
51085108
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
@@ -5279,10 +5279,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
52795279
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
52805280
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
52815281
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
5282-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5282+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52835283
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
52845284
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
5285-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
5285+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
52865286
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
52875287
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
52885288
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
@@ -7480,10 +7480,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
74807480
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
74817481
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
74827482
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7483-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7483+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74847484
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
74857485
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7486-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7486+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74877487
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
74887488
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
74897489
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
@@ -7319,10 +7319,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
73197319
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
73207320
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
73217321
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7322-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7322+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
73237323
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
73247324
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7325-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7325+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
73267326
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
73277327
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
73287328
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
@@ -7471,10 +7471,10 @@ class facebook::react::UIManager : public facebook::react::ShadowTreeDelegate {
74717471
public void setDelegate(facebook::react::UIManagerDelegate* delegate);
74727472
public void setIsJSResponder(const std::shared_ptr<const facebook::react::ShadowNode>& shadowNode, bool isJSResponder, bool blockNativeResponder) const;
74737473
public void setNativeAnimatedDelegate(std::weak_ptr<facebook::react::UIManagerNativeAnimatedDelegate> delegate);
7474-
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7474+
public void setSurfaceProps(facebook::react::SurfaceId surfaceId, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74757475
public void setViewTransitionDelegate(facebook::react::UIManagerViewTransitionDelegate* delegate);
74767476
public void startEmptySurface(facebook::react::ShadowTree::Unique&& shadowTree) const noexcept;
7477-
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, const std::string& moduleName, const folly::dynamic& props, facebook::react::DisplayMode displayMode) const noexcept;
7477+
public void startSurface(facebook::react::ShadowTree::Unique&& shadowTree, std::string moduleName, folly::dynamic props, facebook::react::DisplayMode displayMode) const noexcept;
74787478
public void stopSurfaceForAnimationDelegate(facebook::react::SurfaceId surfaceId) const;
74797479
public void synchronouslyUpdateViewOnUIThread(facebook::react::Tag tag, const folly::dynamic& props);
74807480
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)