Skip to content

Commit

Permalink
Remove register stack size override in hermes.cpp
Browse files Browse the repository at this point in the history
Summary:
We are currently overriding any specified register stack size with a
size that is fairly low. This was added to allow the register stack to
be placed on a thread stack when StackRuntime is enabled, however, it
seems like that functionality was removed some time ago, so this limit
no longer serves a purpose.

Remove it so that sizes set through RuntimeConfig are actually
honoured. Set the default RuntimeConfig value to be roughly what the
old limit was (no need to suddenly increase the stack size if we
haven't had problems yet). Keeping the limit low helps us retain the
ability to place the register stack in StackRuntime again in the future.

Reviewed By: mhorowitz

Differential Revision: D42610775

fbshipit-source-id: 94db4e3eb1d6365749a6db8adfe73a5d298f127c
  • Loading branch information
neildhar authored and Riccardo Cipolleschi committed Mar 7, 2023
1 parent 62d58e5 commit 6146eb3
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 15 deletions.
15 changes: 1 addition & 14 deletions API/hermes/hermes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,6 @@ void hermesFatalErrorHandler(

namespace {

// Max size of the runtime's register stack.
// The runtime register stack needs to be small enough to be allocated on the
// native thread stack in Android (1MiB) and on MacOS's thread stack (512 KiB)
// Calculated by: (thread stack size - size of runtime -
// 8 memory pages for other stuff in the thread)
static constexpr unsigned kMaxNumRegisters =
(512 * 1024 - sizeof(::hermes::vm::Runtime) - 4096 * 8) /
sizeof(::hermes::vm::PinnedHermesValue);

void raw_ostream_append(llvh::raw_ostream &os) {}

template <typename Arg0, typename... Args>
Expand Down Expand Up @@ -163,11 +154,7 @@ class HermesRuntimeImpl final : public HermesRuntime,
HermesRuntimeImpl(const vm::RuntimeConfig &runtimeConfig)
: hermesValues_(runtimeConfig.getGCConfig().getOccupancyTarget()),
weakHermesValues_(runtimeConfig.getGCConfig().getOccupancyTarget()),
rt_(::hermes::vm::Runtime::create(
runtimeConfig.rebuild()
.withRegisterStack(nullptr)
.withMaxNumRegisters(kMaxNumRegisters)
.build())),
rt_(::hermes::vm::Runtime::create(runtimeConfig)),
runtime_(*rt_),
vmExperimentFlags_(runtimeConfig.getVMExperimentFlags()) {
#ifdef HERMES_ENABLE_DEBUGGER
Expand Down
2 changes: 1 addition & 1 deletion public/hermes/Public/RuntimeConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class PinnedHermesValue;
F(constexpr, PinnedHermesValue *, RegisterStack, nullptr) \
\
/* Register Stack Size */ \
F(constexpr, unsigned, MaxNumRegisters, 1024 * 1024) \
F(constexpr, unsigned, MaxNumRegisters, 64 * 1024) \
\
/* Whether or not the JIT is enabled */ \
F(constexpr, bool, EnableJIT, false) \
Expand Down
1 change: 1 addition & 0 deletions tools/hermes/hermes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ static int executeHBCBytecodeFromCL(
.withEnableHermesInternal(cl::EnableHermesInternal)
.withEnableHermesInternalTestMethods(
cl::EnableHermesInternalTestMethods)
.withMaxNumRegisters(1024 * 1024)
.build();

options.basicBlockProfiling = cl::BasicBlockProfiling;
Expand Down
1 change: 1 addition & 0 deletions tools/hvm/hvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ int main(int argc, char **argv) {
.withEnableHermesInternal(cl::EnableHermesInternal)
.withEnableHermesInternalTestMethods(
cl::EnableHermesInternalTestMethods)
.withMaxNumRegisters(1024 * 1024)
.build();

options.stabilizeInstructionCount = cl::StableInstructionCount;
Expand Down

0 comments on commit 6146eb3

Please sign in to comment.