From db375ab449bb8d4370d108ce00ee5b226a78cecc Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Tue, 3 Sep 2024 16:01:30 -0700 Subject: [PATCH] Do not disable AllocInYoung for Hermes by default (#46314) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46314 In Bridgeless's version of the Hermes JVM init path, we defaulted an internal GC option to allocate memory in Hermes' OldGen, and revert that behaviour once an internal API was called which marked the app as loaded. This is an unsuitable default behaviour, since we can't rely on that internal API to be called on every launch. We're also moving to implicit performance instrumentation, which makes it harder to reliably call this API at the right time. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D61937427 fbshipit-source-id: 95e43fc093b56aee6362f43b6b0832d1f439fb3f --- .../AppDelegate/RCTRootViewFactory.mm | 3 +- .../ReactAndroid/api/ReactAndroid.api | 2 +- .../react/runtime/hermes/HermesInstance.kt | 15 +++++--- .../runtime/hermes/jni/JHermesInstance.cpp | 7 ++-- .../runtime/hermes/jni/JHermesInstance.h | 11 ++++-- .../tests/ReactInstanceIntegrationTest.cpp | 3 +- .../react/runtime/hermes/HermesInstance.cpp | 35 +++++++++++-------- .../react/runtime/hermes/HermesInstance.h | 5 +-- .../ios/ReactCommon/RCTHermesInstance.h | 12 ++++--- .../ios/ReactCommon/RCTHermesInstance.mm | 20 ++++++----- 10 files changed, 70 insertions(+), 43 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm index 377e5efeae8..e365cf76aba 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTRootViewFactory.mm @@ -283,7 +283,8 @@ static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabri - (std::shared_ptr)createJSRuntimeFactory { #if USE_HERMES - return std::make_shared(_reactNativeConfig, nullptr); + return std::make_shared( + _reactNativeConfig, nullptr, /* allocInOldGenBeforeTTI */ false); #else return std::make_shared(); #endif diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 7513accbd5f..9589c155177 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -3971,7 +3971,7 @@ public final class com/facebook/react/runtime/ReactSurfaceView : com/facebook/re public final class com/facebook/react/runtime/hermes/HermesInstance : com/facebook/react/runtime/JSRuntimeFactory { public static final field Companion Lcom/facebook/react/runtime/hermes/HermesInstance$Companion; public fun ()V - public fun (Lcom/facebook/react/fabric/ReactNativeConfig;)V + public fun (Lcom/facebook/react/fabric/ReactNativeConfig;Z)V } public final class com/facebook/react/runtime/hermes/HermesInstance$Companion { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/hermes/HermesInstance.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/hermes/HermesInstance.kt index 47efb3f2a79..7eb8544f82d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/hermes/HermesInstance.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/hermes/HermesInstance.kt @@ -13,13 +13,20 @@ import com.facebook.react.fabric.ReactNativeConfig import com.facebook.react.runtime.JSRuntimeFactory import com.facebook.soloader.SoLoader -public class HermesInstance(reactNativeConfig: ReactNativeConfig?) : - JSRuntimeFactory(initHybrid(reactNativeConfig as Any?)) { +public class HermesInstance( + reactNativeConfig: ReactNativeConfig?, + allocInOldGenBeforeTTI: Boolean +) : JSRuntimeFactory(initHybrid(reactNativeConfig as Any?, allocInOldGenBeforeTTI)) { - public constructor() : this(null) + public constructor() : this(null, false) public companion object { - @JvmStatic @DoNotStrip protected external fun initHybrid(reactNativeConfig: Any?): HybridData + @JvmStatic + @DoNotStrip + protected external fun initHybrid( + reactNativeConfig: Any?, + allocInOldGenBeforeTTI: Boolean + ): HybridData init { SoLoader.loadLibrary("hermesinstancejni") diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.cpp index deabc529969..b09f3b5a198 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.cpp @@ -14,12 +14,13 @@ namespace facebook::react { jni::local_ref JHermesInstance::initHybrid( jni::alias_ref /* unused */, - jni::alias_ref reactNativeConfig) { + jni::alias_ref reactNativeConfig, + bool allocInOldGenBeforeTTI) { std::shared_ptr config = reactNativeConfig != nullptr ? std::make_shared(reactNativeConfig) : nullptr; - return makeCxxInstance(config); + return makeCxxInstance(config, allocInOldGenBeforeTTI); } void JHermesInstance::registerNatives() { @@ -31,7 +32,7 @@ void JHermesInstance::registerNatives() { std::unique_ptr JHermesInstance::createJSRuntime( std::shared_ptr msgQueueThread) noexcept { return HermesInstance::createJSRuntime( - reactNativeConfig_, nullptr, msgQueueThread); + reactNativeConfig_, nullptr, msgQueueThread, allocInOldGenBeforeTTI_); } } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.h b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.h index 5ee3450442d..4d5c44ff9bf 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/hermes/jni/JHermesInstance.h @@ -28,12 +28,16 @@ class JHermesInstance static jni::local_ref initHybrid( jni::alias_ref /* unused */, - jni::alias_ref reactNativeConfig); + jni::alias_ref reactNativeConfig, + bool allocInOldGenBeforeTTI); static void registerNatives(); - JHermesInstance(std::shared_ptr reactNativeConfig) - : reactNativeConfig_(reactNativeConfig){}; + JHermesInstance( + std::shared_ptr reactNativeConfig, + bool allocInOldGenBeforeTTI) + : reactNativeConfig_(std::move(reactNativeConfig)), + allocInOldGenBeforeTTI_(allocInOldGenBeforeTTI){}; std::unique_ptr createJSRuntime( std::shared_ptr msgQueueThread) noexcept; @@ -44,6 +48,7 @@ class JHermesInstance friend HybridBase; std::shared_ptr reactNativeConfig_; + bool allocInOldGenBeforeTTI_; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp index 0173466df99..1f48befd367 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp @@ -51,7 +51,8 @@ void ReactInstanceIntegrationTest::SetUp() { auto jsRuntimeFactory = std::make_unique(); std::unique_ptr runtime_ = - jsRuntimeFactory->createJSRuntime(nullptr, nullptr, messageQueueThread); + jsRuntimeFactory->createJSRuntime( + nullptr, nullptr, messageQueueThread, false); jsi::Runtime* jsiRuntime = &runtime_->getRuntime(); // Error handler: diff --git a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp index 0a42996a11e..8e2e193a53c 100644 --- a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.cpp @@ -121,32 +121,37 @@ class HermesJSRuntime : public JSRuntime { std::unique_ptr HermesInstance::createJSRuntime( std::shared_ptr reactNativeConfig, - std::shared_ptr<::hermes::vm::CrashManager> cm, - std::shared_ptr msgQueueThread) noexcept { + std::shared_ptr<::hermes::vm::CrashManager> crashManager, + std::shared_ptr msgQueueThread, + bool allocInOldGenBeforeTTI) noexcept { assert(msgQueueThread != nullptr); + + auto gcConfig = ::hermes::vm::GCConfig::Builder() + // Default to 3GB + .withMaxHeapSize(3072 << 20) + .withName("RNBridgeless"); + + if (allocInOldGenBeforeTTI) { + // For the next two arguments: avoid GC before TTI + // by initializing the runtime to allocate directly + // in the old generation, but revert to normal + // operation when we reach the (first) TTI point. + gcConfig.withAllocInYoung(false).withRevertToYGAtTTI(true); + } + int64_t vmExperimentFlags = reactNativeConfig ? reactNativeConfig->getInt64("ios_hermes:vm_experiment_flags") : 0; ::hermes::vm::RuntimeConfig::Builder runtimeConfigBuilder = ::hermes::vm::RuntimeConfig::Builder() - .withGCConfig(::hermes::vm::GCConfig::Builder() - // Default to 3GB - .withMaxHeapSize(3072 << 20) - .withName("RNBridgeless") - // For the next two arguments: avoid GC before TTI - // by initializing the runtime to allocate directly - // in the old generation, but revert to normal - // operation when we reach the (first) TTI point. - .withAllocInYoung(false) - .withRevertToYGAtTTI(true) - .build()) + .withGCConfig(gcConfig.build()) .withEnableSampleProfiling(true) .withMicrotaskQueue(ReactNativeFeatureFlags::enableMicrotasks()) .withVMExperimentFlags(vmExperimentFlags); - if (cm) { - runtimeConfigBuilder.withCrashMgr(cm); + if (crashManager) { + runtimeConfigBuilder.withCrashMgr(crashManager); } std::unique_ptr hermesRuntime = diff --git a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.h b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.h index 71b08d2fd5b..5a97875d1da 100644 --- a/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/hermes/HermesInstance.h @@ -19,8 +19,9 @@ class HermesInstance { public: static std::unique_ptr createJSRuntime( std::shared_ptr reactNativeConfig, - std::shared_ptr<::hermes::vm::CrashManager> cm, - std::shared_ptr msgQueueThread) noexcept; + std::shared_ptr<::hermes::vm::CrashManager> crashManager, + std::shared_ptr msgQueueThread, + bool allocInOldGenBeforeTTI) noexcept; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.h b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.h index 533aa82b924..ea2565b3543 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.h @@ -13,8 +13,8 @@ #import #import -namespace facebook { -namespace react { +namespace facebook::react { + using CrashManagerProvider = std::function()>; @@ -24,7 +24,8 @@ class RCTHermesInstance : public JSRuntimeFactory { RCTHermesInstance(); RCTHermesInstance( std::shared_ptr reactNativeConfig, - CrashManagerProvider crashManagerProvider); + CrashManagerProvider crashManagerProvider, + bool allocInOldGenBeforeTTI); std::unique_ptr createJSRuntime( std::shared_ptr msgQueueThread) noexcept override; @@ -35,6 +36,7 @@ class RCTHermesInstance : public JSRuntimeFactory { std::shared_ptr _reactNativeConfig; CrashManagerProvider _crashManagerProvider; std::unique_ptr _hermesInstance; + bool _allocInOldGenBeforeTTI; }; -} // namespace react -} // namespace facebook + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.mm index c3e4951ded6..bd3ea5c7a97 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTHermesInstance.mm @@ -7,16 +7,18 @@ #import "RCTHermesInstance.h" -namespace facebook { -namespace react { -RCTHermesInstance::RCTHermesInstance() : RCTHermesInstance(nullptr, nullptr) {} +namespace facebook::react { + +RCTHermesInstance::RCTHermesInstance() : RCTHermesInstance(nullptr, nullptr, false) {} RCTHermesInstance::RCTHermesInstance( std::shared_ptr reactNativeConfig, - CrashManagerProvider crashManagerProvider) + CrashManagerProvider crashManagerProvider, + bool allocInOldGenBeforeTTI) : _reactNativeConfig(std::move(reactNativeConfig)), _crashManagerProvider(std::move(crashManagerProvider)), - _hermesInstance(std::make_unique()) + _hermesInstance(std::make_unique()), + _allocInOldGenBeforeTTI(allocInOldGenBeforeTTI) { } @@ -24,8 +26,10 @@ std::unique_ptr RCTHermesInstance::createJSRuntime( std::shared_ptr msgQueueThread) noexcept { return _hermesInstance->createJSRuntime( - _reactNativeConfig, _crashManagerProvider ? _crashManagerProvider() : nullptr, msgQueueThread); + _reactNativeConfig, + _crashManagerProvider ? _crashManagerProvider() : nullptr, + std::move(msgQueueThread), + _allocInOldGenBeforeTTI); } -} // namespace react -} // namespace facebook +} // namespace facebook::react