Remove name parameter from HistogramAdd function.
This name is only used for a DCHECK. Having it as a parameter leads to unnecessary string copying even on release builds.
This CL instead adds a GetHistogramName function that is only called on debug builds.
Checking if pointer is null is also moved outside HistogramAdd function. Having it there is confusing since Chromium implementation doesn't have it there.
BUG=webrtc:6329
Review-Url: https://codereview.webrtc.org/2337883003
Cr-Commit-Position: refs/heads/master@{#14263}
diff --git a/webrtc/api/android/jni/androidmetrics_jni.cc b/webrtc/api/android/jni/androidmetrics_jni.cc
index 833066a..68b0b73 100644
--- a/webrtc/api/android/jni/androidmetrics_jni.cc
+++ b/webrtc/api/android/jni/androidmetrics_jni.cc
@@ -68,8 +68,10 @@
JOW(void, Metrics_00024Histogram_nativeAddSample)
(JNIEnv* jni, jclass, jlong histogram, jint sample) {
- HistogramAdd(reinterpret_cast<webrtc::metrics::Histogram*>(histogram),
- sample);
+ if (histogram) {
+ HistogramAdd(reinterpret_cast<webrtc::metrics::Histogram*>(histogram),
+ sample);
+ }
}
} // namespace webrtc_jni
diff --git a/webrtc/system_wrappers/include/metrics.h b/webrtc/system_wrappers/include/metrics.h
index ceddc95..9b14736 100644
--- a/webrtc/system_wrappers/include/metrics.h
+++ b/webrtc/system_wrappers/include/metrics.h
@@ -110,35 +110,41 @@
// The name of the histogram should not vary.
// TODO(asapersson): Consider changing string to const char*.
-#define RTC_HISTOGRAM_COMMON_BLOCK(constant_name, sample, \
- factory_get_invocation) \
- do { \
+#define RTC_HISTOGRAM_COMMON_BLOCK(constant_name, sample, \
+ factory_get_invocation) \
+ do { \
static webrtc::metrics::Histogram* atomic_histogram_pointer = nullptr; \
- webrtc::metrics::Histogram* histogram_pointer = \
- rtc::AtomicOps::AcquireLoadPtr(&atomic_histogram_pointer); \
- if (!histogram_pointer) { \
- histogram_pointer = factory_get_invocation; \
- webrtc::metrics::Histogram* prev_pointer = \
- rtc::AtomicOps::CompareAndSwapPtr( \
- &atomic_histogram_pointer, \
- static_cast<webrtc::metrics::Histogram*>(nullptr), \
- histogram_pointer); \
- RTC_DCHECK(prev_pointer == nullptr || \
- prev_pointer == histogram_pointer); \
- } \
- webrtc::metrics::HistogramAdd(histogram_pointer, constant_name, sample); \
+ webrtc::metrics::Histogram* histogram_pointer = \
+ rtc::AtomicOps::AcquireLoadPtr(&atomic_histogram_pointer); \
+ if (!histogram_pointer) { \
+ histogram_pointer = factory_get_invocation; \
+ webrtc::metrics::Histogram* prev_pointer = \
+ rtc::AtomicOps::CompareAndSwapPtr( \
+ &atomic_histogram_pointer, \
+ static_cast<webrtc::metrics::Histogram*>(nullptr), \
+ histogram_pointer); \
+ RTC_DCHECK(prev_pointer == nullptr || \
+ prev_pointer == histogram_pointer); \
+ } \
+ if (histogram_pointer) { \
+ RTC_DCHECK_EQ(constant_name, \
+ webrtc::metrics::GetHistogramName(histogram_pointer)) \
+ << "The name should not vary."; \
+ webrtc::metrics::HistogramAdd(histogram_pointer, sample); \
+ } \
} while (0)
// Deprecated.
// The histogram is constructed/found for each call.
-// May be used for histograms with infrequent updates.
+// May be used for histograms with infrequent updates.`
#define RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, factory_get_invocation) \
- do { \
- webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \
- webrtc::metrics::HistogramAdd(histogram_pointer, name, sample); \
+ do { \
+ webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \
+ if (histogram_pointer) { \
+ webrtc::metrics::HistogramAdd(histogram_pointer, sample); \
+ } \
} while (0)
-
// Helper macros.
// Macros for calling a histogram with varying name (e.g. when using a metric
// in different modes such as real-time vs screenshare).
@@ -212,10 +218,11 @@
Histogram* HistogramFactoryGetEnumeration(
const std::string& name, int boundary);
+// Returns name of the histogram.
+const std::string& GetHistogramName(Histogram* histogram_pointer);
+
// Function for adding a |sample| to a histogram.
-// |name| can be used to verify that it matches the histogram name.
-void HistogramAdd(
- Histogram* histogram_pointer, const std::string& name, int sample);
+void HistogramAdd(Histogram* histogram_pointer, int sample);
} // namespace metrics
} // namespace webrtc
diff --git a/webrtc/system_wrappers/include/metrics_default.h b/webrtc/system_wrappers/include/metrics_default.h
index 94ba3cd..e262198 100644
--- a/webrtc/system_wrappers/include/metrics_default.h
+++ b/webrtc/system_wrappers/include/metrics_default.h
@@ -55,9 +55,6 @@
// Returns the minimum sample value (or -1 if the histogram has no samples).
int MinSample(const std::string& name);
-// Function for adding a |sample| to a histogram without checkking the name.
-void HistogramAdd(Histogram* histogram_pointer, int sample);
-
} // namespace metrics
} // namespace webrtc
diff --git a/webrtc/system_wrappers/source/metrics_default.cc b/webrtc/system_wrappers/source/metrics_default.cc
index fd7610b..8ee10ba 100644
--- a/webrtc/system_wrappers/source/metrics_default.cc
+++ b/webrtc/system_wrappers/source/metrics_default.cc
@@ -232,23 +232,13 @@
return map->GetEnumerationHistogram(name, boundary);
}
-// Fast path. Adds |sample| to cached |histogram_pointer|.
-void HistogramAdd(Histogram* histogram_pointer,
- const std::string& name,
- int sample) {
- if (!histogram_pointer)
- return;
-
+const std::string& GetHistogramName(Histogram* histogram_pointer) {
RtcHistogram* ptr = reinterpret_cast<RtcHistogram*>(histogram_pointer);
- RTC_DCHECK_EQ(name, ptr->name()) << "The name should not vary.";
- ptr->Add(sample);
+ return ptr->name();
}
// Fast path. Adds |sample| to cached |histogram_pointer|.
void HistogramAdd(Histogram* histogram_pointer, int sample) {
- if (!histogram_pointer)
- return;
-
RtcHistogram* ptr = reinterpret_cast<RtcHistogram*>(histogram_pointer);
ptr->Add(sample);
}
