Add a default implementation in metrics_default.cc of histograms methods in system_wrappers/interfac

webrtc/api/java/jni/androidhistograms_jni.cc

+/*

[pbos-webrtc, 2016/05/02 00:29:10]

Shouldn't this file be part of metrics_default? I think the JNI parts of these
should be here, but metrics_default should contain all functionality. That way
if someone wants to wire up metrics through C++ code it'll be easier as well.

Imo metrics could also move to webrtc/base.

[åsapersson, 2016/05/06 13:31:57]

Added implementation to metrics_default and have removed the test
implementation, ptal. 

Java specific parts have been moved to a separate CL:
https://codereview.webrtc.org/1952223007/ 

+ *  Copyright 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <map>
+
+#include "webrtc/api/java/jni/classreferenceholder.h"
+#include "webrtc/api/java/jni/jni_helpers.h"
+#include "webrtc/api/java/jni/native_handle_impl.h"
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/thread_annotations.h"
+#include "webrtc/system_wrappers/include/metrics.h"
+
+namespace {
+// Limit for the maximum number of sample values that can be stored.
+const int kMaxMapSize = 10000;
+
+struct SampleInfo {
+  SampleInfo(const std::string& name, int min, int max, int bucket_count)
+      : name(name), min(min), max(max), bucket_count(bucket_count) {}
+  const std::string name;
+  const int min;
+  const int max;
+  const int bucket_count;
+  std::map<int, int> samples;  // <value, # of events>
+};
+
+// Map holding samples of a histogram (mapped by the histogram name).
+rtc::CriticalSection rtc_histograms_crit_;
+bool rtc_histograms_enabled_ = false;

[pbos-webrtc, 2016/05/02 00:29:10]

Can we expect this to be set before any webrtc code gets called? We do that for
chrome://tracing, and that way you shouldn't have to take rtc_histograms_crit_
before checking rtc_histograms_enabled_.

[åsapersson, 2016/05/06 13:31:57]

Done.

+std::map<std::string, SampleInfo> rtc_histograms_
+    GUARDED_BY(rtc_histograms_crit_);
+}  // namespace
+
+// Android implementation of histogram methods in
+// webrtc/system_wrappers/interface/metrics.h.
+
+namespace webrtc {
+namespace metrics {
+class Histogram;
+
+// Histogram with exponentially spaced buckets.
+Histogram* HistogramFactoryGetCounts(const std::string& name,
+                                     int min,
+                                     int max,
+                                     int bucket_count) {
+  rtc::CritScope cs(&rtc_histograms_crit_);
+  if (rtc_histograms_.find(name) == rtc_histograms_.end()) {
+    rtc_histograms_.insert(
+        std::make_pair(name, SampleInfo(name, min, max, bucket_count)));
+  }
+  auto it = rtc_histograms_.find(name);
+  return reinterpret_cast<Histogram*>(&it->second);
+}
+
+// Histogram with linearly spaced buckets.
+Histogram* HistogramFactoryGetEnumeration(const std::string& name,
+                                          int boundary) {
+  rtc::CritScope cs(&rtc_histograms_crit_);
+  if (rtc_histograms_.find(name) == rtc_histograms_.end()) {
+    rtc_histograms_.insert(
+        std::make_pair(name, SampleInfo(name, 1, boundary, boundary + 1)));
+  }
+  auto it = rtc_histograms_.find(name);
+  return reinterpret_cast<Histogram*>(&it->second);
+}
+
+void HistogramAdd(Histogram* histogram_pointer,
+                  const std::string& name,
+                  int sample) {
+  rtc::CritScope cs(&rtc_histograms_crit_);
+  if (!rtc_histograms_enabled_)
+    return;
+
+  SampleInfo* ptr = reinterpret_cast<SampleInfo*>(histogram_pointer);

[pbos-webrtc, 2016/05/02 00:29:10]

This function shouldn't hold the rtc_histograms_crit_ lock right, for
performance reasons. We want all cached uses of histogram_pointer to be fast.

[åsapersson, 2016/05/06 13:31:57]

Done.

+  RTC_DCHECK(ptr->name == name) << "The name should not vary.";
+
+  if (sample < 0)
+    sample = 0;
+  if (sample > ptr->max)
+    sample = ptr->max;
+
+  if (ptr->samples.size() == kMaxMapSize &&
+      ptr->samples.find(sample) == ptr->samples.end()) {
+    return;
+  }
+  ++ptr->samples[sample];
+}
+
+}  // namespace metrics
+}  // namespace webrtc
+
+
+// Enables collection of samples to |rtc_histograms_|.
+namespace webrtc_jni {
+JOW(void, Histograms_nativeEnable)(JNIEnv* jni, jclass) {

[pbos-webrtc, 2016/05/02 00:29:10]

Can we have C++ versions of these functions as well?

And if we do, does that mean that we can remove the test-target histogram
implementation?

[åsapersson, 2016/05/06 13:31:57]

Done.

+  rtc::CritScope cs(&rtc_histograms_crit_);
+  rtc_histograms_enabled_ = true;
+}
+
+// Gets and resets samples in |rtc_histograms_|.
+JOW(jobject, Histograms_nativeGetAndReset)(JNIEnv* jni, jclass) {
+  jclass j_histograms_class = FindClass(jni, "org/webrtc/Histograms");
+  jmethodID j_add =
+      GetMethodID(jni, j_histograms_class, "add",
+                  "(Ljava/lang/String;Lorg/webrtc/Histograms$HistogramInfo;)V");
+  jclass j_info_class = FindClass(jni, "org/webrtc/Histograms$HistogramInfo");
+  jmethodID j_add_sample = GetMethodID(jni, j_info_class, "addSample", "(II)V");
+
+  // Create |Histograms|.
+  jobject j_histograms =
+      jni->NewObject(j_histograms_class,
+                     GetMethodID(jni, j_histograms_class, "<init>", "()V"));
+
+  rtc::CritScope cs(&rtc_histograms_crit_);
+  for (auto& kv : rtc_histograms_) {
+    // Create and add samples to |HistogramInfo|.
+    jobject j_info = jni->NewObject(
+        j_info_class, GetMethodID(jni, j_info_class, "<init>", "(III)V"),
+        kv.second.min, kv.second.max, kv.second.bucket_count);
+    for (const auto& sample : kv.second.samples) {
+      jni->CallVoidMethod(j_info, j_add_sample, sample.first, sample.second);
+    }
+    kv.second.samples.clear();
+    // Add |HistogramInfo| to |Histograms|.
+    jstring j_name = jni->NewStringUTF(kv.first.c_str());
+    jni->CallVoidMethod(j_histograms, j_add, j_name, j_info);
+    jni->DeleteLocalRef(j_name);
+    jni->DeleteLocalRef(j_info);
+  }
+  CHECK_EXCEPTION(jni);
+  return j_histograms;
+}
+}  // namespace webrtc_jni