From 03f2dffc1d0ef8b2360a6790ad425ce4013e4de3 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 2b19ab1af1c..a7ebfe754a7 100644 --- a/API/hermes/hermes.cpp +++ b/API/hermes/hermes.cpp @@ -110,15 +110,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 @@ -170,11 +161,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 d7e8434329a..a61a37d7f12 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 to allow eval and Function ctor */ \ F(constexpr, bool, EnableEval, true) \ diff --git a/tools/hermes/hermes.cpp b/tools/hermes/hermes.cpp index b1db21990eb..b483e5148d0 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;