From 9e9dfd2ac9474e8fb10348db745b6530a61efb99 Mon Sep 17 00:00:00 2001 From: Marc Horowitz Date: Fri, 13 May 2016 17:15:06 -0700 Subject: [PATCH] Introduce non-copyable JSBigString for managing large strings efficiently Reviewed By: astreet Differential Revision: D3234836 fbshipit-source-id: 2b95b585dc1215988b88cf0d609c778a95b362a1 --- ReactCommon/bridge/Bridge.cpp | 28 +++++---- ReactCommon/bridge/Bridge.h | 8 +-- ReactCommon/bridge/Executor.h | 92 +++++++++++++++++++++++++++-- ReactCommon/bridge/Instance.cpp | 31 +++++----- ReactCommon/bridge/Instance.h | 8 +-- ReactCommon/bridge/JSCExecutor.cpp | 41 +++++++------ ReactCommon/bridge/JSCExecutor.h | 14 ++--- ReactCommon/bridge/JSCHelpers.cpp | 8 +++ ReactCommon/bridge/JSCHelpers.h | 5 ++ ReactCommon/bridge/JSCWebWorker.cpp | 4 +- ReactCommon/bridge/Platform.h | 3 +- ReactCommon/bridge/Value.h | 10 +++- 12 files changed, 180 insertions(+), 72 deletions(-) diff --git a/ReactCommon/bridge/Bridge.cpp b/ReactCommon/bridge/Bridge.cpp index 378ec629f49..9437657bd4e 100644 --- a/ReactCommon/bridge/Bridge.cpp +++ b/ReactCommon/bridge/Bridge.cpp @@ -37,22 +37,26 @@ Bridge::~Bridge() { CHECK(*m_destroyed) << "Bridge::destroy() must be called before deallocating the Bridge!"; } -void Bridge::loadApplicationScript(const std::string& script, const std::string& sourceURL) { +void Bridge::loadApplicationScript(std::unique_ptr script, + std::string sourceURL) { // TODO(t11144533): Add assert that we are on the correct thread - m_mainExecutor->loadApplicationScript(script, sourceURL); + m_mainExecutor->loadApplicationScript(std::move(script), std::move(sourceURL)); } void Bridge::loadApplicationUnbundle( std::unique_ptr unbundle, - const std::string& startupScript, - const std::string& startupScriptSourceURL) { + std::unique_ptr startupScript, + std::string startupScriptSourceURL) { runOnExecutorQueue( *m_mainExecutorToken, - [unbundle=folly::makeMoveWrapper(std::move(unbundle)), startupScript, startupScriptSourceURL] + [unbundle=folly::makeMoveWrapper(std::move(unbundle)), + startupScript=folly::makeMoveWrapper(std::move(startupScript)), + startupScriptSourceURL=std::move(startupScriptSourceURL)] (JSExecutor* executor) mutable { executor->setJSModulesUnbundle(unbundle.move()); - executor->loadApplicationScript(startupScript, startupScriptSourceURL); + executor->loadApplicationScript(std::move(*startupScript), + std::move(startupScriptSourceURL)); }); } @@ -108,10 +112,14 @@ void Bridge::invokeCallback(ExecutorToken executorToken, const double callbackId }); } -void Bridge::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { - runOnExecutorQueue(*m_mainExecutorToken, [=] (JSExecutor* executor) { - executor->setGlobalVariable(propName, jsonValue); - }); +void Bridge::setGlobalVariable(std::string propName, + std::unique_ptr jsonValue) { + runOnExecutorQueue( + *m_mainExecutorToken, + [propName=std::move(propName), jsonValue=folly::makeMoveWrapper(std::move(jsonValue))] + (JSExecutor* executor) mutable { + executor->setGlobalVariable(propName, jsonValue.move()); + }); } void* Bridge::getJavaScriptContext() { diff --git a/ReactCommon/bridge/Bridge.h b/ReactCommon/bridge/Bridge.h index 13bf3afaa9a..ebee2b8aafc 100644 --- a/ReactCommon/bridge/Bridge.h +++ b/ReactCommon/bridge/Bridge.h @@ -87,7 +87,7 @@ public: * contains code for all modules and a runtime that resolves and * executes modules. */ - void loadApplicationScript(const std::string& script, const std::string& sourceURL); + void loadApplicationScript(std::unique_ptr script, std::string sourceURL); /** * An "unbundle" is a backend that stores and injects JavaScript modules as @@ -98,9 +98,9 @@ public: */ void loadApplicationUnbundle( std::unique_ptr unbundle, - const std::string& startupCode, - const std::string& sourceURL); - void setGlobalVariable(const std::string& propName, const std::string& jsonValue); + std::unique_ptr startupCode, + std::string sourceURL); + void setGlobalVariable(std::string propName, std::unique_ptr jsonValue); void* getJavaScriptContext(); bool supportsProfiling(); void startProfiler(const std::string& title); diff --git a/ReactCommon/bridge/Executor.h b/ReactCommon/bridge/Executor.h index 9ea35c4fde2..d005701be93 100644 --- a/ReactCommon/bridge/Executor.h +++ b/ReactCommon/bridge/Executor.h @@ -29,14 +29,95 @@ public: virtual ~JSExecutorFactory() {}; }; +// JSExecutor functions sometimes take large strings, on the order of +// megabytes. Copying these can be expensive. Introducing a +// move-only, non-CopyConstructible type will let the compiler ensure +// that no copies occur. folly::MoveWrapper should be used when a +// large string needs to be curried into a std::function<>, which must +// by CopyConstructible. + +class JSBigString { +public: + JSBigString() = default; + + // Not copyable + JSBigString(const JSBigString&) = delete; + JSBigString& operator=(const JSBigString&) = delete; + + virtual ~JSBigString() {} + + virtual bool isAscii() const = 0; + virtual const char* c_str() const = 0; + virtual size_t size() const = 0; +}; + +// Concrete JSBigString implementation which holds a std::string +// instance. +class JSBigStdString : public JSBigString { +public: + JSBigStdString(std::string str, bool isAscii=false) + : m_isAscii(isAscii) + , m_str(std::move(str)) {} + + bool isAscii() const override { + return m_isAscii; + } + + const char* c_str() const override { + return m_str.c_str(); + } + + size_t size() const override { + return m_str.size(); + } + +private: + bool m_isAscii; + std::string m_str; +}; + +// Concrete JSBigString implementation which holds a heap-allocated +// buffer, and provides an accessor for writing to it. This can be +// used to construct a JSBigString in place, such as by reading from a +// file. +class JSBigBufferString : public facebook::react::JSBigString { +public: + JSBigBufferString(size_t size) + : m_data(new char[size]) + , m_size(size) {} + + ~JSBigBufferString() { + delete[] m_data; + } + + bool isAscii() const override { + return true; + } + + const char* c_str() const override { + return m_data; + } + + size_t size() const override { + return m_size; + } + + char* data() { + return m_data; + } + +private: + char* m_data; + size_t m_size; +}; + class JSExecutor { public: /** * Execute an application script bundle in the JS context. */ - virtual void loadApplicationScript( - const std::string& script, - const std::string& sourceURL) = 0; + virtual void loadApplicationScript(std::unique_ptr script, + std::string sourceURL) = 0; /** * Add an application "unbundle" file @@ -58,9 +139,8 @@ public: */ virtual void invokeCallback(const double callbackId, const folly::dynamic& arguments) = 0; - virtual void setGlobalVariable( - const std::string& propName, - const std::string& jsonValue) = 0; + virtual void setGlobalVariable(std::string propName, + std::unique_ptr jsonValue) = 0; virtual void* getJavaScriptContext() { return nullptr; }; diff --git a/ReactCommon/bridge/Instance.cpp b/ReactCommon/bridge/Instance.cpp index 509979ba0e5..a5b435e15bd 100644 --- a/ReactCommon/bridge/Instance.cpp +++ b/ReactCommon/bridge/Instance.cpp @@ -90,22 +90,22 @@ void Instance::initializeBridge( SystraceSection t("setGlobalVariable"); setGlobalVariable( "__fbBatchedBridgeConfig", - folly::toJson(config)); + folly::make_unique(folly::toJson(config))); } -void Instance::loadScriptFromString(const std::string& string, - const std::string& sourceURL) { +void Instance::loadScriptFromString(std::unique_ptr string, + std::string sourceURL) { callback_->incrementPendingJSCalls(); SystraceSection s("reactbridge_xplat_loadScriptFromString", "sourceURL", sourceURL); // TODO mhorowitz: ReactMarker around loadApplicationScript - bridge_->loadApplicationScript(string, sourceURL); + bridge_->loadApplicationScript(std::move(string), std::move(sourceURL)); } void Instance::loadScriptFromFile(const std::string& filename, const std::string& sourceURL) { // TODO mhorowitz: ReactMarker around file read - std::string script; + std::unique_ptr buf; { SystraceSection s("reactbridge_xplat_loadScriptFromFile", "fileName", filename); @@ -115,23 +115,22 @@ void Instance::loadScriptFromFile(const std::string& filename, LOG(ERROR) << "Unable to load script from file" << filename; } else { jsfile.seekg(0, std::ios::end); - script.reserve(jsfile.tellg()); + buf.reset(new JSBigBufferString(jsfile.tellg())); jsfile.seekg(0, std::ios::beg); - script.assign( - std::istreambuf_iterator(jsfile), - std::istreambuf_iterator()); + jsfile.read(buf->data(), buf->size()); } } - loadScriptFromString(script, sourceURL); + loadScriptFromString(std::move(buf), sourceURL); } void Instance::loadUnbundle(std::unique_ptr unbundle, - const std::string& startupScript, - const std::string& startupScriptSourceURL) { + std::unique_ptr startupScript, + std::string startupScriptSourceURL) { callback_->incrementPendingJSCalls(); SystraceSection s("reactbridge_xplat_setJSModulesUnbundle"); - bridge_->loadApplicationUnbundle(std::move(unbundle), startupScript, startupScriptSourceURL); + bridge_->loadApplicationUnbundle(std::move(unbundle), std::move(startupScript), + std::move(startupScriptSourceURL)); } bool Instance::supportsProfiling() { @@ -146,9 +145,9 @@ void Instance::stopProfiler(const std::string& title, const std::string& filenam return bridge_->stopProfiler(title, filename); } -void Instance::setGlobalVariable(const std::string& propName, - const std::string& jsonValue) { - bridge_->setGlobalVariable(propName, jsonValue); +void Instance::setGlobalVariable(std::string propName, + std::unique_ptr jsonValue) { + bridge_->setGlobalVariable(std::move(propName), std::move(jsonValue)); } void Instance::callJSFunction(ExecutorToken token, const std::string& module, const std::string& method, diff --git a/ReactCommon/bridge/Instance.h b/ReactCommon/bridge/Instance.h index 5b0fa9d4638..53ec7ac1983 100644 --- a/ReactCommon/bridge/Instance.h +++ b/ReactCommon/bridge/Instance.h @@ -33,16 +33,16 @@ class Instance { std::shared_ptr jsQueue, std::unique_ptr nativeQueue, std::shared_ptr moduleRegistry); - void loadScriptFromString(const std::string& string, const std::string& sourceURL); + void loadScriptFromString(std::unique_ptr string, std::string sourceURL); void loadScriptFromFile(const std::string& filename, const std::string& sourceURL); void loadUnbundle( std::unique_ptr unbundle, - const std::string& startupScript, - const std::string& startupScriptSourceURL); + std::unique_ptr startupScript, + std::string startupScriptSourceURL); bool supportsProfiling(); void startProfiler(const std::string& title); void stopProfiler(const std::string& title, const std::string& filename); - void setGlobalVariable(const std::string& propName, const std::string& jsonValue); + void setGlobalVariable(std::string propName, std::unique_ptr jsonValue); void callJSFunction(ExecutorToken token, const std::string& module, const std::string& method, folly::dynamic&& params, const std::string& tracingName); void callJSCallback(ExecutorToken token, uint64_t callbackId, folly::dynamic&& params); diff --git a/ReactCommon/bridge/JSCExecutor.cpp b/ReactCommon/bridge/JSCExecutor.cpp index d715c7d6ad5..f7581bef397 100644 --- a/ReactCommon/bridge/JSCExecutor.cpp +++ b/ReactCommon/bridge/JSCExecutor.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -125,8 +126,8 @@ JSCExecutor::JSCExecutor( std::shared_ptr messageQueueThread, int workerId, JSCExecutor *owner, - const std::string& script, - const std::unordered_map& globalObjAsJSON, + std::string scriptURL, + std::unordered_map globalObjAsJSON, const folly::dynamic& jscConfig) : m_bridge(bridge), m_workerId(workerId), @@ -136,29 +137,32 @@ JSCExecutor::JSCExecutor( m_jscConfig(jscConfig) { // We post initOnJSVMThread here so that the owner doesn't have to wait for // initialization on its own thread - m_messageQueueThread->runOnQueue([this, script, globalObjAsJSON] () { + m_messageQueueThread->runOnQueue([this, scriptURL, + globalObjAsJSON=std::move(globalObjAsJSON)] () { initOnJSVMThread(); installNativeHook<&JSCExecutor::nativePostMessage>("postMessage"); for (auto& it : globalObjAsJSON) { - setGlobalVariable(it.first, it.second); + setGlobalVariable(std::move(it.first), + folly::make_unique(std::move(it.second))); } // Try to load the script from the network if script is a URL // NB: For security, this will only work in debug builds - std::string scriptSrc; - if (script.find("http://") == 0 || script.find("https://") == 0) { + std::unique_ptr script; + if (scriptURL.find("http://") == 0 || scriptURL.find("https://") == 0) { std::stringstream outfileBuilder; outfileBuilder << m_deviceCacheDir << "/workerScript" << m_workerId << ".js"; - scriptSrc = WebWorkerUtil::loadScriptFromNetworkSync(script, outfileBuilder.str()); + script = folly::make_unique( + WebWorkerUtil::loadScriptFromNetworkSync(scriptURL, outfileBuilder.str())); } else { // TODO(9604438): Protect against script does not exist - scriptSrc = WebWorkerUtil::loadScriptFromAssets(script); + script = WebWorkerUtil::loadScriptFromAssets(scriptURL); } // TODO(9994180): Throw on error - loadApplicationScript(scriptSrc, script); + loadApplicationScript(std::move(script), std::move(scriptURL)); }); } @@ -236,9 +240,7 @@ void JSCExecutor::terminateOnJSVMThread() { m_context = nullptr; } -void JSCExecutor::loadApplicationScript( - const std::string& script, - const std::string& sourceURL) { +void JSCExecutor::loadApplicationScript(std::unique_ptr script, std::string sourceURL) { SystraceSection s("JSCExecutor::loadApplicationScript", "sourceURL", sourceURL); @@ -249,7 +251,7 @@ void JSCExecutor::loadApplicationScript( #endif ReactMarker::logMarker("loadApplicationScript_startStringConvert"); - String jsScript = String::createExpectingAscii(script); + String jsScript = jsStringFromBigString(*script); ReactMarker::logMarker("loadApplicationScript_endStringConvert"); #ifdef WITH_FBSYSTRACE @@ -296,13 +298,14 @@ void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& m_bridge->callNativeModules(*this, calls, true); } -void JSCExecutor::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { - // TODO mhorowitz: systrace this. +void JSCExecutor::setGlobalVariable(std::string propName, std::unique_ptr jsonValue) { + SystraceSection s("JSCExecutor.setGlobalVariable", + "propName", propName); auto globalObject = JSContextGetGlobalObject(m_context); String jsPropertyName(propName.c_str()); - String jsValueJSON(jsonValue.c_str()); + String jsValueJSON = jsStringFromBigString(*jsonValue); auto valueToInject = JSValueMakeFromJSONString(m_context, jsValueJSON); JSObjectSetProperty(m_context, globalObject, jsPropertyName, valueToInject, 0, NULL); @@ -364,7 +367,7 @@ void JSCExecutor::loadModule(uint32_t moduleId) { } int JSCExecutor::addWebWorker( - const std::string& script, + std::string scriptURL, JSValueRef workerRef, JSValueRef globalObjRef) { static std::atomic_int nextWorkerId(1); @@ -378,8 +381,8 @@ int JSCExecutor::addWebWorker( std::shared_ptr workerMQT = WebWorkerUtil::createWebWorkerThread(workerId, m_messageQueueThread.get()); std::unique_ptr worker; - workerMQT->runOnQueueSync([this, &worker, &workerMQT, &script, &globalObj, workerId, &workerJscConfig] () { - worker.reset(new JSCExecutor(m_bridge, workerMQT, workerId, this, script, + workerMQT->runOnQueueSync([this, &worker, &workerMQT, &scriptURL, &globalObj, workerId, &workerJscConfig] () { + worker.reset(new JSCExecutor(m_bridge, workerMQT, workerId, this, scriptURL, globalObj.toJSONMap(), workerJscConfig)); }); diff --git a/ReactCommon/bridge/JSCExecutor.h b/ReactCommon/bridge/JSCExecutor.h index 3b6c14f6c08..a0aac1c2774 100644 --- a/ReactCommon/bridge/JSCExecutor.h +++ b/ReactCommon/bridge/JSCExecutor.h @@ -55,8 +55,8 @@ public: ~JSCExecutor() override; virtual void loadApplicationScript( - const std::string& script, - const std::string& sourceURL) override; + std::unique_ptr script, + std::string sourceURL) override; virtual void setJSModulesUnbundle( std::unique_ptr unbundle) override; virtual void callFunction( @@ -67,8 +67,8 @@ public: const double callbackId, const folly::dynamic& arguments) override; virtual void setGlobalVariable( - const std::string& propName, - const std::string& jsonValue) override; + std::string propName, + std::unique_ptr jsonValue) override; virtual void* getJavaScriptContext() override; virtual bool supportsProfiling() override; virtual void startProfiler(const std::string &titleString) override; @@ -97,8 +97,8 @@ private: std::shared_ptr messageQueueThread, int workerId, JSCExecutor *owner, - const std::string& script, - const std::unordered_map& globalObjAsJSON, + std::string scriptURL, + std::unordered_map globalObjAsJSON, const folly::dynamic& jscConfig); void initOnJSVMThread(); @@ -107,7 +107,7 @@ private: void flushQueueImmediate(std::string queueJSON); void loadModule(uint32_t moduleId); - int addWebWorker(const std::string& script, JSValueRef workerRef, JSValueRef globalObjRef); + int addWebWorker(std::string scriptURL, JSValueRef workerRef, JSValueRef globalObjRef); void postMessageToOwnedWebWorker(int worker, JSValueRef message); void postMessageToOwner(JSValueRef result); void receiveMessageFromOwnedWebWorker(int workerId, const std::string& message); diff --git a/ReactCommon/bridge/JSCHelpers.cpp b/ReactCommon/bridge/JSCHelpers.cpp index 362ab61f1a2..f30d9dbfb42 100644 --- a/ReactCommon/bridge/JSCHelpers.cpp +++ b/ReactCommon/bridge/JSCHelpers.cpp @@ -32,6 +32,14 @@ JSValueRef makeJSCException( return JSValueToObject(ctx, exceptionString, NULL); } +String jsStringFromBigString(const JSBigString& bigstr) { + if (bigstr.isAscii()) { + return String::createExpectingAscii(bigstr.c_str(), bigstr.size()); + } else { + return String(bigstr.c_str()); + } +} + JSValueRef evaluateScript(JSContextRef context, JSStringRef script, JSStringRef source) { JSValueRef exn, result; result = JSEvaluateScript(context, script, NULL, source, 0, &exn); diff --git a/ReactCommon/bridge/JSCHelpers.h b/ReactCommon/bridge/JSCHelpers.h index b14fa0e303c..57b03e31e62 100644 --- a/ReactCommon/bridge/JSCHelpers.h +++ b/ReactCommon/bridge/JSCHelpers.h @@ -2,6 +2,9 @@ #pragma once +#include "Executor.h" +#include "Value.h" + #include #include #include @@ -40,6 +43,8 @@ JSValueRef makeJSCException( JSContextRef ctx, const char* exception_text); +String jsStringFromBigString(const JSBigString& bigstr); + JSValueRef evaluateScript( JSContextRef ctx, JSStringRef script, diff --git a/ReactCommon/bridge/JSCWebWorker.cpp b/ReactCommon/bridge/JSCWebWorker.cpp index d3870536902..7c56ba17b79 100644 --- a/ReactCommon/bridge/JSCWebWorker.cpp +++ b/ReactCommon/bridge/JSCWebWorker.cpp @@ -89,8 +89,8 @@ void JSCWebWorker::initJSVMAndLoadScript() { s_globalContextRefToJSCWebWorker[context_] = this; // TODO(9604438): Protect against script does not exist - std::string script = WebWorkerUtil::loadScriptFromAssets(scriptName_); - evaluateScript(context_, String(script.c_str()), String(scriptName_.c_str())); + std::unique_ptr script = WebWorkerUtil::loadScriptFromAssets(scriptName_); + evaluateScript(context_, jsStringFromBigString(*script), String(scriptName_.c_str())); installGlobalFunction(context_, "postMessage", nativePostMessage); } diff --git a/ReactCommon/bridge/Platform.h b/ReactCommon/bridge/Platform.h index e8095aa7115..81e4c82465b 100644 --- a/ReactCommon/bridge/Platform.h +++ b/ReactCommon/bridge/Platform.h @@ -8,6 +8,7 @@ #include +#include "Executor.h" #include "MessageQueueThread.h" namespace facebook { @@ -22,7 +23,7 @@ namespace WebWorkerUtil { using WebWorkerQueueFactory = std::function(int id, MessageQueueThread* ownerMessageQueue)>; extern WebWorkerQueueFactory createWebWorkerThread; -using LoadScriptFromAssets = std::function; +using LoadScriptFromAssets = std::function(const std::string& assetName)>; extern LoadScriptFromAssets loadScriptFromAssets; using LoadScriptFromNetworkSync = std::function; diff --git a/ReactCommon/bridge/Value.h b/ReactCommon/bridge/Value.h index e78be3807ed..7813f083968 100644 --- a/ReactCommon/bridge/Value.h +++ b/ReactCommon/bridge/Value.h @@ -75,15 +75,19 @@ public: return JSStringIsEqualToUTF8CString(m_string, utf8); } - static String createExpectingAscii(std::string const &utf8) { + static String createExpectingAscii(const char* utf8, size_t len) { #if WITH_FBJSCEXTENSIONS return String( - JSStringCreateWithUTF8CStringExpectAscii(utf8.c_str(), utf8.size()), true); + JSStringCreateWithUTF8CStringExpectAscii(utf8, len), true); #else - return String(JSStringCreateWithUTF8CString(utf8.c_str()), true); + return String(JSStringCreateWithUTF8CString(utf8), true); #endif } + static String createExpectingAscii(std::string const &utf8) { + return String::createExpectingAscii(utf8.c_str(), utf8.size()); + } + static String ref(JSStringRef string) { return String(string, false); }