Skip to content

Commit

Permalink
Split loadApplicationScript into initializeRuntime and loadBundle (#2…
Browse files Browse the repository at this point in the history
…7844)

Summary:
This is the first of three PRs related to enabling multi-bundle support in React Native. More details, motivation and reasoning behind it can be found in RFC [here](react-native-community/discussions-and-proposals#152).

Logic responsible for installing globals was pulled out from `loadApplicationScript` to `initializeRuntime` since it should be ran only once, what was left was renamed to `loadBundle`.

It's based on dratwas work from [here](https://github.com/callstack/react-native/tree/feat/multibundle/split-load-application), but applied to current `master` to avoid rebasing 3-months old branch and issues that come with that.

## Changelog

[Internal] [Changed] - split `loadApplicationScript` into `initializeRuntime` and `loadBundle` to enable multi-bundle support in the future
Pull Request resolved: #27844

Test Plan: Initialized new RN app with CLI, set RN to build from source and verified the still app builds and runs OK using code from this branch.

Reviewed By: rickhanlonii

Differential Revision: D19888605

Pulled By: ejanzer

fbshipit-source-id: 24ace48ffe8978796591fe7c6cf53a61b127cce6
  • Loading branch information
simka authored and facebook-github-bot committed Apr 2, 2020
1 parent eab7fc0 commit 6f627f6
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 48 deletions.
2 changes: 1 addition & 1 deletion React/CxxBridge/RCTCxxBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ - (void)executeApplicationScript:(NSData *)script url:(NSURL *)url async:(BOOL)a
} else if (reactInstance) {
reactInstance->loadScriptFromString(std::make_unique<NSDataBigString>(script), sourceUrlStr.UTF8String, !async);
} else {
std::string methodName = async ? "loadApplicationScript" : "loadApplicationScriptSync";
std::string methodName = async ? "loadBundle" : "loadBundleSync";
throw std::logic_error("Attempt to call " + methodName + ": on uninitialized bridge");
}
}];
Expand Down
7 changes: 6 additions & 1 deletion React/CxxBridge/RCTObjcExecutor.mm
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@
setGlobalVariable("__fbBatchedBridgeConfig", std::make_unique<JSBigStdString>(folly::toJson(config)));
}

void loadApplicationScript(std::unique_ptr<const JSBigString> script, std::string sourceURL) override
void initializeRuntime() override
{
// We do nothing here since initialization is done in the constructor
}

void loadBundle(std::unique_ptr<const JSBigString> script, std::string sourceURL) override
{
RCTProfileBeginFlowEvent();
[m_jse executeApplicationScript:[NSData dataWithBytes:script->c_str() length:script->size()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public ProxyExecutorException(Throwable cause) {
* @param sourceURL url or file location from which script content was loaded
*/
@DoNotStrip
void loadApplicationScript(String sourceURL) throws ProxyExecutorException;
void loadBundle(String sourceURL) throws ProxyExecutorException;

/**
* Execute javascript method within js context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void prepareJSRuntime(JSDebuggerCallback callback) {
}
}

public void loadApplicationScript(
public void loadBundle(
String sourceURL, HashMap<String, String> injectedObjects, JSDebuggerCallback callback) {
int requestID = mRequestID.getAndIncrement();
mCallbacks.put(requestID, callback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ public void close() {
}

@Override
public void loadApplicationScript(String sourceURL) throws JavaJSExecutor.ProxyExecutorException {
public void loadBundle(String sourceURL) throws JavaJSExecutor.ProxyExecutorException {
JSExecutorCallbackFuture callback = new JSExecutorCallbackFuture();
Assertions.assertNotNull(mWebSocketClient)
.loadApplicationScript(sourceURL, mInjectedObjects, callback);
Assertions.assertNotNull(mWebSocketClient).loadBundle(sourceURL, mInjectedObjects, callback);
try {
callback.get();
} catch (Throwable cause) {
Expand All @@ -178,7 +177,7 @@ public void loadApplicationScript(String sourceURL) throws JavaJSExecutor.ProxyE

@Override
public void setGlobalVariable(String propertyName, String jsonEncodedValue) {
// Store and use in the next loadApplicationScript() call.
// Store and use in the next loadBundle() call.
mInjectedObjects.put(propertyName, jsonEncodedValue);
}
}
15 changes: 8 additions & 7 deletions ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ ProxyExecutor::~ProxyExecutor() {
m_executor.reset();
}

void ProxyExecutor::loadApplicationScript(
std::unique_ptr<const JSBigString>,
std::string sourceURL) {
void ProxyExecutor::initializeRuntime() {
folly::dynamic nativeModuleConfig = folly::dynamic::array;

{
Expand All @@ -76,14 +74,17 @@ void ProxyExecutor::loadApplicationScript(
"__fbBatchedBridgeConfig",
std::make_unique<JSBigStdString>(folly::toJson(config)));
}
}

static auto loadApplicationScript =
jni::findClassStatic(EXECUTOR_BASECLASS)
->getMethod<void(jstring)>("loadApplicationScript");
void ProxyExecutor::loadBundle(
std::unique_ptr<const JSBigString>,
std::string sourceURL) {
static auto loadBundle = jni::findClassStatic(EXECUTOR_BASECLASS)
->getMethod<void(jstring)>("loadBundle");

// The proxy ignores the script data passed in.

loadApplicationScript(m_executor.get(), jni::make_jstring(sourceURL).get());
loadBundle(m_executor.get(), jni::make_jstring(sourceURL).get());
// We can get pending calls here to native but the queue will be drained when
// we launch the application.
}
Expand Down
3 changes: 2 additions & 1 deletion ReactAndroid/src/main/jni/react/jni/ProxyExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class ProxyExecutor : public JSExecutor {
jni::global_ref<jobject> &&executorInstance,
std::shared_ptr<ExecutorDelegate> delegate);
virtual ~ProxyExecutor() override;
virtual void loadApplicationScript(
virtual void initializeRuntime() override;
virtual void loadBundle(
std::unique_ptr<const JSBigString> script,
std::string sourceURL) override;
virtual void setBundleRegistry(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void test_prepareJSRuntime_ShouldSendCorrectMessage() throws Exception {
}

@Test
public void test_loadApplicationScript_ShouldSendCorrectMessage() throws Exception {
public void test_loadBundle_ShouldSendCorrectMessage() throws Exception {
final JSDebuggerWebSocketClient.JSDebuggerCallback cb =
PowerMockito.mock(JSDebuggerWebSocketClient.JSDebuggerCallback.class);

Expand All @@ -49,7 +49,7 @@ public void test_loadApplicationScript_ShouldSendCorrectMessage() throws Excepti
injectedObjects.put("key1", "value1");
injectedObjects.put("key2", "value2");

client.loadApplicationScript("http://localhost:8080/index.js", injectedObjects, cb);
client.loadBundle("http://localhost:8080/index.js", injectedObjects, cb);
PowerMockito.verifyPrivate(client)
.invoke(
"sendMessage",
Expand Down
24 changes: 13 additions & 11 deletions ReactCommon/cxxreact/Instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ void Instance::initializeBridge(
nativeToJsBridge_ = std::make_shared<NativeToJsBridge>(
jsef.get(), moduleRegistry_, jsQueue, callback_);

nativeToJsBridge_->initializeRuntime();

/**
* After NativeToJsBridge is created, the jsi::Runtime should exist.
* Also, the JS message queue thread exists. So, it's safe to
Expand All @@ -64,33 +66,33 @@ void Instance::initializeBridge(
CHECK(nativeToJsBridge_);
}

void Instance::loadApplication(
void Instance::loadBundle(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> string,
std::string sourceURL) {
callback_->incrementPendingJSCalls();
SystraceSection s("Instance::loadApplication", "sourceURL", sourceURL);
nativeToJsBridge_->loadApplication(
SystraceSection s("Instance::loadBundle", "sourceURL", sourceURL);
nativeToJsBridge_->loadBundle(
std::move(bundleRegistry), std::move(string), std::move(sourceURL));
}

void Instance::loadApplicationSync(
void Instance::loadBundleSync(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> string,
std::string sourceURL) {
std::unique_lock<std::mutex> lock(m_syncMutex);
m_syncCV.wait(lock, [this] { return m_syncReady; });

SystraceSection s("Instance::loadApplicationSync", "sourceURL", sourceURL);
nativeToJsBridge_->loadApplicationSync(
SystraceSection s("Instance::loadBundleSync", "sourceURL", sourceURL);
nativeToJsBridge_->loadBundleSync(
std::move(bundleRegistry), std::move(string), std::move(sourceURL));
}

void Instance::setSourceURL(std::string sourceURL) {
callback_->incrementPendingJSCalls();
SystraceSection s("Instance::setSourceURL", "sourceURL", sourceURL);

nativeToJsBridge_->loadApplication(nullptr, nullptr, std::move(sourceURL));
nativeToJsBridge_->loadBundle(nullptr, nullptr, std::move(sourceURL));
}

void Instance::loadScriptFromString(
Expand All @@ -99,9 +101,9 @@ void Instance::loadScriptFromString(
bool loadSynchronously) {
SystraceSection s("Instance::loadScriptFromString", "sourceURL", sourceURL);
if (loadSynchronously) {
loadApplicationSync(nullptr, std::move(string), std::move(sourceURL));
loadBundleSync(nullptr, std::move(string), std::move(sourceURL));
} else {
loadApplication(nullptr, std::move(string), std::move(sourceURL));
loadBundle(nullptr, std::move(string), std::move(sourceURL));
}
}

Expand Down Expand Up @@ -157,12 +159,12 @@ void Instance::loadRAMBundle(
std::string startupScriptSourceURL,
bool loadSynchronously) {
if (loadSynchronously) {
loadApplicationSync(
loadBundleSync(
std::move(bundleRegistry),
std::move(startupScript),
std::move(startupScriptSourceURL));
} else {
loadApplication(
loadBundle(
std::move(bundleRegistry),
std::move(startupScript),
std::move(startupScriptSourceURL));
Expand Down
6 changes: 4 additions & 2 deletions ReactCommon/cxxreact/Instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class RN_EXPORT Instance {
std::shared_ptr<MessageQueueThread> jsQueue,
std::shared_ptr<ModuleRegistry> moduleRegistry);

void initializeRuntime();

void setSourceURL(std::string sourceURL);

void loadScriptFromString(
Expand Down Expand Up @@ -130,11 +132,11 @@ class RN_EXPORT Instance {

private:
void callNativeModules(folly::dynamic &&calls, bool isEndOfBatch);
void loadApplication(
void loadBundle(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupScript,
std::string startupScriptSourceURL);
void loadApplicationSync(
void loadBundleSync(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupScript,
std::string startupScriptSourceURL);
Expand Down
7 changes: 6 additions & 1 deletion ReactCommon/cxxreact/JSExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,15 @@ class JSExecutorFactory {

class RN_EXPORT JSExecutor {
public:
/**
* Prepares the JS runtime for React Native by installing global variables.
* Called once before any JS is evaluated.
*/
virtual void initializeRuntime() = 0;
/**
* Execute an application script bundle in the JS context.
*/
virtual void loadApplicationScript(
virtual void loadBundle(
std::unique_ptr<const JSBigString> script,
std::string sourceURL) = 0;

Expand Down
13 changes: 9 additions & 4 deletions ReactCommon/cxxreact/NativeToJsBridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,12 @@ NativeToJsBridge::~NativeToJsBridge() {
<< "NativeToJsBridge::destroy() must be called before deallocating the NativeToJsBridge!";
}

void NativeToJsBridge::loadApplication(
void NativeToJsBridge::initializeRuntime() {
runOnExecutorQueue(
[](JSExecutor *executor) mutable { executor->initializeRuntime(); });
}

void NativeToJsBridge::loadBundle(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupScript,
std::string startupScriptSourceURL) {
Expand All @@ -129,7 +134,7 @@ void NativeToJsBridge::loadApplication(
executor->setBundleRegistry(std::move(bundleRegistry));
}
try {
executor->loadApplicationScript(
executor->loadBundle(
std::move(*startupScript), std::move(startupScriptSourceURL));
} catch (...) {
m_applicationScriptHasFailure = true;
Expand All @@ -138,15 +143,15 @@ void NativeToJsBridge::loadApplication(
});
}

void NativeToJsBridge::loadApplicationSync(
void NativeToJsBridge::loadBundleSync(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupScript,
std::string startupScriptSourceURL) {
if (bundleRegistry) {
m_executor->setBundleRegistry(std::move(bundleRegistry));
}
try {
m_executor->loadApplicationScript(
m_executor->loadBundle(
std::move(startupScript), std::move(startupScriptSourceURL));
} catch (...) {
m_applicationScriptHasFailure = true;
Expand Down
11 changes: 8 additions & 3 deletions ReactCommon/cxxreact/NativeToJsBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class RAMBundleRegistry;
// executors and their threads. All functions here can be called from
// any thread.
//
// Except for loadApplicationScriptSync(), all void methods will queue
// Except for loadBundleSync(), all void methods will queue
// work to run on the jsQueue passed to the ctor, and return
// immediately.
class NativeToJsBridge {
Expand Down Expand Up @@ -63,16 +63,21 @@ class NativeToJsBridge {
*/
void invokeCallback(double callbackId, folly::dynamic &&args);

/**
* Sets global variables in the JS Context.
*/
void initializeRuntime();

/**
* Starts the JS application. If bundleRegistry is non-null, then it is
* used to fetch JavaScript modules as individual scripts.
* Otherwise, the script is assumed to include all the modules.
*/
void loadApplication(
void loadBundle(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupCode,
std::string sourceURL);
void loadApplicationSync(
void loadBundleSync(
std::unique_ptr<RAMBundleRegistry> bundleRegistry,
std::unique_ptr<const JSBigString> startupCode,
std::string sourceURL);
Expand Down
22 changes: 14 additions & 8 deletions ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,8 @@ JSIExecutor::JSIExecutor(
*runtime, "__jsiExecutorDescription", runtime->description());
}

void JSIExecutor::loadApplicationScript(
std::unique_ptr<const JSBigString> script,
std::string sourceURL) {
SystraceSection s("JSIExecutor::loadApplicationScript");

// TODO: check for and use precompiled HBC

void JSIExecutor::initializeRuntime() {
SystraceSection s("JSIExecutor::initializeRuntime");
runtime_->global().setProperty(
*runtime_,
"nativeModuleProxy",
Expand Down Expand Up @@ -134,6 +129,18 @@ void JSIExecutor::loadApplicationScript(
if (runtimeInstaller_) {
runtimeInstaller_(*runtime_);
}
bool hasLogger(ReactMarker::logTaggedMarker);
if (hasLogger) {
ReactMarker::logMarker(ReactMarker::CREATE_REACT_CONTEXT_STOP);
}
}

void JSIExecutor::loadBundle(
std::unique_ptr<const JSBigString> script,
std::string sourceURL) {
SystraceSection s("JSIExecutor::loadBundle");

// TODO: check for and use precompiled HBC

bool hasLogger(ReactMarker::logTaggedMarker);
std::string scriptName = simpleBasename(sourceURL);
Expand All @@ -145,7 +152,6 @@ void JSIExecutor::loadApplicationScript(
std::make_unique<BigStringBuffer>(std::move(script)), sourceURL);
flush();
if (hasLogger) {
ReactMarker::logMarker(ReactMarker::CREATE_REACT_CONTEXT_STOP);
ReactMarker::logTaggedMarker(
ReactMarker::RUN_JS_BUNDLE_STOP, scriptName.c_str());
}
Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/jsiexecutor/jsireact/JSIExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ class JSIExecutor : public JSExecutor {
std::shared_ptr<ExecutorDelegate> delegate,
const JSIScopedTimeoutInvoker &timeoutInvoker,
RuntimeInstaller runtimeInstaller);
void loadApplicationScript(
void initializeRuntime() override;
void loadBundle(
std::unique_ptr<const JSBigString> script,
std::string sourceURL) override;
void setBundleRegistry(std::unique_ptr<RAMBundleRegistry>) override;
Expand Down

0 comments on commit 6f627f6

Please sign in to comment.