Skip to content

Commit

Permalink
Fix hermes profiler (#34129)
Browse files Browse the repository at this point in the history
Summary:
The hermes profiler doesn't work currently, I tracked down the problem to a couple of things.

- Need to call `registerForProfiling` to enable profiling for a specific runtime. I added the call at the same place where we enable the debugger.
- `runInExecutor` didn't work and call its callback. Not sure exactly why, but using `executor_->add` like we do in a lot of other places to run code on the executor works.
- `GetHeapUsageRequest` seems to cause some deadlocks. JS contexts were not detected reliably, I suspect this is related to deadlocks when trying to run on inspector executor. `GetHeapUsageRequest` doesn't actually need any data from the inspector so there is no need to run it on that queue. To fix it I moved the call to use `runInExecutor` instead.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Fix hermes profiler

Pull Request resolved: #34129

Reviewed By: cortinico

Differential Revision: D37669469

Pulled By: philIip

fbshipit-source-id: 6cf3b2857ac60b0a1518837b9c56b9c093ed222f
  • Loading branch information
janicduplessis authored and Dmitry Rykun committed Sep 14, 2022
1 parent 98e5134 commit 2483bc0
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 16 deletions.
6 changes: 6 additions & 0 deletions ReactCommon/hermes/executor/HermesExecutorFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ std::unique_ptr<JSExecutor> HermesExecutorFactory::createJSExecutor(
decoratedRuntime, delegate, jsQueue, timeoutInvoker_, runtimeInstaller_);
}

::hermes::vm::RuntimeConfig HermesExecutorFactory::defaultRuntimeConfig() {
return ::hermes::vm::RuntimeConfig::Builder()
.withEnableSampleProfiling(true)
.build();
}

HermesExecutor::HermesExecutor(
std::shared_ptr<jsi::Runtime> runtime,
std::shared_ptr<ExecutorDelegate> delegate,
Expand Down
4 changes: 3 additions & 1 deletion ReactCommon/hermes/executor/HermesExecutorFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class HermesExecutorFactory : public JSExecutorFactory {
JSIExecutor::RuntimeInstaller runtimeInstaller,
const JSIScopedTimeoutInvoker &timeoutInvoker =
JSIExecutor::defaultTimeoutInvoker,
::hermes::vm::RuntimeConfig runtimeConfig = ::hermes::vm::RuntimeConfig())
::hermes::vm::RuntimeConfig runtimeConfig = defaultRuntimeConfig())
: runtimeInstaller_(runtimeInstaller),
timeoutInvoker_(timeoutInvoker),
runtimeConfig_(std::move(runtimeConfig)) {
Expand All @@ -33,6 +33,8 @@ class HermesExecutorFactory : public JSExecutorFactory {
std::shared_ptr<MessageQueueThread> jsQueue) override;

private:
static ::hermes::vm::RuntimeConfig defaultRuntimeConfig();

JSIExecutor::RuntimeInstaller runtimeInstaller_;
JSIScopedTimeoutInvoker timeoutInvoker_;
::hermes::vm::RuntimeConfig runtimeConfig_;
Expand Down
24 changes: 9 additions & 15 deletions ReactCommon/hermes/inspector/chrome/Connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class Connection::Impl : public inspector::InspectorObserver,

template <typename C>
void runInExecutor(int id, C callback) {
folly::via(executor_.get(), [cb = std::move(callback)]() { cb(); });
executor_->add([cb = std::move(callback)]() { cb(); });
}

std::shared_ptr<RuntimeAdapter> runtimeAdapter_;
Expand Down Expand Up @@ -1411,20 +1411,14 @@ Connection::Impl::makePropsFromValue(
}

void Connection::Impl::handle(const m::runtime::GetHeapUsageRequest &req) {
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
resp->id = req.id;

inspector_
->executeIfEnabled(
"Runtime.getHeapUsage",
[this, req, resp](const debugger::ProgramState &state) {
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
resp->usedSize = heapInfo["hermes_allocatedBytes"];
resp->totalSize = heapInfo["hermes_heapSize"];
})
.via(executor_.get())
.thenValue([this, resp](auto &&) { sendResponseToClient(*resp); })
.thenError<std::exception>(sendErrorToClient(req.id));
runInExecutor(req.id, [this, req]() {
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
resp->id = req.id;
resp->usedSize = heapInfo["hermes_allocatedBytes"];
resp->totalSize = heapInfo["hermes_heapSize"];
sendResponseToClient(*resp);
});
}

void Connection::Impl::handle(const m::runtime::GetPropertiesRequest &req) {
Expand Down

0 comments on commit 2483bc0

Please sign in to comment.