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}
This commit is contained in:
sakal 2016-09-16 06:56:15 -07:00 committed by Commit bot
parent a6974d7f7e
commit 71b8393b6a
4 changed files with 37 additions and 41 deletions

View File

@ -68,8 +68,10 @@ JOW(jlong, Metrics_00024Histogram_nativeCreateCounts)
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

View File

@ -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* HistogramFactoryGetCounts(
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

View File

@ -55,9 +55,6 @@ int NumSamples(const std::string& name);
// 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

View File

@ -232,23 +232,13 @@ Histogram* HistogramFactoryGetEnumeration(const std::string& name,
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);
}