Addres peer review

PanayotCankov · PanayotCankov · commit 3f67853889b9 · 2017-06-06T18:12:33.000+03:00
diff --git a/build-artifacts/project-template-gradle/src/main/java/com/tns/RuntimeHelper.java b/build-artifacts/project-template-gradle/src/main/java/com/tns/RuntimeHelper.java
@@ -119,14 +119,7 @@ public static Runtime initRuntime(Application app) {
                 }
 
                 AppConfig appConfig = new AppConfig(appDir);
-                switch (appConfig.getProfilingMode()) {
-                    case "timeline":
-                        ManualInstrumentation.setMode(ManualInstrumentation.Mode.Timeline);
-                        break;
-                    default:
-                        ManualInstrumentation.setMode(ManualInstrumentation.Mode.Disabled);
-                }
-                Runtime.SetManualInstrumentationMode(appConfig.getProfilingMode());
+                ManualInstrumentation.setMode(appConfig.getProfilingMode());
 
                 ClassLoader classLoader = app.getClassLoader();
                 File dexDir = new File(rootDir, "code_cache/secondary-dexes");
diff --git a/runtime/src/main/java/com/tns/ManualInstrumentation.java b/runtime/src/main/java/com/tns/ManualInstrumentation.java
@@ -10,6 +10,17 @@
 public class ManualInstrumentation {
     private static Mode mode = Mode.Pending;
 
+    public static void setMode(String mode) {
+        switch (mode) {
+            case "timeline":
+                ManualInstrumentation.setMode(ManualInstrumentation.Mode.Timeline);
+                break;
+            default:
+                ManualInstrumentation.setMode(ManualInstrumentation.Mode.Disabled);
+        }
+        Runtime.SetManualInstrumentationMode(mode);
+    }
+
     public static void setMode(Mode mode) {
         if (ManualInstrumentation.mode == Mode.Pending) {
             switch(mode) {
diff --git a/runtime/src/main/jni/ManualInstrumentation.cpp b/runtime/src/main/jni/ManualInstrumentation.cpp
@@ -5,4 +5,4 @@
 #include "ManualInstrumentation.h"
 
 bool tns::instrumentation::Frame::disabled = true;
-const std::chrono::steady_clock::time_point tns::instrumentation::Frame::disabled_time = std::chrono::steady_clock::now();
+const std::chrono::steady_clock::time_point tns::instrumentation::Frame::disabled_time = std::chrono::steady_clock::time_point();
diff --git a/runtime/src/main/jni/ManualInstrumentation.h b/runtime/src/main/jni/ManualInstrumentation.h
@@ -2,22 +2,23 @@
 // Created by Panayot Cankov on 26/05/2017.
 //
 
-#ifndef TEST_APP_MANUALINSTRUMENTATION_H
-#define TEST_APP_MANUALINSTRUMENTATION_H
+#ifndef MANUALINSTRUMENTATION_H
+#define MANUALINSTRUMENTATION_H
 
 #include "v8.h"
 #import <chrono>
 #import <NativeScriptAssert.h>
+#include <string>
 
 namespace tns {
     namespace instrumentation {
         class Frame {
         public:
-            inline Frame() : Frame(nullptr) { }
-            inline Frame(const char *name) : name(name), start(disabled ? disabled_time : std::chrono::steady_clock::now()) {}
+            inline Frame() : Frame("") { }
+            inline Frame(std::string name) : name(name), start(disabled ? disabled_time : std::chrono::steady_clock::now()) {}
 
             inline ~Frame() {
-                if (name && check()) {
+                if (!name.empty() && check()) {
                     log(name);
                 }
             }
@@ -42,6 +43,10 @@ namespace tns {
                 __android_log_print(ANDROID_LOG_DEBUG, "JS", "Timeline: %.3fms: Runtime: %s  (%.3fms - %.3fms)", duration / 1000.0, message, startMilis, endMilis);
             }
 
+            inline void log(const std::string& message) {
+                log(message.c_str());
+            }
+
             static inline void enable() { disabled = false; }
             static inline void disable() { disabled = true; }
 
@@ -50,7 +55,7 @@ namespace tns {
             static const std::chrono::steady_clock::time_point disabled_time; // Couldn't find reasonable constant
 
             const std::chrono::steady_clock::time_point start;
-            const char *name;
+            const std::string name;
 
             Frame(const Frame &) = delete;
             Frame &operator=(const Frame &) = delete;
@@ -63,4 +68,4 @@ namespace tns {
  */
 #define TNSPERF() tns::instrumentation::Frame __tns_manual_instrumentation(__func__)
 
-#endif //TEST_APP_MANUALINSTRUMENTATION_H
+#endif //MANUALINSTRUMENTATION_H
diff --git a/runtime/src/main/jni/MetadataNode.cpp b/runtime/src/main/jni/MetadataNode.cpp
@@ -756,8 +756,7 @@ Local<FunctionTemplate> MetadataNode::GetConstructorFunctionTemplate(Isolate* is
     SetTypeMetadata(isolate, wrappedCtorFunc, new TypeMetadata(s_metadataReader.ReadTypeName(treeNode)));
 
     if (frame.check()) {
-        std::string log = "Materizlizing class: " + node->m_name;
-        frame.log(log.c_str());
+        frame.log("Materizlizing class: " + node->m_name);
     }
 
     return ctorFuncTemplate;
@@ -896,8 +895,7 @@ void MetadataNode::InterfaceConstructorCallback(const v8::FunctionCallbackInfo<v
         auto success = CallbackHandlers::RegisterInstance(isolate, thiz, className, argWrapper, implementationObject, true);
 
         if (frame.check()) {
-            std::string msg = "Interface constructor: " + node->m_name;
-            frame.log(msg.c_str());
+            frame.log("Interface constructor: " + node->m_name);
         }
     } catch (NativeScriptException& e) {
         e.ReThrowToV8();
@@ -1377,8 +1375,7 @@ void MetadataNode::ExtendMethodCallback(const v8::FunctionCallbackInfo<v8::Value
         cache->ExtendedCtorFuncCache.insert(make_pair(fullExtendedName, cacheData));
 
         if (frame.check()) {
-            std::string msg = "Extending: " + node->m_name;
-            frame.log(msg.c_str());
+            frame.log("Extending: " + node->m_name);
         }
     } catch (NativeScriptException& e) {
         e.ReThrowToV8();
diff --git a/runtime/src/main/jni/ModuleInternal.cpp b/runtime/src/main/jni/ModuleInternal.cpp
@@ -157,8 +157,7 @@ void ModuleInternal::RequireCallbackImpl(const v8::FunctionCallbackInfo<v8::Valu
     }
 
     string moduleName = ArgConverter::ConvertToString(args[0].As<String>());
-    string frameName("RequireCallback " + moduleName);
-    tns::instrumentation::Frame frame(frameName.c_str());
+    tns::instrumentation::Frame frame(("RequireCallback " + moduleName).c_str());
     string callingModuleDirName = ArgConverter::ConvertToString(args[1].As<String>());
     auto isData = false;
 
diff --git a/runtime/src/main/jni/ObjectManager.cpp b/runtime/src/main/jni/ObjectManager.cpp
@@ -520,14 +520,9 @@ void ObjectManager::MarkReachableObjects(Isolate* isolate, const Local<Object>&
 
     if (frame.check()) {
         auto cls = fromJsInfo->ObjectClazz;
-        JEnv env;
-        jclass clsClazz = env.GetObjectClass(cls);
-        jmethodID methodId = env.GetMethodID(clsClazz, "getSimpleName", "()Ljava/lang/String;");
-        jstring className = (jstring) env.CallObjectMethod(cls, methodId);
-        const char* str = env.GetStringUTFChars(className, NULL);
-        auto jsObjClassName = "MarkReachableObjects: " + std::string(str);
-        frame.log(jsObjClassName.c_str());
-        env.ReleaseStringUTFChars(className, str);
+        jclass clsClazz = m_env.GetObjectClass(cls);
+        jstring className = (jstring) m_env.CallObjectMethod(cls, GET_NAME_METHOD_ID);
+        frame.log("MarkReachableObjects: " + ArgConverter::jstringToString(className));
     }
 }
 
diff --git a/test-app/app/src/main/java/com/tns/RuntimeHelper.java b/test-app/app/src/main/java/com/tns/RuntimeHelper.java
@@ -119,14 +119,7 @@ public static Runtime initRuntime(Application app) {
                 }
 
                 AppConfig appConfig = new AppConfig(appDir);
-                switch (appConfig.getProfilingMode()) {
-                    case "timeline":
-                        ManualInstrumentation.setMode(ManualInstrumentation.Mode.Timeline);
-                        break;
-                    default:
-                        ManualInstrumentation.setMode(ManualInstrumentation.Mode.Disabled);
-                }
-                Runtime.SetManualInstrumentationMode(appConfig.getProfilingMode());
+                ManualInstrumentation.setMode(appConfig.getProfilingMode());
 
                 ClassLoader classLoader = app.getClassLoader();
                 File dexDir = new File(rootDir, "code_cache/secondary-dexes");

Original file line number	Diff line number	Diff line change
`@@ -157,8 +157,7 @@ void ModuleInternal::RequireCallbackImpl(const v8::FunctionCallbackInfo<v8::Valu`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`string moduleName = ArgConverter::ConvertToString(args[0].As<String>());`
`160`		`- string frameName("RequireCallback " + moduleName);`
`161`		`- tns::instrumentation::Frame frame(frameName.c_str());`
	`160`	`+ tns::instrumentation::Frame frame(("RequireCallback " + moduleName).c_str());`
`162`	`161`	`string callingModuleDirName = ArgConverter::ConvertToString(args[1].As<String>());`
`163`	`162`	`auto isData = false;`
`164`	`163`