Skip to content

Commit 9b50ea7

Browse files
janicduplessisTitozzz
authored andcommitted
Fix hermes profiler (#34129)
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
1 parent f57fb0e commit 9b50ea7

File tree

3 files changed

+18
-16
lines changed

3 files changed

+18
-16
lines changed

ReactCommon/hermes/executor/HermesExecutorFactory.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,12 @@ std::unique_ptr<JSExecutor> HermesExecutorFactory::createJSExecutor(
221221
decoratedRuntime, delegate, jsQueue, timeoutInvoker_, runtimeInstaller_);
222222
}
223223

224+
::hermes::vm::RuntimeConfig HermesExecutorFactory::defaultRuntimeConfig() {
225+
return ::hermes::vm::RuntimeConfig::Builder()
226+
.withEnableSampleProfiling(true)
227+
.build();
228+
}
229+
224230
HermesExecutor::HermesExecutor(
225231
std::shared_ptr<jsi::Runtime> runtime,
226232
std::shared_ptr<ExecutorDelegate> delegate,

ReactCommon/hermes/executor/HermesExecutorFactory.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class HermesExecutorFactory : public JSExecutorFactory {
2121
JSIExecutor::RuntimeInstaller runtimeInstaller,
2222
const JSIScopedTimeoutInvoker &timeoutInvoker =
2323
JSIExecutor::defaultTimeoutInvoker,
24-
::hermes::vm::RuntimeConfig runtimeConfig = ::hermes::vm::RuntimeConfig())
24+
::hermes::vm::RuntimeConfig runtimeConfig = defaultRuntimeConfig())
2525
: runtimeInstaller_(runtimeInstaller),
2626
timeoutInvoker_(timeoutInvoker),
2727
runtimeConfig_(std::move(runtimeConfig)) {
@@ -33,6 +33,8 @@ class HermesExecutorFactory : public JSExecutorFactory {
3333
std::shared_ptr<MessageQueueThread> jsQueue) override;
3434

3535
private:
36+
static ::hermes::vm::RuntimeConfig defaultRuntimeConfig();
37+
3638
JSIExecutor::RuntimeInstaller runtimeInstaller_;
3739
JSIScopedTimeoutInvoker timeoutInvoker_;
3840
::hermes::vm::RuntimeConfig runtimeConfig_;

ReactCommon/hermes/inspector/chrome/Connection.cpp

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class Connection::Impl : public inspector::InspectorObserver,
139139

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

145145
std::shared_ptr<RuntimeAdapter> runtimeAdapter_;
@@ -1411,20 +1411,14 @@ Connection::Impl::makePropsFromValue(
14111411
}
14121412

14131413
void Connection::Impl::handle(const m::runtime::GetHeapUsageRequest &req) {
1414-
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
1415-
resp->id = req.id;
1416-
1417-
inspector_
1418-
->executeIfEnabled(
1419-
"Runtime.getHeapUsage",
1420-
[this, req, resp](const debugger::ProgramState &state) {
1421-
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
1422-
resp->usedSize = heapInfo["hermes_allocatedBytes"];
1423-
resp->totalSize = heapInfo["hermes_heapSize"];
1424-
})
1425-
.via(executor_.get())
1426-
.thenValue([this, resp](auto &&) { sendResponseToClient(*resp); })
1427-
.thenError<std::exception>(sendErrorToClient(req.id));
1414+
runInExecutor(req.id, [this, req]() {
1415+
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
1416+
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
1417+
resp->id = req.id;
1418+
resp->usedSize = heapInfo["hermes_allocatedBytes"];
1419+
resp->totalSize = heapInfo["hermes_heapSize"];
1420+
sendResponseToClient(*resp);
1421+
});
14281422
}
14291423

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

0 commit comments

Comments
 (0)