Skip to content

Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises #14691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 19, 2025
+69 −45
Merged

Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises #14691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71786db
Reenable optimization in TMs to avoid going through a dynamic for cal…
acoates-ms May 19, 2025
ff1914d
Change files
acoates-ms May 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/react-native-windows-9adb4921-0e07-4622-8839-b36a3e017a3a.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Reenable optimization in TMs to avoid going through a dynamic for callbacks/promises",
"packageName": "react-native-windows",
"email": "30809111+acoates-ms@users.noreply.github.com",
"dependentChangeType": "patch"
}
86 changes: 44 additions & 42 deletions vnext/Microsoft.ReactNative/CallInvokerWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ namespace winrt::Microsoft::ReactNative {
CallInvokerWriter::CallInvokerWriter(
const std::shared_ptr<facebook::react::CallInvoker> &jsInvoker,
std::weak_ptr<LongLivedJsiRuntime> jsiRuntimeHolder) noexcept
: m_callInvoker(jsInvoker), m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)) {}
: m_callInvoker(jsInvoker),
m_jsiRuntimeHolder(std::move(jsiRuntimeHolder)),
m_threadId(std::this_thread::get_id()) {}

CallInvokerWriter::~CallInvokerWriter() {
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
Expand All @@ -26,32 +28,30 @@ CallInvokerWriter::~CallInvokerWriter() {
void CallInvokerWriter::WithResultArgs(
Mso::Functor<void(facebook::jsi::Runtime &rt, facebook::jsi::Value const *args, size_t argCount)>
handler) noexcept {
/*
if (m_jsDispatcher.HasThreadAccess()) {
VerifyElseCrash(!m_dynamicWriter);
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
const facebook::jsi::Value *args{nullptr};
size_t argCount{0};
m_jsiWriter->AccessResultAsArgs(args, argCount);
handler(jsiRuntimeHolder->Runtime(), args, argCount);
m_jsiWriter = nullptr;
}
} else {
*/
VerifyElseCrash(!m_jsiWriter);
folly::dynamic dynValue = m_dynamicWriter->TakeValue();
VerifyElseCrash(dynValue.isArray());
m_callInvoker->invokeAsync(
[handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()](
facebook::jsi::Runtime &runtime) {
std::vector<facebook::jsi::Value> args;
args.reserve(dynValue.size());
for (auto const &item : dynValue) {
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
}
handler(runtime, args.data(), args.size());
});
//}
if (m_threadId == std::this_thread::get_id() && m_fastPath) {
VerifyElseCrash(!m_dynamicWriter);
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
const facebook::jsi::Value *args{nullptr};
size_t argCount{0};
m_jsiWriter->AccessResultAsArgs(args, argCount);
handler(jsiRuntimeHolder->Runtime(), args, argCount);
m_jsiWriter = nullptr;
}
} else {
VerifyElseCrash(!m_jsiWriter);
folly::dynamic dynValue = m_dynamicWriter->TakeValue();
VerifyElseCrash(dynValue.isArray());
m_callInvoker->invokeAsync(
[handler, dynValue = std::move(dynValue), weakJsiRuntimeHolder = m_jsiRuntimeHolder, self = get_strong()](
facebook::jsi::Runtime &runtime) {
std::vector<facebook::jsi::Value> args;
args.reserve(dynValue.size());
for (auto const &item : dynValue) {
args.emplace_back(facebook::jsi::valueFromDynamic(runtime, item));
}
handler(runtime, args.data(), args.size());
});
}
}

void CallInvokerWriter::WriteNull() noexcept {
Expand Down Expand Up @@ -96,24 +96,26 @@ void CallInvokerWriter::WriteArrayEnd() noexcept {

IJSValueWriter CallInvokerWriter::GetWriter() noexcept {
if (!m_writer) {
/*
if (m_jsDispatcher.HasThreadAccess()) {
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
m_jsiWriter = winrt::make_self<JsiWriter>(jsiRuntimeHolder->Runtime());
m_writer = m_jsiWriter.as<IJSValueWriter>();
} else {
m_writer = winrt::make<JSNoopWriter>();
}
} else {
*/
m_dynamicWriter = winrt::make_self<DynamicWriter>();
m_writer = m_dynamicWriter.as<IJSValueWriter>();
//}
if (m_threadId == std::this_thread::get_id() && m_fastPath) {
if (auto jsiRuntimeHolder = m_jsiRuntimeHolder.lock()) {
m_jsiWriter = winrt::make_self<JsiWriter>(jsiRuntimeHolder->Runtime());
m_writer = m_jsiWriter.as<IJSValueWriter>();
} else {
m_writer = winrt::make<JSNoopWriter>();
}
} else {
m_dynamicWriter = winrt::make_self<DynamicWriter>();
m_writer = m_dynamicWriter.as<IJSValueWriter>();
}
}
Debug(VerifyElseCrash(m_dynamicWriter != nullptr /* || m_jsDispatcher.HasThreadAccess()*/));
Debug(VerifyElseCrash(m_dynamicWriter != nullptr || (m_threadId == std::this_thread::get_id() && m_fastPath)));
return m_writer;
}

void CallInvokerWriter::ExitCurrentCallInvokeScope() noexcept {
m_fastPath = false;
}

//===========================================================================
// JSNoopWriter implementation
//===========================================================================
Expand All @@ -129,4 +131,4 @@ void JSNoopWriter::WriteObjectEnd() noexcept {}
void JSNoopWriter::WriteArrayBegin() noexcept {}
void JSNoopWriter::WriteArrayEnd() noexcept {}

} // namespace winrt::Microsoft::ReactNative
} // namespace winrt::Microsoft::ReactNative
12 changes: 11 additions & 1 deletion vnext/Microsoft.ReactNative/CallInvokerWriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ struct CallInvokerWriter : winrt::implements<CallInvokerWriter, IJSValueWriter>
void WriteArrayBegin() noexcept;
void WriteArrayEnd() noexcept;

// This should be called before the code flow exits the scope of the CallInvoker,
// thus requiring the CallInokerWriter to call m_callInvoker->invokeAsync to call back into JS.
void ExitCurrentCallInvokeScope() noexcept;

private:
IJSValueWriter GetWriter() noexcept;

Expand All @@ -43,6 +47,12 @@ struct CallInvokerWriter : winrt::implements<CallInvokerWriter, IJSValueWriter>
winrt::com_ptr<DynamicWriter> m_dynamicWriter;
winrt::com_ptr<JsiWriter> m_jsiWriter;
IJSValueWriter m_writer;

// If a callback is invoked synchronously we can call the JS callback directly.
// However, if we post to another thread, or call the callback on the same thread but after we exit the current
// RuntimeExecutor callback, then we need to save the callback args in a dynamic and post it back to the CallInvoker
bool m_fastPath{true};
const std::thread::id m_threadId;
};

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

} // namespace winrt::Microsoft::ReactNative
} // namespace winrt::Microsoft::ReactNative
9 changes: 7 additions & 2 deletions vnext/Microsoft.ReactNative/TurboModulesProvider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,13 @@ class TurboModuleImpl : public facebook::react::TurboModule {
VerifyElseCrash(argCount > 0);
if (auto strongLongLivedObjectCollection = longLivedObjectCollection.lock()) {
auto jsiRuntimeHolder = LongLivedJsiRuntime::CreateWeak(strongLongLivedObjectCollection, rt);
auto writer = winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder);
method(
winrt::make<JsiReader>(rt, args, argCount - 1),
winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder),
writer,
MakeCallback(rt, strongLongLivedObjectCollection, args[argCount - 1]),
nullptr);
winrt::get_self<CallInvokerWriter>(writer)->ExitCurrentCallInvokeScope();
}
return facebook::jsi::Value::undefined();
});
Expand All @@ -253,9 +255,10 @@ class TurboModuleImpl : public facebook::react::TurboModule {
auto weakCallback2 = LongLivedJsiFunction::CreateWeak(
strongLongLivedObjectCollection, rt, args[argCount - 1].getObject(rt).getFunction(rt));

auto writer = winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder);
method(
winrt::make<JsiReader>(rt, args, argCount - 2),
winrt::make<CallInvokerWriter>(jsInvoker, jsiRuntimeHolder),
writer,
[weakCallback1, weakCallback2, jsiRuntimeHolder](const IJSValueWriter &writer) noexcept {
writer.as<CallInvokerWriter>()->WithResultArgs(
[weakCallback1, weakCallback2, jsiRuntimeHolder](
Expand Down Expand Up @@ -288,6 +291,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
}
});
});
winrt::get_self<CallInvokerWriter>(writer)->ExitCurrentCallInvokeScope();
}
return facebook::jsi::Value::undefined();
});
Expand Down Expand Up @@ -369,6 +373,7 @@ class TurboModuleImpl : public facebook::react::TurboModule {
}
});
});
winrt::get_self<CallInvokerWriter>(argWriter)->ExitCurrentCallInvokeScope();
});
}
return facebook::jsi::Value::undefined();
Expand Down