Skip to content

Commit 721dff3

Browse files
acoates-msacoates-ms
authored
Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises (#14691)
* Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises * Change files
1 parent 935e5a7 commit 721dff3

File tree

4 files changed

+69
-45
lines changed

4 files changed

+69
-45
lines changed

‎change/react-native-windows-9adb4921-0e07-4622-8839-b36a3e017a3a.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises",
4+
"packageName": "react-native-windows",
5+
"email": "30809111+acoates-ms@users.noreply.github.com",
6+
"dependentChangeType": "patch"
7+
}

‎vnext/Microsoft.ReactNative/CallInvokerWriter.cpp‎

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ namespace winrt::Microsoft::ReactNative {
1515
CallInvokerWriter::CallInvokerWriter(
1616
const std::shared_ptr<facebook::react::CallInvoker> &jsInvoker,
1717
std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept
18-
: m_callInvoker(jsInvoker), m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)) {}
18+
: m_callInvoker(jsInvoker),
19+
m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)),
20+
m_threadId(std::this_thread::get_id()) {}
1921

2022
CallInvokerWriter::~CallInvokerWriter() {
2123
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
@@ -26,32 +28,30 @@ CallInvokerWriter::~CallInvokerWriter() {
2628
void CallInvokerWriter::WithResultArgs(
2729
Mso::Functor<void(facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t argCount)>
2830
handler) noexcept {
29-
/*
30-
if (m_jsDispatcher.HasThreadAccess()) {
31-
VerifyElseCrash(!m_dynamicWriter);
32-
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
33-
const facebook::jsi::Value *args{nullptr};
34-
size_t argCount{0};
35-
m_jsiWriter->AccessResultAsArgs(args, argCount);
36-
handler(jsiRuntimeHolder->Runtime(), args, argCount);
37-
m_jsiWriter = nullptr;
38-
}
39-
} else {
40-
*/
41-
VerifyElseCrash(!m_jsiWriter);
42-
folly::dynamic dynValue = m_dynamicWriter->TakeValue();
43-
VerifyElseCrash(dynValue.isArray());
44-
m_callInvoker->invokeAsync(
45-
[handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()](
46-
facebook::jsi::Runtime &runtime) {
47-
std::vector<facebook::jsi::Value> args;
48-
args.reserve(dynValue.size());
49-
for (auto const &item : dynValue) {
50-
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
51-
}
52-
handler(runtime, args.data(), args.size());
53-
});
54-
//}
31+
if (m_threadId == std::this_thread::get_id() && m_fastPath) {
32+
VerifyElseCrash(!m_dynamicWriter);
33+
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
34+
const facebook::jsi::Value *args{nullptr};
35+
size_t argCount{0};
36+
m_jsiWriter->AccessResultAsArgs(args, argCount);
37+
handler(jsiRuntimeHolder->Runtime(), args, argCount);
38+
m_jsiWriter = nullptr;
39+
}
40+
} else {
41+
VerifyElseCrash(!m_jsiWriter);
42+
folly::dynamic dynValue = m_dynamicWriter->TakeValue();
43+
VerifyElseCrash(dynValue.isArray());
44+
m_callInvoker->invokeAsync(
45+
[handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()](
46+
facebook::jsi::Runtime &runtime) {
47+
std::vector<facebook::jsi::Value> args;
48+
args.reserve(dynValue.size());
49+
for (auto const &item : dynValue) {
50+
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
51+
}
52+
handler(runtime, args.data(), args.size());
53+
});
54+
}
5555
}
5656

5757
void CallInvokerWriter::WriteNull() noexcept {
@@ -96,24 +96,26 @@ void CallInvokerWriter::WriteArrayEnd() noexcept {
9696

9797
IJSValueWriter CallInvokerWriter::GetWriter() noexcept {
9898
if (!m_writer) {
99-
/*
100-
if (m_jsDispatcher.HasThreadAccess()) {
101-
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
102-
m_jsiWriter = winrt::make_self<JsiWriter>(jsiRuntimeHolder->Runtime());
103-
m_writer = m_jsiWriter.as<IJSValueWriter>();
104-
} else {
105-
m_writer = winrt::make<JSNoopWriter>();
106-
}
107-
} else {
108-
*/
109-
m_dynamicWriter = winrt::make_self<DynamicWriter>();
110-
m_writer = m_dynamicWriter.as<IJSValueWriter>();
111-
//}
99+
if (m_threadId == std::this_thread::get_id() && m_fastPath) {
100+
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
101+
m_jsiWriter = winrt::make_self<JsiWriter>(jsiRuntimeHolder->Runtime());
102+
m_writer = m_jsiWriter.as<IJSValueWriter>();
103+
} else {
104+
m_writer = winrt::make<JSNoopWriter>();
105+
}
106+
} else {
107+
m_dynamicWriter = winrt::make_self<DynamicWriter>();
108+
m_writer = m_dynamicWriter.as<IJSValueWriter>();
109+
}
112110
}
113-
Debug(VerifyElseCrash(m_dynamicWriter != nullptr /* || m_jsDispatcher.HasThreadAccess()*/));
111+
Debug(VerifyElseCrash(m_dynamicWriter != nullptr || (m_threadId == std::this_thread::get_id() && m_fastPath)));
114112
return m_writer;
115113
}
116114

115+
void CallInvokerWriter::ExitCurrentCallInvokeScope() noexcept {
116+
m_fastPath = false;
117+
}
118+
117119
//===========================================================================
118120
// JSNoopWriter implementation
119121
//===========================================================================
@@ -129,4 +131,4 @@ void JSNoopWriter::WriteObjectEnd() noexcept {}
129131
void JSNoopWriter::WriteArrayBegin() noexcept {}
130132
void JSNoopWriter::WriteArrayEnd() noexcept {}
131133

132-
} // namespace winrt::Microsoft::ReactNative
134+
} // namespace winrt::Microsoft::ReactNative

‎vnext/Microsoft.ReactNative/CallInvokerWriter.h‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ struct CallInvokerWriter : winrt::implements<CallInvokerWriter, IJSValueWriter>
3434
void WriteArrayBegin() noexcept;
3535
void WriteArrayEnd() noexcept;
3636

37+
// This should be called before the code flow exits the scope of the CallInvoker,
38+
// thus requiring the CallInokerWriter to call m_callInvoker->invokeAsync to call back into JS.
39+
void ExitCurrentCallInvokeScope() noexcept;
40+
3741
private:
3842
IJSValueWriter GetWriter() noexcept;
3943

@@ -43,6 +47,12 @@ struct CallInvokerWriter : winrt::implements<CallInvokerWriter, IJSValueWriter>
4347
winrt::com_ptr<DynamicWriter> m_dynamicWriter;
4448
winrt::com_ptr<JsiWriter> m_jsiWriter;
4549
IJSValueWriter m_writer;
50+
51+
// If a callback is invoked synchronously we can call the JS callback directly.
52+
// However, if we post to another thread, or call the callback on the same thread but after we exit the current
53+
// RuntimeExecutor callback, then we need to save the callback args in a dynamic and post it back to the CallInvoker
54+
bool m_fastPath{true};
55+
const std::thread::id m_threadId;
4656
};
4757

4858
// Special IJSValueWriter that does nothing.
@@ -61,4 +71,4 @@ struct JSNoopWriter : winrt::implements<JSNoopWriter, IJSValueWriter> {
6171
void WriteArrayEnd() noexcept;
6272
};
6373

64-
} // namespace winrt::Microsoft::ReactNative
74+
} // namespace winrt::Microsoft::ReactNative

‎vnext/Microsoft.ReactNative/TurboModulesProvider.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,13 @@ class TurboModuleImpl : public facebook::react::TurboModule {
225225
VerifyElseCrash(argCount > 0);
226226
if (auto strongLongLivedObjectCollection = longLivedObjectCollection.lock()) {
227227
auto jsiRuntimeHolder = LongLivedJsiRuntime::CreateWeak(strongLongLivedObjectCollection, rt);
228+
auto writer = winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder);
228229
method(
229230
winrt::make<JsiReader>(rt, args, argCount - 1),
230-
winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder),
231+
writer,
231232
MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 1]),
232233
nullptr);
234+
winrt::get_self<CallInvokerWriter>(writer)->ExitCurrentCallInvokeScope();
233235
}
234236
return facebook::jsi::Value::undefined();
235237
});
@@ -253,9 +255,10 @@ class TurboModuleImpl : public facebook::react::TurboModule {
253255
auto weakCallback2 = LongLivedJsiFunction::CreateWeak(
254256
strongLongLivedObjectCollection, rt, args[argCount - 1].getObject(rt).getFunction(rt));
255257

258+
auto writer = winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder);
256259
method(
257260
winrt::make<JsiReader>(rt, args, argCount - 2),
258-
winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder),
261+
writer,
259262
[weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
260263
writer.as<CallInvokerWriter>()->WithResultArgs(
261264
[weakCallback1, weakCallback2, jsiRuntimeHolder](
@@ -288,6 +291,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
288291
}
289292
});
290293
});
294+
winrt::get_self<CallInvokerWriter>(writer)->ExitCurrentCallInvokeScope();
291295
}
292296
return facebook::jsi::Value::undefined();
293297
});
@@ -369,6 +373,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
369373
}
370374
});
371375
});
376+
winrt::get_self<CallInvokerWriter>(argWriter)->ExitCurrentCallInvokeScope();
372377
});
373378
}
374379
return facebook::jsi::Value::undefined();

0 commit comments

Comments
 (0)