From 6abc097bf32ade1438fcf6c33bf6cc8c192249f8 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 21 Mar 2023 09:52:50 -0700 Subject: [PATCH] Remove dev-mode Hermes bytecode experiment Summary: Changelog: [General][Removed] Remove experimental support for loading bytecode from Metro Removes the experimental bundling strategy that offloads Hermes bytecode compilation to the packager server. The React Native parts of this experiment were never part of the public API, and the Metro parts never fully shipped in open source. Followup from D43597007. Reviewed By: robhogan Differential Revision: D43604705 fbshipit-source-id: db3be553750ccbf286d876f75858299c5b750f19 --- BUCK | 5 +--- .../React/Base/RCTBundleURLProvider.mm | 10 ++------ .../React/Base/RCTJavaScriptLoader.h | 2 -- .../React/Base/RCTJavaScriptLoader.mm | 13 ++-------- .../React/CxxBridge/RCTCxxBridge.mm | 15 +----------- .../react-native/ReactAndroid/build.gradle | 1 - .../main/java/com/facebook/react/common/BUCK | 3 +-- .../react/common/build/ReactBuildConfig.java | 1 - .../react/devsupport/DevServerHelper.java | 10 ++------ .../jni/react/jni/CatalystInstanceImpl.cpp | 24 ------------------- .../ReactCommon/cxxreact/JSBundleType.cpp | 5 ---- .../ReactCommon/cxxreact/JSBundleType.h | 1 - packages/rn-tester/BUCK | 5 +--- .../RCTBundleURLProviderTests.m | 20 ---------------- tools/build_defs/oss/rn_defs.bzl | 2 -- 15 files changed, 10 insertions(+), 107 deletions(-) diff --git a/BUCK b/BUCK index 78b7b158cd7..0e0de2f5f92 100644 --- a/BUCK +++ b/BUCK @@ -12,7 +12,6 @@ load( "//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "APPLE", - "HERMES_BYTECODE_VERSION", "IOS", "RCT_IMAGE_DATA_DECODER_SOCKET", "RCT_IMAGE_URL_LOADER_SOCKET", @@ -429,9 +428,7 @@ rn_apple_xplat_cxx_library( "linux", ["-D PIC_MODIFIER=@PLT"], )], - preprocessor_flags = get_objc_arc_preprocessor_flags() + get_preprocessor_flags_for_build_mode() + [ - "-DHERMES_BYTECODE_VERSION={}".format(HERMES_BYTECODE_VERSION), - ] + rn_extra_build_flags(), + preprocessor_flags = get_objc_arc_preprocessor_flags() + get_preprocessor_flags_for_build_mode() + rn_extra_build_flags(), visibility = [ "//fbobjc/Apps/Internal/SparkLabs/...", "//fbobjc/Apps/Internal/Venice/...", diff --git a/packages/react-native/React/Base/RCTBundleURLProvider.mm b/packages/react-native/React/Base/RCTBundleURLProvider.mm index 33a9ae8ae9c..d8ec3bcfa1e 100644 --- a/packages/react-native/React/Base/RCTBundleURLProvider.mm +++ b/packages/react-native/React/Base/RCTBundleURLProvider.mm @@ -265,19 +265,13 @@ static NSURL *serverRootWithHostPort(NSString *hostPort, NSString *scheme) runModule:(BOOL)runModule { NSString *path = [NSString stringWithFormat:@"/%@.bundle", bundleRoot]; -#ifdef HERMES_BYTECODE_VERSION - NSString *runtimeBytecodeVersion = [NSString stringWithFormat:@"&runtimeBytecodeVersion=%u", HERMES_BYTECODE_VERSION]; -#else - NSString *runtimeBytecodeVersion = @""; -#endif // When we support only iOS 8 and above, use queryItems for a better API. - NSString *query = [NSString stringWithFormat:@"platform=ios&dev=%@&minify=%@&modulesOnly=%@&runModule=%@%@", + NSString *query = [NSString stringWithFormat:@"platform=ios&dev=%@&minify=%@&modulesOnly=%@&runModule=%@", enableDev ? @"true" : @"false", enableMinification ? @"true" : @"false", modulesOnly ? @"true" : @"false", - runModule ? @"true" : @"false", - runtimeBytecodeVersion]; + runModule ? @"true" : @"false"]; NSString *bundleID = [[NSBundle mainBundle] objectForInfoDictionaryKey:(NSString *)kCFBundleIdentifierKey]; if (bundleID) { diff --git a/packages/react-native/React/Base/RCTJavaScriptLoader.h b/packages/react-native/React/Base/RCTJavaScriptLoader.h index a8a95f72639..9893e693166 100755 --- a/packages/react-native/React/Base/RCTJavaScriptLoader.h +++ b/packages/react-native/React/Base/RCTJavaScriptLoader.h @@ -11,8 +11,6 @@ extern NSString *const RCTJavaScriptLoaderErrorDomain; -extern const uint32_t RCT_BYTECODE_ALIGNMENT; - NS_ENUM(NSInteger){ RCTJavaScriptLoaderErrorNoScriptURL = 1, RCTJavaScriptLoaderErrorFailedOpeningFile = 2, diff --git a/packages/react-native/React/Base/RCTJavaScriptLoader.mm b/packages/react-native/React/Base/RCTJavaScriptLoader.mm index 133851a1d7f..8839095e6a4 100755 --- a/packages/react-native/React/Base/RCTJavaScriptLoader.mm +++ b/packages/react-native/React/Base/RCTJavaScriptLoader.mm @@ -19,8 +19,6 @@ NSString *const RCTJavaScriptLoaderErrorDomain = @"RCTJavaScriptLoaderErrorDomain"; -const uint32_t RCT_BYTECODE_ALIGNMENT = 4; - @interface RCTSource () { @public NSURL *_url; @@ -41,12 +39,7 @@ static RCTSource *RCTSourceCreate(NSURL *url, NSData *data, int64_t length) NS_R RCTSource *source = [RCTSource new]; source->_url = url; - // Multipart responses may give us an unaligned view into the buffer. This ensures memory is aligned. - if (parseTypeFromHeader(header) == ScriptTag::MetroHBCBundle && ((long)[data bytes] % RCT_BYTECODE_ALIGNMENT)) { - source->_data = [[NSData alloc] initWithData:data]; - } else { - source->_data = data; - } + source->_data = data; source->_length = length; source->_filesChangedCount = RCTSourceFilesChangedCountNotBuiltByBundler; return source; @@ -171,7 +164,6 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init) facebook::react::ScriptTag tag = facebook::react::parseTypeFromHeader(header); switch (tag) { - case facebook::react::ScriptTag::MetroHBCBundle: case facebook::react::ScriptTag::RAMBundle: break; @@ -285,8 +277,7 @@ static void attemptAsynchronousLoadOfBundleAtURL( // Validate that the packager actually returned javascript. NSString *contentType = headers[@"Content-Type"]; NSString *mimeType = [[contentType componentsSeparatedByString:@";"] firstObject]; - if (![mimeType isEqualToString:@"application/javascript"] && ![mimeType isEqualToString:@"text/javascript"] && - ![mimeType isEqualToString:@"application/x-metro-bytecode-bundle"]) { + if (![mimeType isEqualToString:@"application/javascript"] && ![mimeType isEqualToString:@"text/javascript"]) { NSString *description; if ([mimeType isEqualToString:@"application/json"]) { NSError *parseError; diff --git a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm index fdb89650392..5cec2735d44 100644 --- a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm +++ b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm @@ -1494,11 +1494,6 @@ RCT_NOT_IMPLEMENTED(-(instancetype)initWithBundleURL [self executeApplicationScript:script url:url async:NO]; } -static uint32_t RCTReadUInt32LE(NSData *script, uint32_t offset) -{ - return [script length] < offset + 4 ? 0 : CFSwapInt32LittleToHost(*(((uint32_t *)[script bytes]) + offset / 4)); -} - - (void)executeApplicationScript:(NSData *)script url:(NSURL *)url async:(BOOL)async { [self _tryAndHandleError:^{ @@ -1514,15 +1509,7 @@ static uint32_t RCTReadUInt32LE(NSData *script, uint32_t offset) // hold a local reference to reactInstance in case a parallel thread // resets it between null check and usage auto reactInstance = self->_reactInstance; - if (reactInstance && scriptType == ScriptTag::MetroHBCBundle) { - uint32_t offset = 8; - while (offset < script.length) { - uint32_t fileLength = RCTReadUInt32LE(script, offset); - NSData *unit = [script subdataWithRange:NSMakeRange(offset + 4, fileLength)]; - reactInstance->loadScriptFromString(std::make_unique(unit), sourceUrlStr.UTF8String, false); - offset += ((fileLength + RCT_BYTECODE_ALIGNMENT - 1) & ~(RCT_BYTECODE_ALIGNMENT - 1)) + 4; - } - } else if (scriptType == ScriptTag::RAMBundle) { + if (scriptType == ScriptTag::RAMBundle) { [self->_performanceLogger markStartForTag:RCTPLRAMBundleLoad]; auto ramBundle = std::make_unique(sourceUrlStr.UTF8String); std::unique_ptr scriptStr = ramBundle->getStartupCode(); diff --git a/packages/react-native/ReactAndroid/build.gradle b/packages/react-native/ReactAndroid/build.gradle index f473ba011bd..6f5cff28852 100644 --- a/packages/react-native/ReactAndroid/build.gradle +++ b/packages/react-native/ReactAndroid/build.gradle @@ -454,7 +454,6 @@ android { buildConfigField("boolean", "IS_INTERNAL_BUILD", "false") buildConfigField("int", "EXOPACKAGE_FLAGS", "0") - buildConfigField("int", "HERMES_BYTECODE_VERSION", "0") resValue "integer", "react_native_dev_server_port", reactNativeDevServerPort() resValue "integer", "react_native_inspector_proxy_port", reactNativeInspectorProxyPort() diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/BUCK b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/BUCK index f3fc61e4e0b..211017b73ea 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/BUCK +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/BUCK @@ -1,4 +1,4 @@ -load("//tools/build_defs/oss:rn_defs.bzl", "HERMES_BYTECODE_VERSION", "react_native_dep", "rn_android_build_config", "rn_android_library") +load("//tools/build_defs/oss:rn_defs.bzl", "react_native_dep", "rn_android_build_config", "rn_android_library") SUB_PROJECTS = [ "network/**/*", @@ -40,7 +40,6 @@ rn_android_build_config( package = "com.facebook.react", values = [ "boolean IS_INTERNAL_BUILD = true", - "int HERMES_BYTECODE_VERSION = {}".format(HERMES_BYTECODE_VERSION), ], visibility = [ "PUBLIC", diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/build/ReactBuildConfig.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/build/ReactBuildConfig.java index 70273030bbe..b34769a1932 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/build/ReactBuildConfig.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/build/ReactBuildConfig.java @@ -20,5 +20,4 @@ public class ReactBuildConfig { public static final boolean DEBUG = BuildConfig.DEBUG; public static final boolean IS_INTERNAL_BUILD = BuildConfig.IS_INTERNAL_BUILD; public static final int EXOPACKAGE_FLAGS = BuildConfig.EXOPACKAGE_FLAGS; - public static final int HERMES_BYTECODE_VERSION = BuildConfig.HERMES_BYTECODE_VERSION; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java index e16de601218..a28621f77cc 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java @@ -14,7 +14,6 @@ import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.ReactContext; import com.facebook.react.common.ReactConstants; -import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener; import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; import com.facebook.react.devsupport.interfaces.StackFrame; @@ -427,13 +426,9 @@ public class DevServerHelper { private String createBundleURL( String mainModuleID, BundleType type, String host, boolean modulesOnly, boolean runModule) { - String runtimeBytecodeVersion = - ReactBuildConfig.HERMES_BYTECODE_VERSION != 0 - ? "&runtimeBytecodeVersion=" + ReactBuildConfig.HERMES_BYTECODE_VERSION - : ""; return String.format( Locale.US, - "http://%s/%s.%s?platform=android&dev=%s&minify=%s&app=%s&modulesOnly=%s&runModule=%s%s", + "http://%s/%s.%s?platform=android&dev=%s&minify=%s&app=%s&modulesOnly=%s&runModule=%s", host, mainModuleID, type.typeID(), @@ -441,8 +436,7 @@ public class DevServerHelper { getJSMinifyMode(), mPackageName, modulesOnly ? "true" : "false", - runModule ? "true" : "false", - runtimeBytecodeVersion); + runModule ? "true" : "false"); } private String createBundleURL(String mainModuleID, BundleType type) { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index e44913e4fd0..e3345f038be 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -290,30 +290,6 @@ void CatalystInstanceImpl::jniLoadScriptFromFile( } switch (getScriptTagFromFile(fileName.c_str())) { - case ScriptTag::MetroHBCBundle: { - std::unique_ptr script; - RecoverableError::runRethrowingAsRecoverable( - [&fileName, &script]() { - script = JSBigFileString::fromPath(fileName); - }); - const char *buffer = script->c_str(); - uint32_t bufferLength = (uint32_t)script->size(); - uint32_t offset = 8; - while (offset < bufferLength) { - uint32_t segment = offset + 4; - uint32_t moduleLength = - bufferLength < segment ? 0 : *(((uint32_t *)buffer) + offset / 4); - - reactInstance->loadScriptFromString( - std::make_unique( - std::string(buffer + segment, buffer + moduleLength + segment)), - sourceURL, - false); - - offset += ((moduleLength + 3) & ~3) + 4; - } - break; - } case ScriptTag::RAMBundle: instance_->loadRAMBundleFromFile(fileName, sourceURL, loadSynchronously); break; diff --git a/packages/react-native/ReactCommon/cxxreact/JSBundleType.cpp b/packages/react-native/ReactCommon/cxxreact/JSBundleType.cpp index 036397c8113..8e6f17ab008 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSBundleType.cpp +++ b/packages/react-native/ReactCommon/cxxreact/JSBundleType.cpp @@ -11,7 +11,6 @@ namespace facebook { namespace react { static uint32_t constexpr RAMBundleMagicNumber = 0xFB0BD1E5; -static uint32_t constexpr MetroHBCBundleMagicNumber = 0xFFE7C3C3; // "Hermes" in ancient Greek encoded in UTF-16BE and truncated to 8 bytes. static uint64_t constexpr HermesBCBundleMagicNumber = 0x1F1903C103BC1FC6; @@ -20,8 +19,6 @@ ScriptTag parseTypeFromHeader(const BundleHeader &header) { switch (header.magic32.value) { case RAMBundleMagicNumber: return ScriptTag::RAMBundle; - case MetroHBCBundleMagicNumber: - return ScriptTag::MetroHBCBundle; default: return ScriptTag::String; } @@ -33,8 +30,6 @@ const char *stringForScriptTag(const ScriptTag &tag) { return "String"; case ScriptTag::RAMBundle: return "RAM Bundle"; - case ScriptTag::MetroHBCBundle: - return "Metro Hermes Bytecode Bundle"; } return ""; } diff --git a/packages/react-native/ReactCommon/cxxreact/JSBundleType.h b/packages/react-native/ReactCommon/cxxreact/JSBundleType.h index 0dee53c0be9..7a966a94b7e 100644 --- a/packages/react-native/ReactCommon/cxxreact/JSBundleType.h +++ b/packages/react-native/ReactCommon/cxxreact/JSBundleType.h @@ -27,7 +27,6 @@ namespace react { enum struct ScriptTag { String = 0, RAMBundle, - MetroHBCBundle, }; /** diff --git a/packages/rn-tester/BUCK b/packages/rn-tester/BUCK index 284c184fb35..ef3f2549ea0 100644 --- a/packages/rn-tester/BUCK +++ b/packages/rn-tester/BUCK @@ -1,5 +1,4 @@ load("@fbsource//tools/build_defs:glob_defs.bzl", "subdir_glob") -load("@fbsource//xplat/hermes/defs:hermes.bzl", "HERMES_BYTECODE_VERSION") load("//tools/build_defs:fb_native_wrapper.bzl", "fb_native") load("//tools/build_defs:fb_xplat_platform_specific_rule.bzl", "fb_xplat_platform_specific_rule") load("//tools/build_defs:fb_xplat_resource.bzl", "fb_xplat_resource") @@ -138,9 +137,7 @@ fb_apple_test( "QuartzCore", "UIKit", ], - preprocessor_flags = get_objc_arc_preprocessor_flags() + [ - "-DHERMES_BYTECODE_VERSION={}".format(HERMES_BYTECODE_VERSION), - ] + get_preprocessor_flags_for_build_mode(), + preprocessor_flags = get_objc_arc_preprocessor_flags() + get_preprocessor_flags_for_build_mode(), visibility = [ "//fbobjc/Libraries/FBReactKit:workspace", ], diff --git a/packages/rn-tester/RNTesterUnitTests/RCTBundleURLProviderTests.m b/packages/rn-tester/RNTesterUnitTests/RCTBundleURLProviderTests.m index 89164bc0bb4..004da62a664 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTBundleURLProviderTests.m +++ b/packages/rn-tester/RNTesterUnitTests/RCTBundleURLProviderTests.m @@ -22,42 +22,22 @@ static NSURL *mainBundleURL() static NSURL *localhostBundleURL() { -#ifdef HERMES_BYTECODE_VERSION - return [NSURL - URLWithString: - [NSString - stringWithFormat: - @"http://localhost:8081/%@.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&runtimeBytecodeVersion=%u&app=com.apple.dt.xctest.tool", - testFile, - HERMES_BYTECODE_VERSION]]; -#else return [NSURL URLWithString: [NSString stringWithFormat: @"http://localhost:8081/%@.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool", testFile]]; -#endif } static NSURL *ipBundleURL() { -#ifdef HERMES_BYTECODE_VERSION - return [NSURL - URLWithString: - [NSString - stringWithFormat: - @"http://192.168.1.1:8081/%@.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&runtimeBytecodeVersion=%u&app=com.apple.dt.xctest.tool", - testFile, - HERMES_BYTECODE_VERSION]]; -#else return [NSURL URLWithString: [NSString stringWithFormat: @"http://192.168.1.1:8081/%@.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=com.apple.dt.xctest.tool", testFile]]; -#endif } @implementation NSBundle (RCTBundleURLProviderTests) diff --git a/tools/build_defs/oss/rn_defs.bzl b/tools/build_defs/oss/rn_defs.bzl index c1ad82e48bb..0813b09ccd5 100644 --- a/tools/build_defs/oss/rn_defs.bzl +++ b/tools/build_defs/oss/rn_defs.bzl @@ -428,8 +428,6 @@ def react_fabric_component_plugin_provider(name, native_class_func): def react_cxx_module_plugin_provider(name, function): return None -HERMES_BYTECODE_VERSION = -1 - RCT_IMAGE_DATA_DECODER_SOCKET = None RCT_IMAGE_URL_LOADER_SOCKET = None RCT_URL_REQUEST_HANDLER_SOCKET = None