From 6146eb39e9eb8e712169697eac75001c2aeab40d Mon Sep 17 00:00:00 2001 From: Neil Dhar Date: Thu, 19 Jan 2023 18:37:38 -0800 Subject: [PATCH] Remove register stack size override in hermes.cpp 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 --- API/hermes/hermes.cpp | 15 +-------------- public/hermes/Public/RuntimeConfig.h | 2 +- tools/hermes/hermes.cpp | 1 + tools/hvm/hvm.cpp | 1 + 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/API/hermes/hermes.cpp b/API/hermes/hermes.cpp index 5159b6c84e3..c9af3e1981a 100644 --- a/API/hermes/hermes.cpp +++ b/API/hermes/hermes.cpp @@ -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 @@ -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 diff --git a/public/hermes/Public/RuntimeConfig.h b/public/hermes/Public/RuntimeConfig.h index 2c4c83e1567..384111d7adc 100644 --- a/public/hermes/Public/RuntimeConfig.h +++ b/public/hermes/Public/RuntimeConfig.h @@ -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) \ diff --git a/tools/hermes/hermes.cpp b/tools/hermes/hermes.cpp index ddffc7677a8..cd026ce9474 100644 --- a/tools/hermes/hermes.cpp +++ b/tools/hermes/hermes.cpp @@ -112,6 +112,7 @@ static int executeHBCBytecodeFromCL( .withEnableHermesInternal(cl::EnableHermesInternal) .withEnableHermesInternalTestMethods( cl::EnableHermesInternalTestMethods) + .withMaxNumRegisters(1024 * 1024) .build(); options.basicBlockProfiling = cl::BasicBlockProfiling; diff --git a/tools/hvm/hvm.cpp b/tools/hvm/hvm.cpp index dcea74bf2f3..654a8a98067 100644 --- a/tools/hvm/hvm.cpp +++ b/tools/hvm/hvm.cpp @@ -126,6 +126,7 @@ int main(int argc, char **argv) { .withEnableHermesInternal(cl::EnableHermesInternal) .withEnableHermesInternalTestMethods( cl::EnableHermesInternalTestMethods) + .withMaxNumRegisters(1024 * 1024) .build(); options.stabilizeInstructionCount = cl::StableInstructionCount;