Move compilation StackExecutor to Runtime

Summary:
This allows us to use `StackExecutor` for both local and global `eval`.
Move the consideration of stack space into the VM, and the compiler no
longer uses it directly.

NOTE: As is, the diff deliberately keeps the changes in HBC.cpp small to
make it easier to pick into stable if necessary. We could refactor
things to avoid taking the `void *` argument in the worker functions.

Reviewed By: tmikov

Differential Revision: D66398356

fbshipit-source-id: a11539ecf028bd5c609ae8a880af8f0f1376edd2

include/hermes/BCGen/HBC/BCProviderFromSrc.h

@@ -10,7 +10,6 @@
 
 #include "hermes/BCGen/HBC/BCProvider.h"
 #include "hermes/BCGen/HBC/Bytecode.h"
-#include "hermes/Support/StackExecutor.h"
 
 #include "llvh/ADT/Optional.h"
 
@@ -100,18 +99,6 @@ class BCProviderFromSrc final : public BCProviderBase {
   /// variables/information.
   CompilationData compilationData_;
 
-  /// Use an 8MB stack, which is the default size on mac and linux.
-  static constexpr size_t kExecutorStackSize = 1 << 23;
-
-  /// Idle for 1 second before letting the executor thread be cleaned up,
-  /// after which further tasks will start a new thread.
-  static constexpr std::chrono::milliseconds kExecutorTimeout =
-      std::chrono::milliseconds(1000);
-
-  /// The executor used to run the compiler.
-  std::shared_ptr<StackExecutor> stackExecutor_ =
-      newStackExecutor(kExecutorStackSize, kExecutorTimeout);
-
   /// The BytecodeModule that provides the bytecode data.
   /// Placed below CompilationData to ensure its destruction before the
   /// Module gets deleted.
@@ -257,10 +244,6 @@ class BCProviderFromSrc final : public BCProviderBase {
     sourceHash_ = hash;
   };
 
-  StackExecutor &getStackExecutor() {
-    return *stackExecutor_;
-  }
-
   static bool classof(const BCProviderBase *provider) {
     return provider->getKind() == BCProviderKind::BCProviderFromSrc;
   }

include/hermes/VM/Runtime.h

@@ -14,6 +14,7 @@
 #include "hermes/Public/RuntimeConfig.h"
 #include "hermes/Support/Compiler.h"
 #include "hermes/Support/ErrorHandling.h"
+#include "hermes/Support/StackExecutor.h"
 #include "hermes/Support/StackOverflowGuard.h"
 #include "hermes/VM/AllocOptions.h"
 #include "hermes/VM/AllocResult.h"
@@ -881,6 +882,11 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
   }
 #endif
 
+  /// \return the stored StackExecutor.
+  StackExecutor &getStackExecutor() {
+    return *stackExecutor_;
+  }
+
   /// \return the newly allocated script ID, incrementing the internal counter.
   facebook::hermes::debugger::ScriptID allocateScriptId() {
     return nextScriptId_++;
@@ -1352,6 +1358,18 @@ class Runtime : public RuntimeBase, public HandleRootOwner {
   Debugger debugger_{*this};
 #endif
 
+  /// Use an 8MB stack, which is the default size on mac and linux.
+  static constexpr size_t kExecutorStackSize = 1 << 23;
+
+  /// Idle for 1 second before letting the executor thread be cleaned up,
+  /// after which further tasks will start a new thread.
+  static constexpr std::chrono::milliseconds kExecutorTimeout =
+      std::chrono::milliseconds(1000);
+
+  /// The executor used to run the compiler.
+  std::shared_ptr<StackExecutor> stackExecutor_ =
+      newStackExecutor(kExecutorStackSize, kExecutorTimeout);
+
   /// Holds references to persistent BC providers for the lifetime of the
   /// Runtime. This is needed because the identifier table may contain pointers
   /// into bytecode, and so memory backing these must be preserved.

lib/BCGen/HBC/HBC.cpp

@@ -342,10 +342,9 @@ std::pair<bool, llvh::StringRef> compileLazyFunction(
     return {false, *errMsgOpt};
   }
 
-  // Run in a new stack to prevent stack overflow when deep inside JS execution.
+  // Use this callback-style API to reduce conflicts with stable for now.
   LazyCompilationThreadData data{provider, funcID};
-  executeInStack(
-      provider->getStackExecutor(), &data, compileLazyFunctionWorker);
+  compileLazyFunctionWorker(&data);
 
   if (data.success) {
     return std::make_pair(true, llvh::StringRef{});
@@ -406,9 +405,9 @@ std::pair<std::unique_ptr<BCProviderFromSrc>, std::string> compileEvalModule(
     hbc::BCProviderFromSrc *provider,
     uint32_t enclosingFuncID,
     const CompileFlags &compileFlags) {
-  // Run in a new stack to prevent stack overflow when deep inside JS execution.
+  // Use this callback-style API to reduce conflicts with stable for now.
   EvalThreadData data{std::move(src), provider, enclosingFuncID, compileFlags};
-  executeInStack(provider->getStackExecutor(), &data, compileEvalWorker);
+  compileEvalWorker(&data);
 
   return data.success
       ? std::make_pair(std::move(data.result), "")

lib/VM/CodeBlock.cpp

@@ -215,7 +215,13 @@ ExecutionStatus CodeBlock::lazyCompileImpl(Runtime &runtime) {
   assert(isLazy() && "Laziness has not been checked");
   auto *provider = runtimeModule_->getBytecode();
 
-  auto [success, errMsg] = hbc::compileLazyFunction(provider, functionID_);
+  bool success;
+  llvh::StringRef errMsg;
+  executeInStack(
+      runtime.getStackExecutor(), [&success, &errMsg, &provider, this]() {
+        std::tie(success, errMsg) =
+            hbc::compileLazyFunction(provider, functionID_);
+      });
   if (!success) {
     // Raise a SyntaxError to be consistent with eval().
     return runtime.raiseSyntaxError(llvh::StringRef{errMsg});

lib/VM/JSLib/eval.cpp

@@ -81,12 +81,18 @@ CallResult<HermesValue> evalInEnvironment(
             "Code compiled without support for direct eval");
       }
       // Local eval.
-      auto [newBCProviderFromSrc, error] = hbc::compileEvalModule(
-          std::move(buffer),
-          llvh::cast<hbc::BCProviderFromSrc>(
-              codeBlock->getRuntimeModule()->getBytecode()),
-          codeBlock->getFunctionID(),
-          compileFlags);
+      std::unique_ptr<hbc::BCProviderFromSrc> newBCProviderFromSrc;
+      std::string error;
+      executeInStack(
+          runtime.getStackExecutor(),
+          [&newBCProviderFromSrc, &error, &buffer, codeBlock, compileFlags]() {
+            std::tie(newBCProviderFromSrc, error) = hbc::compileEvalModule(
+                std::move(buffer),
+                llvh::cast<hbc::BCProviderFromSrc>(
+                    codeBlock->getRuntimeModule()->getBytecode()),
+                codeBlock->getFunctionID(),
+                compileFlags);
+          });
       if (!newBCProviderFromSrc) {
         return runtime.raiseSyntaxError(llvh::StringRef(error));
       }
@@ -95,15 +101,21 @@ CallResult<HermesValue> evalInEnvironment(
       // Global eval.
       // Creates a new AST Context and compiles everything independently:
       // new SemContext, new IR Module, everything.
-      auto bytecode_err = hbc::BCProviderFromSrc::createBCProviderFromSrc(
-          std::move(buffer),
-          "eval",
-          nullptr,
-          compileFlags,
-          "eval",
-          {},
-          nullptr,
-          runOptimizationPasses);
+      std::pair<std::unique_ptr<hbc::BCProviderFromSrc>, std::string>
+          bytecode_err;
+      executeInStack(
+          runtime.getStackExecutor(),
+          [&bytecode_err, &buffer, compileFlags, &runOptimizationPasses]() {
+            bytecode_err = hbc::BCProviderFromSrc::createBCProviderFromSrc(
+                std::move(buffer),
+                "eval",
+                nullptr,
+                compileFlags,
+                "eval",
+                {},
+                nullptr,
+                runOptimizationPasses);
+          });
       if (!bytecode_err.first) {
         return runtime.raiseSyntaxError(TwineChar16(bytecode_err.second));
       }