Skip to content

Commit

Permalink
Fix Hermes sampling profiler (#11033)
Browse files Browse the repository at this point in the history
* Fix Hermes sampling profiler

* Change files

* Use latest Hermes ABI-safe API

* Change files

* Fix build break for missing VerifyElseCrash

* Fix binding to x86 hermes.dll

---------

Co-authored-by: Vladimir Morozov <vmoroz@users.noreply.github.com>
  • Loading branch information
vmoroz and vmoroz committed Feb 9, 2023
1 parent 68b8ef1 commit 08f6ac5
Show file tree
Hide file tree
Showing 18 changed files with 295 additions and 154 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Use latest Hermes ABI-safe API",
"packageName": "@react-native-windows/telemetry",
"email": "vmoroz@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Fix Hermes sampling profiler",
"packageName": "react-native-windows",
"email": "vmoroz@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
<packages>
<package id="Microsoft.Windows.CppWinRT" version="2.0.210312.4" targetFramework="native"/>
<package id="Microsoft.UI.Xaml" version="2.6.0" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1001-19052299" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1002-2d4bf1df" targetFramework="native"/>
</packages>
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
<package id="Microsoft.UI.Xaml" version="2.7.0-prerelease.210913003" targetFramework="native"/>
<package id="Microsoft.VCRTForwarders.140" version="1.0.2-rc" targetFramework="native"/>
<package id="Microsoft.Windows.CppWinRT" version="2.0.210312.4" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1001-19052299" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1002-2d4bf1df" targetFramework="native"/>
</packages>
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
<package id="Microsoft.UI.Xaml" version="2.7.0-prerelease.210913003" targetFramework="native"/>
<package id="Microsoft.VCRTForwarders.140" version="1.0.2-rc" targetFramework="native"/>
<package id="Microsoft.Windows.CppWinRT" version="2.0.210312.4" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1001-19052299" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1002-2d4bf1df" targetFramework="native"/>
</packages>
2 changes: 1 addition & 1 deletion packages/playground/windows/playground/packages.config
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
<package id="Microsoft.Windows.CppWinRT" version="2.0.210312.4" targetFramework="native"/>
<package id="Microsoft.UI.Xaml" version="2.7.0" targetFramework="native"/>
<package id="Microsoft.WinUI" version="3.0.0-preview4.210210.4" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1001-19052299" targetFramework="native"/>
<package id="ReactNative.Hermes.Windows" version="0.0.0-2302.1002-2d4bf1df" targetFramework="native"/>
</packages>
4 changes: 4 additions & 0 deletions vnext/Microsoft.ReactNative/IReactDispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ void ReactDispatcher::InvokeElsePost(Mso::DispatchTask &&task) const noexcept {
return jsThreadDispatcherProperty;
}

/*static*/ IReactDispatcher ReactDispatcher::GetJSDispatcher(IReactPropertyBag const &properties) noexcept {
return properties.Get(JSDispatcherProperty()).try_as<IReactDispatcher>();
}

/*static*/ IReactPropertyName ReactDispatcher::JSDispatcherTaskStartingEventName() noexcept {
static IReactPropertyName jsThreadDispatcherProperty{ReactPropertyBagHelper::GetName(
ReactPropertyBagHelper::GetNamespace(L"ReactNative.Dispatcher"), L"JSDispatcherTaskStartingEventName")};
Expand Down
1 change: 1 addition & 0 deletions vnext/Microsoft.ReactNative/IReactDispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ struct ReactDispatcher : implements<ReactDispatcher, IReactDispatcher, Mso::Reac
static void SetUIThreadDispatcher(IReactPropertyBag const &properties) noexcept;

static IReactPropertyName JSDispatcherProperty() noexcept;
static IReactDispatcher GetJSDispatcher(IReactPropertyBag const &properties) noexcept;
static IReactPropertyName JSDispatcherTaskStartingEventName() noexcept;
static IReactPropertyName JSDispatcherIdleWaitStartingEventName() noexcept;
static IReactPropertyName JSDispatcherIdleWaitCompletedEventName() noexcept;
Expand Down
9 changes: 7 additions & 2 deletions vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ ReactInstanceWin::ReactInstanceWin(
onDestroyed = m_options.OnInstanceDestroyed,
reactContext = m_reactContext]() noexcept {
whenLoaded.TryCancel(); // It only has an effect if whenLoaded was not set before
facebook::react::HermesRuntimeHolder::storeTo(ReactPropertyBag(reactContext->Properties()), nullptr);
if (onDestroyed) {
onDestroyed.Get()->Invoke(reactContext);
}
Expand Down Expand Up @@ -484,10 +485,14 @@ void ReactInstanceWin::Initialize() noexcept {
std::unique_ptr<facebook::jsi::PreparedScriptStore> preparedScriptStore = nullptr;

switch (m_options.JsiEngine()) {
case JSIEngine::Hermes:
devSettings->jsiRuntimeHolder =
case JSIEngine::Hermes: {
auto hermesRuntimeHolder =
std::make_shared<facebook::react::HermesRuntimeHolder>(devSettings, m_jsMessageThread.Load());
facebook::react::HermesRuntimeHolder::storeTo(
ReactPropertyBag(m_reactContext->Properties()), hermesRuntimeHolder);
devSettings->jsiRuntimeHolder = hermesRuntimeHolder;
break;
}
case JSIEngine::V8:
#if defined(USE_V8)
{
Expand Down
6 changes: 3 additions & 3 deletions vnext/Microsoft.ReactNative/Views/DevMenu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void DevMenuManager::CreateAndShowUI() noexcept {

std::ostringstream os;
if (Microsoft::ReactNative::HermesSamplingProfiler::IsStarted()) {
os << "Hermes Sampling profiler is running.. !";
os << "Hermes Sampling profiler is running!";
} else {
os << "Click to start.";
}
Expand Down Expand Up @@ -210,9 +210,9 @@ void DevMenuManager::CreateAndShowUI() noexcept {
if (auto strongThis = wkThis.lock()) {
strongThis->Hide();
if (!Microsoft::ReactNative::HermesSamplingProfiler::IsStarted()) {
Microsoft::ReactNative::HermesSamplingProfiler::Start();
Microsoft::ReactNative::HermesSamplingProfiler::Start(strongThis->m_context);
} else {
auto traceFilePath = co_await Microsoft::ReactNative::HermesSamplingProfiler::Stop();
auto traceFilePath = co_await Microsoft::ReactNative::HermesSamplingProfiler::Stop(strongThis->m_context);
auto uiDispatcher =
React::implementation::ReactDispatcher::GetUIDispatcher(strongThis->m_context->Properties());
uiDispatcher.Post([traceFilePath]() {
Expand Down
2 changes: 1 addition & 1 deletion vnext/PropertySheets/JSEngine.props
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<!-- Enabling this will (1) Include hermes glues in the Microsoft.ReactNative binaries AND (2) Make hermes the default engine -->
<UseHermes Condition="'$(UseHermes)' == ''">false</UseHermes>
<!-- This will be true if (1) the client want to use hermes by setting UseHermes to true OR (2) We are building for UWP where dynamic switching is enabled -->
<HermesVersion Condition="'$(HermesVersion)' == ''">0.0.0-2302.1001-19052299</HermesVersion>
<HermesVersion Condition="'$(HermesVersion)' == ''">0.0.0-2302.1002-2d4bf1df</HermesVersion>
<HermesPackage Condition="'$(HermesPackage)' == '' And Exists('$(PkgReactNative_Hermes_Windows)')">$(PkgReactNative_Hermes_Windows)</HermesPackage>
<HermesPackage Condition="'$(HermesPackage)' == ''">$(NuGetPackageRoot)\ReactNative.Hermes.Windows\$(HermesVersion)</HermesPackage>
<EnableHermesInspectorInReleaseFlavor Condition="'$(EnableHermesInspectorInReleaseFlavor)' == ''">false</EnableHermesInspectorInReleaseFlavor>
Expand Down
83 changes: 53 additions & 30 deletions vnext/Shared/HermesRuntimeHolder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,40 @@

#include "pch.h"

#include <memory>
#include <mutex>
#include "HermesRuntimeHolder.h"

#include <JSI/decorator.h>
#include <crash/verifyElseCrash.h>
#include <cxxreact/MessageQueueThread.h>
#include <cxxreact/SystraceSection.h>
#include <hermes/hermes.h>
#include "HermesRuntimeHolder.h"
#include "HermesShim.h"

#if defined(HERMES_ENABLE_DEBUGGER)
#include <hermes/inspector/chrome/Registration.h>
#endif

#include <memory>
#include <mutex>

using namespace facebook;
using namespace Microsoft::ReactNative;

namespace facebook {
namespace react {
namespace React {
using namespace winrt::Microsoft::ReactNative;
}

namespace {
namespace facebook::react {

std::unique_ptr<facebook::hermes::HermesRuntime> makeHermesRuntimeSystraced(bool enableDefaultCrashHandler) {
SystraceSection s("HermesExecutorFactory::makeHermesRuntimeSystraced");
if (enableDefaultCrashHandler) {
return HermesShim::makeHermesRuntimeWithWER();
} else {
auto runtimeConfig = ::hermes::vm::RuntimeConfig();
return HermesShim::makeHermesRuntime(runtimeConfig);
}
React::ReactPropertyId<React::ReactNonAbiValue<std::shared_ptr<HermesRuntimeHolder>>>
HermesRuntimeHolderProperty() noexcept {
static React::ReactPropertyId<React::ReactNonAbiValue<std::shared_ptr<HermesRuntimeHolder>>> propId{
L"ReactNative.HermesRuntimeHolder", L"HermesRuntimeHolder"};
return propId;
}

namespace {

#ifdef HERMES_ENABLE_DEBUGGER
class HermesExecutorRuntimeAdapter final : public facebook::hermes::inspector::RuntimeAdapter {
public:
Expand Down Expand Up @@ -71,10 +73,19 @@ class HermesExecutorRuntimeAdapter final : public facebook::hermes::inspector::R
};
#endif

std::shared_ptr<HermesShim> makeHermesShimSystraced(bool enableDefaultCrashHandler) {
SystraceSection s("HermesExecutorFactory::makeHermesRuntimeSystraced");
if (enableDefaultCrashHandler) {
return HermesShim::makeWithWER();
} else {
return HermesShim::make();
}
}

} // namespace

void HermesRuntimeHolder::crashHandler(int fileDescriptor) noexcept {
HermesShim::hermesCrashHandler(*m_hermesRuntime, fileDescriptor);
m_hermesShim->dumpCrashData(fileDescriptor);
}

void HermesRuntimeHolder::teardown() noexcept {
Expand All @@ -90,15 +101,9 @@ facebook::react::JSIEngineOverride HermesRuntimeHolder::getRuntimeType() noexcep
}

std::shared_ptr<jsi::Runtime> HermesRuntimeHolder::getRuntime() noexcept {
std::call_once(m_once_flag, [this]() { initRuntime(); });

if (!m_hermesRuntime)
std::terminate();

// Make sure that the runtime instance is not consumed from multiple threads.
if (m_own_thread_id != std::this_thread::get_id())
std::terminate();

std::call_once(m_onceFlag, [this]() { initRuntime(); });
VerifyElseCrash(m_hermesRuntime);
VerifyElseCrashSz(m_ownThreadId == std::this_thread::get_id(), "Must be accessed from JS thread.");
return m_hermesRuntime;
}

Expand All @@ -109,11 +114,11 @@ HermesRuntimeHolder::HermesRuntimeHolder(

void HermesRuntimeHolder::initRuntime() noexcept {
auto devSettings = m_weakDevSettings.lock();
if (!devSettings)
std::terminate();
VerifyElseCrash(devSettings);

m_hermesRuntime = makeHermesRuntimeSystraced(devSettings->enableDefaultCrashHandler);
m_own_thread_id = std::this_thread::get_id();
m_hermesShim = makeHermesShimSystraced(devSettings->enableDefaultCrashHandler);
m_hermesRuntime = m_hermesShim->getRuntime();
m_ownThreadId = std::this_thread::get_id();

#ifdef HERMES_ENABLE_DEBUGGER
if (devSettings->useDirectDebugger) {
Expand All @@ -132,5 +137,23 @@ void HermesRuntimeHolder::initRuntime() noexcept {
errorPrototype.setProperty(*m_hermesRuntime, "jsEngine", "hermes");
}

} // namespace react
} // namespace facebook
std::shared_ptr<HermesRuntimeHolder> HermesRuntimeHolder::loadFrom(
React::ReactPropertyBag const &propertyBag) noexcept {
return *(propertyBag.Get(HermesRuntimeHolderProperty()));
}

void HermesRuntimeHolder::storeTo(
React::ReactPropertyBag const &propertyBag,
std::shared_ptr<HermesRuntimeHolder> const &holder) noexcept {
propertyBag.Set(HermesRuntimeHolderProperty(), holder);
}

void HermesRuntimeHolder::addToProfiling() const noexcept {
m_hermesShim->addToProfiling();
}

void HermesRuntimeHolder::removeFromProfiling() const noexcept {
m_hermesShim->removeFromProfiling();
}

} // namespace facebook::react
37 changes: 26 additions & 11 deletions vnext/Shared/HermesRuntimeHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,52 @@
#include <thread>

#include <DevSettings.h>
#include <ReactPropertyBag.h>

namespace facebook::hermes {
class HermesRuntime;
}

namespace facebook {
namespace react {
namespace Microsoft::ReactNative {
class HermesShim;
}

namespace facebook::react {

class MessageQueueThread;

class HermesRuntimeHolder : public Microsoft::JSI::RuntimeHolderLazyInit {
public:
public: // RuntimeHolderLazyInit implementation.
std::shared_ptr<facebook::jsi::Runtime> getRuntime() noexcept override;
facebook::react::JSIEngineOverride getRuntimeType() noexcept override;

void crashHandler(int fileDescriptor) noexcept override;

void teardown() noexcept override;

public:
HermesRuntimeHolder(
std::shared_ptr<facebook::react::DevSettings> devSettings,
std::shared_ptr<facebook::react::MessageQueueThread> jsQueue) noexcept;

static std::shared_ptr<HermesRuntimeHolder> loadFrom(
winrt::Microsoft::ReactNative::ReactPropertyBag const &propertyBag) noexcept;

static void storeTo(
winrt::Microsoft::ReactNative::ReactPropertyBag const &propertyBag,
std::shared_ptr<HermesRuntimeHolder> const &holder) noexcept;

void addToProfiling() const noexcept;
void removeFromProfiling() const noexcept;

private:
void initRuntime() noexcept;
std::shared_ptr<facebook::hermes::HermesRuntime> m_hermesRuntime;

std::once_flag m_once_flag;
std::thread::id m_own_thread_id;

private:
std::shared_ptr<Microsoft::ReactNative::HermesShim> m_hermesShim;
std::shared_ptr<facebook::hermes::HermesRuntime> m_hermesRuntime;
std::once_flag m_onceFlag{};
std::thread::id m_ownThreadId{};
std::weak_ptr<facebook::react::DevSettings> m_weakDevSettings;
std::shared_ptr<facebook::react::MessageQueueThread> m_jsQueue;
};

} // namespace react
} // namespace facebook
} // namespace facebook::react
Loading

0 comments on commit 08f6ac5

Please sign in to comment.