[Android] Cache Java-based histogram lookup through RecordHistogram

Avoid JNI string translation for each histogrom update routed through
RecordHistogram by caching the histogram lookup. Use the Java string
handle as the key.

BUG=442300

Review URL: https://codereview.chromium.org/867063006

Cr-Commit-Position: refs/heads/master@{#313174}

base/android/java/src/org/chromium/base/metrics/RecordHistogram.java

@@ -21,7 +21,7 @@ public class RecordHistogram {
      * @param sample sample to be recorded, either true or false
      */
     public static void recordBooleanHistogram(String name, boolean sample) {
-        nativeRecordBooleanHistogram(name, sample);
+        nativeRecordBooleanHistogram(name, System.identityHashCode(name), sample);
     }
 
     /**
@@ -33,7 +33,7 @@ public class RecordHistogram {
      *        lower than |boundary|
      */
     public static void recordEnumeratedHistogram(String name, int sample, int boundary) {
-        nativeRecordEnumeratedHistogram(name, sample, boundary);
+        nativeRecordEnumeratedHistogram(name, System.identityHashCode(name), sample, boundary);
     }
 
     /**
@@ -53,9 +53,9 @@ public class RecordHistogram {
         nativeInitialize();
     }
 
-    private static native void nativeRecordBooleanHistogram(String name, boolean sample);
+    private static native void nativeRecordBooleanHistogram(String name, int key, boolean sample);
     private static native void nativeRecordEnumeratedHistogram(
-            String name, int sample, int boundary);
+            String name, int key, int sample, int boundary);
 
     private static native int nativeGetHistogramValueCountForTesting(String name, int sample);
     private static native void nativeInitialize();

base/android/record_histogram.cc

@@ -2,39 +2,108 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/android/record_histogram.h"
+
+#include <map>
+
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
-#include "base/android/record_histogram.h"
+#include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/statistics_recorder.h"
+#include "base/synchronization/lock.h"
 #include "jni/RecordHistogram_jni.h"
 
 namespace base {
 namespace android {
+namespace {
+
+// Simple thread-safe wrapper for caching histograms. This avoids
+// relatively expensive JNI string translation for each recording.
+class HistogramCache {
+ public:
+  HistogramCache() {}
+
+  HistogramBase* BooleanHistogram(JNIEnv* env,
+                                  jstring j_histogram_name,
+                                  jint j_histogram_key) {
+    DCHECK(j_histogram_name);
+    DCHECK(j_histogram_key);
+    HistogramBase* histogram = FindLocked(j_histogram_key);
+    if (histogram)
+      return histogram;
+
+    std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
+    histogram = BooleanHistogram::FactoryGet(
+        histogram_name, HistogramBase::kUmaTargetedHistogramFlag);
+    return InsertLocked(j_histogram_key, histogram);
+  }
+
+  HistogramBase* EnumeratedHistogram(JNIEnv* env,
+                                     jstring j_histogram_name,
+                                     jint j_histogram_key,
+                                     jint j_boundary) {
+    DCHECK(j_histogram_name);
+    DCHECK(j_histogram_key);
+    HistogramBase* histogram = FindLocked(j_histogram_key);
+    if (histogram)
+      return histogram;
+
+    std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
+    // Note: This caching bypasses the boundary validation that occurs between
+    // repeated lookups by the same name. It is up to the caller to ensure that
+    // the provided boundary remains consistent.
+    int boundary = static_cast<int>(j_boundary);
+    histogram =
+        LinearHistogram::FactoryGet(histogram_name, 1, boundary, boundary + 1,
+                                    HistogramBase::kUmaTargetedHistogramFlag);
+    return InsertLocked(j_histogram_key, histogram);
+  }
+
+ private:
+  HistogramBase* FindLocked(jint j_histogram_key) {
+    base::AutoLock locked(lock_);
+    auto histogram_it = histograms_.find(j_histogram_key);
+    return histogram_it != histograms_.end() ? histogram_it->second : nullptr;
+  }
+
+  HistogramBase* InsertLocked(jint j_histogram_key, HistogramBase* histogram) {
+    base::AutoLock locked(lock_);
+    histograms_.insert(std::make_pair(j_histogram_key, histogram));
+    return histogram;
+  }
+
+  base::Lock lock_;
+  std::map<jint, HistogramBase*> histograms_;
+
+  DISALLOW_COPY_AND_ASSIGN(HistogramCache);
+};
+
+base::LazyInstance<HistogramCache>::Leaky g_histograms;
+
+}  // namespace
 
 void RecordBooleanHistogram(JNIEnv* env,
                             jclass clazz,
                             jstring j_histogram_name,
+                            jint j_histogram_key,
                             jboolean j_sample) {
-  std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
   bool sample = static_cast<bool>(j_sample);
-
-  BooleanHistogram::FactoryGet(histogram_name,
-                               HistogramBase::kUmaTargetedHistogramFlag)
+  g_histograms.Get()
+      .BooleanHistogram(env, j_histogram_name, j_histogram_key)
       ->AddBoolean(sample);
 }
 
 void RecordEnumeratedHistogram(JNIEnv* env,
                                jclass clazz,
                                jstring j_histogram_name,
+                               jint j_histogram_key,
                                jint j_sample,
                                jint j_boundary) {
-  std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
   int sample = static_cast<int>(j_sample);
-  int boundary = static_cast<int>(j_boundary);
 
-  LinearHistogram::FactoryGet(histogram_name, 1, boundary, boundary + 1,
-                              HistogramBase::kUmaTargetedHistogramFlag)
+  g_histograms.Get()
+      .EnumeratedHistogram(env, j_histogram_name, j_histogram_key, j_boundary)
       ->Add(sample);
 }