From e9fc7311c472290a2b8c3b00590dff607494d99a Mon Sep 17 00:00:00 2001
From: Pieter De Baets <pieterdb@fb.com>
Date: Fri, 26 May 2017 09:23:50 -0700
Subject: [PATCH] Cleanup markers in getConstants

Summary:
* Only add the moduleName to the outer marker
* Include module init in the marker time
* Include the WritableNativeArray time in CONVERT_CONSTANTS
* Stop all markers from finally block in case of exception

Reviewed By: kathryngray

Differential Revision: D5031411

fbshipit-source-id: bf86ee3d59f2cb38866afd52e84af5699db1a003
---
 .../react/bridge/JavaModuleWrapper.java       | 33 ++++++++++---------
 .../facebook/react/bridge/ModuleHolder.java   | 13 +++-----
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java
index 9f59fef1d..afe877412 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java
@@ -129,29 +129,32 @@ public class JavaModuleWrapper {
     if (!mModuleHolder.getHasConstants()) {
       return null;
     }
-    BaseJavaModule baseJavaModule = getModule();
-    ReactMarker.logMarker(GET_CONSTANTS_START, getName());
-    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "Map constants")
-      .arg("moduleName", getName())
+
+    final String moduleName = getName();
+    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "JavaModuleWrapper.getConstants")
+      .arg("moduleName", moduleName)
       .flush();
+    ReactMarker.logMarker(GET_CONSTANTS_START, moduleName);
+
+    BaseJavaModule baseJavaModule = getModule();
+
+    Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "module.getConstants");
     Map<String, Object> map = baseJavaModule.getConstants();
     Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
 
-    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "WritableNativeMap constants")
-      .arg("moduleName", getName())
-      .flush();
-    ReactMarker.logMarker(CONVERT_CONSTANTS_START, getName());
-    WritableNativeMap writableNativeMap;
+    Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "create WritableNativeMap");
+    ReactMarker.logMarker(CONVERT_CONSTANTS_START, moduleName);
     try {
-      writableNativeMap = Arguments.makeNativeMap(map);
+      WritableNativeArray array = new WritableNativeArray();
+      array.pushMap(Arguments.makeNativeMap(map));
+      return array;
     } finally {
+      ReactMarker.logMarker(CONVERT_CONSTANTS_END);
+      Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
+
+      ReactMarker.logMarker(GET_CONSTANTS_END);
       Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
     }
-    WritableNativeArray array = new WritableNativeArray();
-    array.pushMap(writableNativeMap);
-    ReactMarker.logMarker(CONVERT_CONSTANTS_END);
-    ReactMarker.logMarker(GET_CONSTANTS_END);
-    return array;
   }
 
   @DoNotStrip
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java
index 3cab0efc6..8c520a71d 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java
@@ -99,7 +99,7 @@ public class ModuleHolder {
     SoftAssertions.assertCondition(mModule == null, "Creating an already created module.");
     int instanceKey = sInstanceKeyCounter.getAndIncrement();
     ReactMarker.logMarker(CREATE_MODULE_START, mName, instanceKey);
-    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createModule")
+    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "ModuleHolder.createModule")
       .arg("name", mName)
       .flush();
     NativeModule module;
@@ -118,14 +118,9 @@ public class ModuleHolder {
   }
 
   private void doInitialize(NativeModule module) {
-    SystraceMessage.Builder section =
-      SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "initialize");
-    if (module instanceof CxxModuleWrapper) {
-      section.arg("className", module.getClass().getSimpleName());
-    } else {
-      section.arg("name", mName);
-    }
-    section.flush();
+    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "ModuleHolder.initialize")
+      .arg("name", mName)
+      .flush();
     ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_START, mName);
     try {
       module.initialize();