From 81564c1a3dae4222858de2a9a34089097f665e82 Mon Sep 17 00:00:00 2001
From: Janic Duplessis <janicduplessis@gmail.com>
Date: Tue, 30 Aug 2022 19:22:51 -0700
Subject: [PATCH] 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: https://github.com/facebook/react-native/pull/34129

Reviewed By: cortinico

Differential Revision: D37669469

Pulled By: philIip

fbshipit-source-id: 6cf3b2857ac60b0a1518837b9c56b9c093ed222f
---
 .../hermes/executor/HermesExecutorFactory.cpp |  6 +++++
 .../hermes/executor/HermesExecutorFactory.h   |  4 +++-
 .../hermes/inspector/chrome/Connection.cpp    | 24 +++++++------------
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp
index 53a454f8822044..e1626d4f20ccc5 100644
--- a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp
+++ b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp
@@ -212,6 +212,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,
diff --git a/ReactCommon/hermes/executor/HermesExecutorFactory.h b/ReactCommon/hermes/executor/HermesExecutorFactory.h
index 5d4468e3668bd1..d06323ec7d4ebf 100644
--- a/ReactCommon/hermes/executor/HermesExecutorFactory.h
+++ b/ReactCommon/hermes/executor/HermesExecutorFactory.h
@@ -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)) {
@@ -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_;
diff --git a/ReactCommon/hermes/inspector/chrome/Connection.cpp b/ReactCommon/hermes/inspector/chrome/Connection.cpp
index 9eeb4acc31879e..c3cf75c26d46a6 100644
--- a/ReactCommon/hermes/inspector/chrome/Connection.cpp
+++ b/ReactCommon/hermes/inspector/chrome/Connection.cpp
@@ -142,7 +142,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_;
@@ -1454,20 +1454,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) {