Merge pull request #585 from NativeScript/pete/workers-patch

Keep strong references of worker instances, update tests submodule

Author: petekanev

runtime/src/main/java/com/tns/Runtime.java

@@ -233,6 +233,7 @@ public void handleMessage(Message msg) {
                 WorkerGlobalOnMessageCallback(currentRuntime.runtimeId, msg.obj.toString());
             } else if (msg.arg1 == MessageType.TerminateThread) {
                 currentRuntime.isTerminating = true;
+                currentRuntime.gcListener.unsubscribe(currentRuntime);
 
                 runtimeCache.remove(currentRuntime.runtimeId);
 
@@ -251,6 +252,7 @@ public void handleMessage(Message msg) {
                 currentRuntime.mainThreadHandler.sendMessage(msgToMain);
 
                 currentRuntime.isTerminating = true;
+                currentRuntime.gcListener.unsubscribe(currentRuntime);
 
                 runtimeCache.remove(currentRuntime.runtimeId);
 
@@ -291,8 +293,17 @@ public void run() {
                     WorkThreadScheduler workThreadScheduler = new WorkThreadScheduler(new WorkerThreadHandler());
 
                     DynamicConfiguration dynamicConfiguration = new DynamicConfiguration(workerId, workThreadScheduler, mainThreadScheduler, callingJsDir);
+
+                    if(staticConfiguration.logger.isEnabled()) {
+                        staticConfiguration.logger.write("Worker (id=" + workerId + ")'s Runtime is initializing!");
+                    }
+
                     Runtime runtime = initRuntime(dynamicConfiguration);
 
+                    if(staticConfiguration.logger.isEnabled()) {
+                        staticConfiguration.logger.write("Worker (id=" + workerId + ")'s Runtime initialized!");
+                    }
+
 					/*
 						Send a message to the Main Thread to `shake hands`,
 						Main Thread will cache the Worker Handler for later use

runtime/src/main/jni/ArgConverter.cpp

@@ -184,18 +184,10 @@ std::string ArgConverter::jstringToString(jstring value)
 		return string();
 	}
 
-	jsize utfLength;
-	bool readInBuffer = ReadJStringInBuffer(value, utfLength);
-	if (readInBuffer)
-	{
-		string s(charBuffer, utfLength);
-		return s;
-	}
-
 	JEnv env;
 
-	jboolean f = false;
-	const char* chars = env.GetStringUTFChars(value, &f);
+	jboolean f = JNI_FALSE;
+	auto chars = env.GetStringUTFChars(value, &f);
 	string s(chars);
 	env.ReleaseStringUTFChars(value, chars);
 
@@ -218,27 +210,6 @@ Local<Value> ArgConverter::jstringToV8String(Isolate *isolate, jstring value)
 	return v8String;
 }
 
-bool ArgConverter::ReadJStringInBuffer(jstring value, jsize& utfLength)
-{
-	if (value == nullptr)
-	{
-		return false;
-	}
-
-	JEnv env;
-	utfLength = env.GetStringUTFLength(value);
-	if (utfLength > BUFFER_SIZE)
-	{
-		return false;
-	}
-
-	jsize strLength = env.GetStringLength(value);
-	// use existing buffer to prevent extensive memory allocation
-	env.GetStringUTFRegion(value, (jsize) 0, strLength, charBuffer);
-
-	return true;
-}
-
 Local<String> ArgConverter::jcharToV8String(Isolate *isolate, jchar value)
 {
 	auto v8String = ConvertToV8String(isolate, &value, 1);
@@ -340,5 +311,4 @@ Local<String> ArgConverter::ConvertToV8String(Isolate *isolate, const char *data
 }
 
 
-std::map<Isolate*, ArgConverter::TypeLongOperationsCache *> ArgConverter::s_type_long_operations_cache;
-char* ArgConverter::charBuffer = new char[ArgConverter::BUFFER_SIZE];
+std::map<Isolate*, ArgConverter::TypeLongOperationsCache *> ArgConverter::s_type_long_operations_cache;

runtime/src/main/jni/ArgConverter.h

@@ -55,8 +55,6 @@ namespace tns
 
 			static TypeLongOperationsCache *GetTypeLongCache(v8::Isolate *isolate);
 
-			static bool ReadJStringInBuffer(jstring value, jsize& utfLength);
-
 			static jstring ObjectToString(jobject object);
 
 			static v8::Local<v8::String> jcharToV8String(v8::Isolate *isolate, jchar value);
@@ -67,9 +65,6 @@ namespace tns
 
 			static void NativeScriptLongToStringFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
 
-			static char *charBuffer;
-			static const int BUFFER_SIZE = 1024 * 64; // 64KB size. TODO: Do we need a larger/smaller buffer?
-
 			/*
 			 * "s_type_long_operations_cache" used to keep function
 			 * dealing with operations concerning java long -> javascript number.

runtime/src/main/jni/CallbackHandlers.cpp

@@ -919,8 +919,6 @@ void CallbackHandlers::NewThreadCallback(const v8::FunctionCallbackInfo<v8::Valu
 
         id2WorkerMap.insert(make_pair(workerId, persistentWorker));
 
-        persistentWorker->SetWeak();
-
         DEBUG_WRITE("Called Worker constructor id=%d", workerId);
 
         JEnv env;

runtime/src/main/jni/MetadataNode.cpp

@@ -829,6 +829,8 @@ Local<FunctionTemplate> MetadataNode::GetConstructorFunctionTemplate(Isolate *is
 
 	node->SetInstanceMembers(isolate, ctorFuncTemplate, prototypeTemplate, instanceMethodsCallbackData, baseInstanceMethodsCallbackData, treeNode);
 
+	ctorFuncTemplate->SetClassName(ConvertToV8String(node->m_treeNode->name));
+
 	auto ctorFunc = ctorFuncTemplate->GetFunction();
 
     auto origin = Constants::APP_ROOT_FOLDER_PATH + node->m_name;

runtime/src/main/jni/Runtime.cpp

@@ -417,6 +417,8 @@ static void InitializeV8() {
 	V8::Initialize();
 }
 
+bool x = false;
+
 Isolate* Runtime::PrepareV8Runtime(const string& filesPath, jstring packageName, jstring callingDir, jobject jsDebugger, jstring profilerOutputDir)
 {
 	Isolate::CreateParams create_params;
@@ -605,7 +607,10 @@ Isolate* Runtime::PrepareV8Runtime(const string& filesPath, jstring packageName,
 	m_profiler.Init(isolate, global, pckName, outputDir);
 	JsDebugger::Init(isolate, pckName, jsDebugger);
 
-	MetadataNode::BuildMetadata(filesPath);
+	// Do not build metadata (which should be static for the process) for non-main threads
+	if (!s_mainThreadInitialized) {
+		MetadataNode::BuildMetadata(filesPath);
+	}
 
 	auto enableProfiler = !outputDir.empty();
 	MetadataNode::EnableProfiler(enableProfiler);

test-app/app/src/main/assets/app/mainpage.js

@@ -19,7 +19,7 @@ var shared = require("./shared");
 shared.runRequireTests();
 shared.runWeakRefTests();
 shared.runRuntimeTests();
-//shared.runWorkerTests();
+shared.runWorkerTests();
 
 require("./tests/testMetadata");
 require("./tests/testAsserts");

test-app/app/src/main/assets/app/shared

@@ -8,7 +8,7 @@ describe("Tests array operations", function () {
 		jasmine.addCustomEqualityTester(myCustomEquality);
 	});
 
-	it("TestWorkingWithJavaArrayDoesNotMakeMemoryLeak", function () {
+	xit("TestWorkingWithJavaArrayDoesNotMakeMemoryLeak", function () {
 		__log("TEST: TestWorkingWithJavaArrayDoesNotMakeMemoryLeak");
 
 		var size = 10 * 1024 * 1024;

test-app/app/src/main/assets/app/tests/testArrays.js

@@ -622,27 +622,6 @@ describe("Tests ", function () {
 		expect(file).toBe(file2);
 	});
 
-	it("TestWorkingWithJavaArrayDoesNotMakeMemoryLeak", function () {
-		__log("TEST: TestWorkingWithJavaArrayDoesNotMakeMemoryLeak");
-
-		var size = 10 * 1024 * 1024;
-
-		for (var i = 0; i < 100; i++) {
-
-			var arr = java.lang.reflect.Array.newInstance(java.lang.Byte.class.getField("TYPE").get(null), size);
-
-			var length = arr.length;
-
-			expect(length).toEqual(size);
-
-			arr[0] = 123;
-
-			var el = arr[0];
-
-			expect(el).toEqual(123);
-		}
-	});
-
 	it("TestConstructorOverride", function () {