Skip to content

Commit 81564c1

Browse files
janicduplessisfacebook-github-bot
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 361b310 commit 81564c1

File tree

3 files changed

+18
-16
lines changed

3 files changed

+18
-16
lines changed

ReactCommon/hermes/executor/HermesExecutorFactory.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,12 @@ std::unique_ptr<JSExecutor> HermesExecutorFactory::createJSExecutor(
212212
decoratedRuntime, delegate, jsQueue, timeoutInvoker_, runtimeInstaller_);
213213
}
214214

215+
::hermes::vm::RuntimeConfig HermesExecutorFactory::defaultRuntimeConfig() {
216+
return ::hermes::vm::RuntimeConfig::Builder()
217+
.withEnableSampleProfiling(true)
218+
.build();
219+
}
220+
215221
HermesExecutor::HermesExecutor(
216222
std::shared_ptr<jsi::Runtime> runtime,
217223
std::shared_ptr<ExecutorDelegate> delegate,

ReactCommon/hermes/executor/HermesExecutorFactory.h

+3-1
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

+9-15
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ class Connection::Impl : public inspector::InspectorObserver,
142142

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

148148
std::shared_ptr<RuntimeAdapter> runtimeAdapter_;
@@ -1454,20 +1454,14 @@ Connection::Impl::makePropsFromValue(
14541454
}
14551455

14561456
void Connection::Impl::handle(const m::runtime::GetHeapUsageRequest &req) {
1457-
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
1458-
resp->id = req.id;
1459-
1460-
inspector_
1461-
->executeIfEnabled(
1462-
"Runtime.getHeapUsage",
1463-
[this, req, resp](const debugger::ProgramState &state) {
1464-
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
1465-
resp->usedSize = heapInfo["hermes_allocatedBytes"];
1466-
resp->totalSize = heapInfo["hermes_heapSize"];
1467-
})
1468-
.via(executor_.get())
1469-
.thenValue([this, resp](auto &&) { sendResponseToClient(*resp); })
1470-
.thenError<std::exception>(sendErrorToClient(req.id));
1457+
runInExecutor(req.id, [this, req]() {
1458+
auto heapInfo = getRuntime().instrumentation().getHeapInfo(false);
1459+
auto resp = std::make_shared<m::runtime::GetHeapUsageResponse>();
1460+
resp->id = req.id;
1461+
resp->usedSize = heapInfo["hermes_allocatedBytes"];
1462+
resp->totalSize = heapInfo["hermes_heapSize"];
1463+
sendResponseToClient(*resp);
1464+
});
14711465
}
14721466

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

0 commit comments

Comments
 (0)