Skip to content

Commit

Permalink
use the StackExecutor API for lazy compilation and debug eval
Browse files Browse the repository at this point in the history
Summary:
Use the new API. For now this provides no benefit, but allows us to add
a more efficient implementation later.

Note that `eval()` still does not use any of these APIs and is thus
subject to stack overflow. The main challenge is the sharing of the
executor.

Reviewed By: avp

Differential Revision: D66343421

fbshipit-source-id: b473529cead70a60f2ad3813be112c297889cdae
  • Loading branch information
Tzvetan Mikov authored and facebook-github-bot committed Nov 22, 2024
1 parent 2db6eea commit 51cd64a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 29 deletions.
9 changes: 5 additions & 4 deletions include/hermes/BCGen/HBC/BCProviderFromSrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include "hermes/BCGen/HBC/BCProvider.h"
#include "hermes/BCGen/HBC/Bytecode.h"
#include "hermes/Support/SerialExecutor.h"
#include "hermes/Support/StackExecutor.h"

#include "llvh/ADT/Optional.h"

Expand Down Expand Up @@ -109,7 +109,8 @@ class BCProviderFromSrc final : public BCProviderBase {
std::chrono::milliseconds(1000);

/// The executor used to run the compiler.
SerialExecutor serialExecutor_{kExecutorStackSize, kExecutorTimeout};
std::shared_ptr<StackExecutor> stackExecutor_ =
newStackExecutor(kExecutorStackSize, kExecutorTimeout);

/// The BytecodeModule that provides the bytecode data.
/// Placed below CompilationData to ensure its destruction before the
Expand Down Expand Up @@ -256,8 +257,8 @@ class BCProviderFromSrc final : public BCProviderBase {
sourceHash_ = hash;
};

SerialExecutor &getSerialExecutor() {
return serialExecutor_;
StackExecutor &getStackExecutor() {
return *stackExecutor_;
}

static bool classof(const BCProviderBase *provider) {
Expand Down
30 changes: 5 additions & 25 deletions lib/BCGen/HBC/HBC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

#include "llvh/Support/raw_ostream.h"

#include <future>

#define DEBUG_TYPE "hbc-backend"

namespace hermes {
Expand Down Expand Up @@ -67,21 +65,6 @@ std::unique_ptr<BytecodeModule> generateBytecodeModuleForEval(

namespace {

/// Execute a function in a SerialExecutor, blocking until the function
/// completes.
/// \param executor the executor to use.
/// \param f the function to execute.
/// \param data the data to pass to the function.
static void executeBlockingInSerialExecutor(
SerialExecutor &executor,
void (*f)(void *),
void *data) {
std::packaged_task<void()> task([f, data]() { f(data); });
auto future = task.get_future();
executor.add([&task]() { task(); });
future.wait();
}

/// Data for the compileLazyFunctionWorker.
class LazyCompilationThreadData {
public:
Expand Down Expand Up @@ -359,11 +342,10 @@ std::pair<bool, llvh::StringRef> compileLazyFunction(
return {false, *errMsgOpt};
}

// Run on a thread to prevent stack overflow if this is run from deep inside
// JS execution.
// Run in a new stack to prevent stack overflow when deep inside JS execution.
LazyCompilationThreadData data{provider, funcID};
executeBlockingInSerialExecutor(
provider->getSerialExecutor(), compileLazyFunctionWorker, &data);
executeInStack(
provider->getStackExecutor(), &data, compileLazyFunctionWorker);

if (data.success) {
return std::make_pair(true, llvh::StringRef{});
Expand Down Expand Up @@ -424,11 +406,9 @@ std::pair<std::unique_ptr<BCProviderFromSrc>, std::string> compileEvalModule(
hbc::BCProviderFromSrc *provider,
uint32_t enclosingFuncID,
const CompileFlags &compileFlags) {
// Run on a thread to prevent stack overflow if this is run from deep inside
// JS execution.
// Run in a new stack to prevent stack overflow when deep inside JS execution.
EvalThreadData data{std::move(src), provider, enclosingFuncID, compileFlags};
executeBlockingInSerialExecutor(
provider->getSerialExecutor(), compileEvalWorker, &data);
executeInStack(provider->getStackExecutor(), &data, compileEvalWorker);

return data.success
? std::make_pair(std::move(data.result), "")
Expand Down

0 comments on commit 51cd64a

Please sign in to comment.