perf: remove instantiations and cache runtime in isolate

vmutafov · vmutafov · commit f23c1bb5b61c · 2022-11-16T20:39:26.000+02:00
diff --git a/test-app/runtime/CMakeLists.txt b/test-app/runtime/CMakeLists.txt
@@ -59,8 +59,8 @@ include_directories(
 if (OPTIMIZED_BUILD OR OPTIMIZED_WITH_INSPECTOR_BUILD)
     set(CMAKE_CXX_FLAGS "${COMMON_CMAKE_ARGUMENTS} -O3 -fvisibility=hidden -ffunction-sections -fno-data-sections")
 else ()
-#    set(CMAKE_CXX_FLAGS "${COMMON_CMAKE_ARGUMENTS} -g")
-    set(CMAKE_CXX_FLAGS "${COMMON_CMAKE_ARGUMENTS} -O3 -fvisibility=hidden -ffunction-sections -fno-data-sections")
+    set(CMAKE_CXX_FLAGS "${COMMON_CMAKE_ARGUMENTS} -g")
+#    set(CMAKE_CXX_FLAGS "${COMMON_CMAKE_ARGUMENTS} -O3 -fvisibility=hidden -ffunction-sections -fno-data-sections")
 endif ()
 
 # Command info: https://cmake.org/cmake/help/v3.4/command/add_library.html
diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp
@@ -102,7 +102,7 @@ bool CallbackHandlers::RegisterInstance(Isolate *isolate, const Local<Object> &j
             instance = env.NewObject(generatedJavaClass, mi.mid);
         } else {
             // resolve arguments before passing them on to the constructor
-            JSToJavaConverter argConverter(argWrapper.args, mi.signature);
+            JSToJavaConverter argConverter(isolate, argWrapper.args, mi.signature);
             auto ctorArgs = argConverter.ToArgs();
 
             instance = env.NewObjectA(generatedJavaClass, mi.mid, ctorArgs);
@@ -209,6 +209,8 @@ void CallbackHandlers::CallJavaMethod(const Local<Object> &caller, const string
     auto retType = MethodReturnType::Unknown;
     MethodCache::CacheMethodInfo mi;
 
+    auto isolate = args.GetIsolate();
+
     if ((entry != nullptr) && entry->isResolved) {
         isStatic = entry->isStatic;
 
@@ -320,27 +322,20 @@ void CallbackHandlers::CallJavaMethod(const Local<Object> &caller, const string
                     methodName.c_str());
     }
 
-    JSToJavaConverter *argConverter;
-
-    if (entry != nullptr && entry->isExtensionFunction) {
-        argConverter = new JSToJavaConverter(args, *sig, entry, caller);
-    } else {
-        argConverter = new JSToJavaConverter(args, *sig, entry);
-    }
+    JSToJavaConverter argConverter = (entry != nullptr && entry->isExtensionFunction)
+                                     ? JSToJavaConverter(isolate, args, *sig, entry, caller)
+                                     : JSToJavaConverter(isolate, args, *sig, entry);
 
 //    if (!argConverter->IsValid()) {
 //        JSToJavaConverter::Error err = argConverter->GetError();
 //        throw NativeScriptException(err.msg);
 //    }
 
-    auto isolate = args.GetIsolate();
-
     JniLocalRef callerJavaObject;
 
-    jvalue *javaArgs = argConverter->ToArgs();
-
+    jvalue *javaArgs = argConverter.ToArgs();
 
-    auto runtime = Runtime::GetRuntime(isolate);
+    auto runtime = Runtime::GetRuntimeFromIsolateData(isolate);
     auto objectManager = runtime->GetObjectManager();
 
     if (!isStatic) {
@@ -542,8 +537,6 @@ void CallbackHandlers::CallJavaMethod(const Local<Object> &caller, const string
 //    if ((++adjustMemCount % 2) == 0) {
 //        AdjustAmountOfExternalAllocatedMemory(env, isolate);
 //    }
-
-    delete argConverter;
 }
 
 void CallbackHandlers::AdjustAmountOfExternalAllocatedMemory(JEnv &env, Isolate *isolate) {
diff --git a/test-app/runtime/src/main/cpp/MetadataNode.cpp b/test-app/runtime/src/main/cpp/MetadataNode.cpp
@@ -922,7 +922,6 @@ Local<FunctionTemplate> MetadataNode::GetConstructorFunctionTemplate(Isolate* is
 
     auto node = GetOrCreateInternal(treeNode);
 
-
     JEnv env;
     // if we already have an exception (which will be rethrown later)
     // then we don't want to ignore the next exception
@@ -2172,6 +2171,7 @@ void MetadataNode::SymbolHasInstanceCallback(const v8::FunctionCallbackInfo<v8::
     auto isolate = info.GetIsolate();
     auto context = isolate->GetCurrentContext();
     auto runtime = Runtime::GetRuntime(isolate);
+
     auto objectManager = runtime->GetObjectManager();
     auto obj = objectManager->GetJavaObjectByJsObject(arg->ToObject(context).ToLocalChecked());
 
diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp
@@ -127,10 +127,7 @@ Runtime* Runtime::GetRuntime(int runtimeId) {
 }
 
 Runtime* Runtime::GetRuntime(v8::Isolate* isolate) {
-    auto itFound = s_isolate2RuntimesCache.find(isolate);
-    auto runtime = (itFound != s_isolate2RuntimesCache.end())
-                   ? itFound->second
-                   : nullptr;
+    auto runtime = s_isolate2RuntimesCache.at(isolate);
 
     if (runtime == nullptr) {
         stringstream ss;
@@ -141,19 +138,21 @@ Runtime* Runtime::GetRuntime(v8::Isolate* isolate) {
     return runtime;
 }
 
-ObjectManager* Runtime::GetObjectManager(v8::Isolate* isolate) {
-    auto itFound = s_isolate2RuntimesCache.find(isolate);
-    auto runtime = (itFound != s_isolate2RuntimesCache.end())
-                   ? itFound->second
-                   : nullptr;
+Runtime* Runtime::GetRuntimeFromIsolateData(v8::Isolate* isolate) {
+    void* maybeRuntime = isolate->GetData((uint32_t)Runtime::IsolateData::RUNTIME);
+    auto runtime = static_cast<Runtime*>(maybeRuntime);
 
     if (runtime == nullptr) {
         stringstream ss;
         ss << "Cannot find runtime for isolate: " << isolate;
         throw NativeScriptException(ss.str());
     }
 
-    return runtime->GetObjectManager();
+    return runtime;
+}
+
+ObjectManager* Runtime::GetObjectManager(v8::Isolate* isolate) {
+    return GetRuntime(isolate)->GetObjectManager();
 }
 
 Isolate* Runtime::GetIsolate() const {
@@ -606,7 +605,7 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, const string& native
     auto isolate = Isolate::New(create_params);
     isolateFrame.log("Isolate.New");
 
-    s_isolate2RuntimesCache.insert(make_pair(isolate, this));
+    s_isolate2RuntimesCache[isolate] = this;
     v8::Locker locker(isolate);
     Isolate::Scope isolate_scope(isolate);
     HandleScope handleScope(isolate);
@@ -615,6 +614,7 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, const string& native
 
     // Sets a structure with v8 String constants on the isolate object at slot 1
     auto consts = new V8StringConstants::PerIsolateV8Constants(isolate);
+    isolate->SetData((uint32_t)Runtime::IsolateData::RUNTIME, this);
     isolate->SetData((uint32_t)Runtime::IsolateData::CONSTANTS, consts);
 
     V8::SetFlagsFromString(Constants::V8_STARTUP_FLAGS.c_str(), Constants::V8_STARTUP_FLAGS.size());
@@ -874,7 +874,7 @@ int Runtime::GetReader(){
 JavaVM* Runtime::s_jvm = nullptr;
 jmethodID Runtime::GET_USED_MEMORY_METHOD_ID = nullptr;
 map<int, Runtime*> Runtime::s_id2RuntimeCache;
-map<Isolate*, Runtime*> Runtime::s_isolate2RuntimesCache;
+unordered_map<Isolate*, Runtime*> Runtime::s_isolate2RuntimesCache;
 bool Runtime::s_mainThreadInitialized = false;
 v8::Platform* Runtime::platform = nullptr;
 int Runtime::m_androidVersion = Runtime::GetAndroidVersion();
diff --git a/test-app/runtime/src/main/cpp/Runtime.h b/test-app/runtime/src/main/cpp/Runtime.h
@@ -29,6 +29,8 @@ class Runtime {
 
         static Runtime* GetRuntime(v8::Isolate* isolate);
 
+        static Runtime* GetRuntimeFromIsolateData(v8::Isolate* isolate);
+
         static ObjectManager* GetObjectManager(v8::Isolate* isolate);
 
         static void Init(JavaVM* vm, void* reserved);
@@ -117,7 +119,7 @@ class Runtime {
 
         static std::map<int, Runtime*> s_id2RuntimeCache;
 
-        static std::map<v8::Isolate*, Runtime*> s_isolate2RuntimesCache;
+        static std::unordered_map<v8::Isolate*, Runtime*> s_isolate2RuntimesCache;
 
         static JavaVM* s_jvm;
 
diff --git a/test-app/runtime/src/main/cpp/conversions/JSToJavaConverter.cpp b/test-app/runtime/src/main/cpp/conversions/JSToJavaConverter.cpp
@@ -19,11 +19,12 @@ using namespace std;
 using namespace tns;
 
 JSToJavaConverter::JSToJavaConverter(
+        v8::Isolate* isolate,
         const FunctionCallbackInfo<v8::Value> &args,
         const string &methodSignature,
         MetadataEntry *entry)
         :
-        m_isolate(args.GetIsolate()),
+        m_isolate(isolate),
         m_methodSignature(methodSignature) {
     m_argsLen = args.Length();
 
@@ -39,10 +40,9 @@ JSToJavaConverter::JSToJavaConverter(
             std::vector<std::string> parsed = parser.Parse();
             m_tokens = new std::vector<std::string>();
             m_tokens->reserve(parsed.size());
-            for (std::string& p : parsed) {
+            for (std::string &p: parsed) {
                 m_tokens->push_back(p);
             }
-//            std::copy(parsed.begin(),  parsed.end(), m_tokens->begin());
         }
 
         for (int i = 0; i < m_argsLen; i++) {
@@ -52,12 +52,13 @@ JSToJavaConverter::JSToJavaConverter(
 }
 
 JSToJavaConverter::JSToJavaConverter(
+        v8::Isolate* isolate,
         const v8::FunctionCallbackInfo<Value> &args,
         const string &methodSignature,
         MetadataEntry *entry,
         const v8::Local<v8::Object> kotlinExtensionFunctionThis)
         :
-        m_isolate(args.GetIsolate()),
+        m_isolate(isolate),
         m_methodSignature(methodSignature) {
 
     int v8ProvidedArgumentsLength = args.Length();
@@ -75,7 +76,7 @@ JSToJavaConverter::JSToJavaConverter(
             auto parsed = parser.Parse();
             m_tokens = new std::vector<std::string>();
             m_tokens->reserve(parsed.size());
-            for (std::string& p : parsed) {
+            for (std::string &p: parsed) {
                 m_tokens->push_back(p);
             }
         }
@@ -89,18 +90,19 @@ JSToJavaConverter::JSToJavaConverter(
 }
 
 JSToJavaConverter::JSToJavaConverter(
+        v8::Isolate* isolate,
         const v8::FunctionCallbackInfo<Value> &args,
         const string &methodSignature)
         :
-        m_isolate(args.GetIsolate()),
+        m_isolate(isolate),
         m_methodSignature(methodSignature) {
     m_argsLen = args.Length();
 
     JniSignatureParser parser(m_methodSignature);
     auto parsed = parser.Parse();
     m_tokens = new std::vector<std::string>();
     m_tokens->reserve(parsed.size());
-    for (std::string& p : parsed) {
+    for (std::string &p: parsed) {
         m_tokens->push_back(p);
     }
 
@@ -110,22 +112,20 @@ JSToJavaConverter::JSToJavaConverter(
 }
 
 bool JSToJavaConverter::ConvertArg(const Local<Value> &arg, int index) {
-    const auto &typeSignature = m_tokens->at(index);
 
-    if (arg->IsArray() && typeSignature[0] == '[') {
-        return tns::ConvertJavaScriptArray(
+
+    if (arg->IsNumber() || arg->IsNumberObject()) {
+        return tns::ConvertJavaScriptNumber(
                 m_isolate,
                 arg,
                 index,
                 m_tokens,
                 m_jniArgRefsState
         );
-    } else if (arg->IsNumber() || arg->IsNumberObject()) {
-        return tns::ConvertJavaScriptNumber(
-                m_isolate,
+    } else if (arg->IsString() || arg->IsStringObject()) {
+        return tns::ConvertJavaScriptString(
                 arg,
                 index,
-                m_tokens,
                 m_jniArgRefsState
         );
     } else if (arg->IsBoolean() || arg->IsBooleanObject()) {
@@ -136,10 +136,12 @@ bool JSToJavaConverter::ConvertArg(const Local<Value> &arg, int index) {
                 m_tokens,
                 m_jniArgRefsState
         );
-    } else if (arg->IsString() || arg->IsStringObject()) {
-        return tns::ConvertJavaScriptString(
+    } else if (arg->IsArray() && m_tokens->at(index)[0] == '[') {
+        return tns::ConvertJavaScriptArray(
+                m_isolate,
                 arg,
                 index,
+                m_tokens,
                 m_jniArgRefsState
         );
     } else if (arg->IsObject()) {
@@ -155,10 +157,11 @@ bool JSToJavaConverter::ConvertArg(const Local<Value> &arg, int index) {
     } else if (arg->IsUndefined() || arg->IsNull()) {
         m_jniArgRefsState.SetConvertedObject(index, nullptr);
         return true;
-    } else {
-        m_jniArgRefsState.SetConvertedObject(index, nullptr);
-        return false;
     }
+
+    m_jniArgRefsState.SetConvertedObject(index, nullptr);
+    return false;
+
 }
 
 int JSToJavaConverter::Length() const {
diff --git a/test-app/runtime/src/main/cpp/conversions/JSToJavaConverter.h b/test-app/runtime/src/main/cpp/conversions/JSToJavaConverter.h
@@ -16,17 +16,20 @@ namespace tns {
     public:
 
         JSToJavaConverter(
+                v8::Isolate* isolate,
                 const v8::FunctionCallbackInfo<v8::Value> &args,
                 const std::string &methodSignature,
                 MetadataEntry *entry);
 
         JSToJavaConverter(
+                v8::Isolate* isolate,
                 const v8::FunctionCallbackInfo<v8::Value> &args,
                 const std::string &methodSignature,
                 MetadataEntry *entry,
                 const v8::Local<v8::Object> kotlinExtensionFunctionThis);
 
         JSToJavaConverter(
+                v8::Isolate* isolate,
                 const v8::FunctionCallbackInfo<v8::Value> &args,
                 const std::string &methodSignature);
 
