diff options
author | asapersson <asapersson@webrtc.org> | 2016-01-07 01:02:42 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-07 09:02:49 +0000 |
commit | 1fe48a5e1dbc752b24b6c63eb2e6abd80c01c1dc (patch) | |
tree | a432fd3069d9c98cab311fb55b985bf8c807f403 /webrtc | |
parent | 4331fcd51736b030f30c31a35c732f119b31b6fc (diff) | |
download | webrtc-1fe48a5e1dbc752b24b6c63eb2e6abd80c01c1dc.tar.gz |
Add implementation in metrics.h that uses atomic pointer.
Update test implementation (test/histograms.h) to be more similar a real implementation (where histogram get functions return a Histogram pointer). Add check that the name of a histogram does not change.
BUG=webrtc:5283
Review URL: https://codereview.webrtc.org/1528403003
Cr-Commit-Position: refs/heads/master@{#11161}
Diffstat (limited to 'webrtc')
-rw-r--r-- | webrtc/system_wrappers/include/metrics.h | 81 | ||||
-rw-r--r-- | webrtc/system_wrappers/source/metrics_unittest.cc | 91 | ||||
-rw-r--r-- | webrtc/system_wrappers/system_wrappers_tests.gyp | 2 | ||||
-rw-r--r-- | webrtc/test/histogram.cc | 49 | ||||
-rw-r--r-- | webrtc/test/histogram.h | 2 |
5 files changed, 190 insertions, 35 deletions
diff --git a/webrtc/system_wrappers/include/metrics.h b/webrtc/system_wrappers/include/metrics.h index 5e8ca11e91..4cd74c5e84 100644 --- a/webrtc/system_wrappers/include/metrics.h +++ b/webrtc/system_wrappers/include/metrics.h @@ -13,6 +13,8 @@ #include <string> +#include "webrtc/base/atomicops.h" +#include "webrtc/base/checks.h" #include "webrtc/common_types.h" // Macros for allowing WebRTC clients (e.g. Chrome) to gather and aggregate @@ -58,17 +60,29 @@ // Macros for adding samples to a named histogram. -// -// NOTE: this is a temporary solution. -// The aim is to mimic the behaviour in Chromium's src/base/metrics/histograms.h -// However as atomics are not supported in webrtc, this is for now a modified -// and temporary solution. Note that the histogram is constructed/found for -// each call. Therefore, for now only use this implementation for metrics -// that do not need to be updated frequently. -// TODO(asapersson): Change implementation when atomics are supported. -// Also consider changing string to const char* when switching to atomics. -// Histogram for counters. +// Histogram for counters (exponentially spaced buckets). +#define RTC_HISTOGRAM_COUNTS_100(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 100, 50) + +#define RTC_HISTOGRAM_COUNTS_200(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 200, 50) + +#define RTC_HISTOGRAM_COUNTS_1000(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 1000, 50) + +#define RTC_HISTOGRAM_COUNTS_10000(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 10000, 50) + +#define RTC_HISTOGRAM_COUNTS_100000(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 100000, 50) + +#define RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count) \ + RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \ + webrtc::metrics::HistogramFactoryGetCounts(name, min, max, bucket_count)) + +// Deprecated. +// TODO(asapersson): Remove. #define RTC_HISTOGRAM_COUNTS_SPARSE_100(name, sample) \ RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 100, 50) @@ -86,26 +100,59 @@ #define RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, min, max, bucket_count) \ RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, \ - webrtc::metrics::HistogramFactoryGetCounts( \ - name, min, max, bucket_count)) + webrtc::metrics::HistogramFactoryGetCounts(name, min, max, bucket_count)) + +// Histogram for percentage (evenly spaced buckets). +#define RTC_HISTOGRAM_PERCENTAGE(name, sample) \ + RTC_HISTOGRAM_ENUMERATION(name, sample, 101) -// Histogram for percentage. +// Deprecated. +// TODO(asapersson): Remove. #define RTC_HISTOGRAM_PERCENTAGE_SPARSE(name, sample) \ RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, 101) -// Histogram for enumerators. +// Histogram for enumerators (evenly spaced buckets). // |boundary| should be above the max enumerator sample. +#define RTC_HISTOGRAM_ENUMERATION(name, sample, boundary) \ + RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \ + webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary)) + +// Deprecated. +// TODO(asapersson): Remove. #define RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, boundary) \ RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, \ webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary)) -#define RTC_HISTOGRAM_COMMON_BLOCK_SLOW(constant_name, sample, \ - factory_get_invocation) \ +// 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 { \ - webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \ + 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); \ } while (0) +// Deprecated. +// The histogram is constructed/found for each call. +// 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); \ + } while (0) + namespace webrtc { namespace metrics { diff --git a/webrtc/system_wrappers/source/metrics_unittest.cc b/webrtc/system_wrappers/source/metrics_unittest.cc new file mode 100644 index 0000000000..8319b78ee0 --- /dev/null +++ b/webrtc/system_wrappers/source/metrics_unittest.cc @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2015 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 "testing/gtest/include/gtest/gtest.h" + +#include "webrtc/system_wrappers/include/metrics.h" +#include "webrtc/test/histogram.h" + +namespace webrtc { +namespace { +const int kSample = 22; +const std::string kName = "Name"; + +void AddSparseSample(const std::string& name, int sample) { + RTC_HISTOGRAM_COUNTS_SPARSE_100(name, sample); +} +#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +void AddSample(const std::string& name, int sample) { + RTC_HISTOGRAM_COUNTS_100(name, sample); +} +#endif // GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +} // namespace + +TEST(MetricsTest, InitiallyNoSamples) { + test::ClearHistograms(); + EXPECT_EQ(0, test::NumHistogramSamples(kName)); + EXPECT_EQ(-1, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramPercent_AddSample) { + test::ClearHistograms(); + RTC_HISTOGRAM_PERCENTAGE(kName, kSample); + EXPECT_EQ(1, test::NumHistogramSamples(kName)); + EXPECT_EQ(kSample, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramEnumeration_AddSample) { + test::ClearHistograms(); + RTC_HISTOGRAM_ENUMERATION(kName, kSample, kSample + 1); + EXPECT_EQ(1, test::NumHistogramSamples(kName)); + EXPECT_EQ(kSample, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramCountsSparse_AddSample) { + test::ClearHistograms(); + RTC_HISTOGRAM_COUNTS_SPARSE_100(kName, kSample); + EXPECT_EQ(1, test::NumHistogramSamples(kName)); + EXPECT_EQ(kSample, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramCounts_AddSample) { + test::ClearHistograms(); + RTC_HISTOGRAM_COUNTS_100(kName, kSample); + EXPECT_EQ(1, test::NumHistogramSamples(kName)); + EXPECT_EQ(kSample, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramCounts_AddMultipleSamples) { + test::ClearHistograms(); + const int kNumSamples = 10; + for (int i = 0; i < kNumSamples; ++i) { + RTC_HISTOGRAM_COUNTS_100(kName, i); + } + EXPECT_EQ(kNumSamples, test::NumHistogramSamples(kName)); + EXPECT_EQ(kNumSamples - 1, test::LastHistogramSample(kName)); +} + +TEST(MetricsTest, RtcHistogramSparse_NonConstantNameWorks) { + test::ClearHistograms(); + AddSparseSample("Name1", kSample); + AddSparseSample("Name2", kSample); + EXPECT_EQ(1, test::NumHistogramSamples("Name1")); + EXPECT_EQ(1, test::NumHistogramSamples("Name2")); +} + +#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +TEST(MetricsTest, RtcHistogram_FailsForNonConstantName) { + test::ClearHistograms(); + AddSample("Name1", kSample); + EXPECT_DEATH(AddSample("Name2", kSample), ""); +} +#endif // GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) + +} // namespace webrtc diff --git a/webrtc/system_wrappers/system_wrappers_tests.gyp b/webrtc/system_wrappers/system_wrappers_tests.gyp index f668dfa3ed..a0ae14d6cf 100644 --- a/webrtc/system_wrappers/system_wrappers_tests.gyp +++ b/webrtc/system_wrappers/system_wrappers_tests.gyp @@ -15,6 +15,7 @@ 'dependencies': [ '<(DEPTH)/testing/gtest.gyp:gtest', '<(webrtc_root)/system_wrappers/system_wrappers.gyp:system_wrappers', + '<(webrtc_root)/test/test.gyp:histogram', '<(webrtc_root)/test/test.gyp:test_support_main', ], 'sources': [ @@ -29,6 +30,7 @@ 'source/data_log_helpers_unittest.cc', 'source/data_log_c_helpers_unittest.c', 'source/data_log_c_helpers_unittest.h', + 'source/metrics_unittest.cc', 'source/ntp_time_unittest.cc', 'source/rtp_to_ntp_unittest.cc', 'source/scoped_vector_unittest.cc', diff --git a/webrtc/test/histogram.cc b/webrtc/test/histogram.cc index 6fcdb6864f..2893e4389a 100644 --- a/webrtc/test/histogram.cc +++ b/webrtc/test/histogram.cc @@ -12,6 +12,7 @@ #include <map> +#include "webrtc/base/checks.h" #include "webrtc/base/criticalsection.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/system_wrappers/include/metrics.h" @@ -22,10 +23,10 @@ namespace webrtc { namespace { struct SampleInfo { - SampleInfo(int sample) - : last(sample), total(1) {} - int last; // Last added sample. - int total; // Total number of added samples. + SampleInfo(const std::string& name) : name_(name), last_(-1), total_(0) {} + const std::string name_; + int last_; // Last added sample. + int total_; // Total number of added samples. }; rtc::CriticalSection histogram_crit_; @@ -36,21 +37,33 @@ std::map<std::string, SampleInfo> histograms_ GUARDED_BY(histogram_crit_); namespace metrics { Histogram* HistogramFactoryGetCounts(const std::string& name, int min, int max, - int bucket_count) { return NULL; } + int bucket_count) { + rtc::CritScope cs(&histogram_crit_); + if (histograms_.find(name) == histograms_.end()) { + histograms_.insert(std::make_pair(name, SampleInfo(name))); + } + auto it = histograms_.find(name); + return reinterpret_cast<Histogram*>(&it->second); +} Histogram* HistogramFactoryGetEnumeration(const std::string& name, - int boundary) { return NULL; } + int boundary) { + rtc::CritScope cs(&histogram_crit_); + if (histograms_.find(name) == histograms_.end()) { + histograms_.insert(std::make_pair(name, SampleInfo(name))); + } + auto it = histograms_.find(name); + return reinterpret_cast<Histogram*>(&it->second); +} void HistogramAdd( Histogram* histogram_pointer, const std::string& name, int sample) { rtc::CritScope cs(&histogram_crit_); - auto it = histograms_.find(name); - if (it == histograms_.end()) { - histograms_.insert(std::make_pair(name, SampleInfo(sample))); - return; - } - it->second.last = sample; - ++it->second.total; + SampleInfo* ptr = reinterpret_cast<SampleInfo*>(histogram_pointer); + // The name should not vary. + RTC_CHECK(ptr->name_ == name); + ptr->last_ = sample; + ++ptr->total_; } } // namespace metrics @@ -61,7 +74,7 @@ int LastHistogramSample(const std::string& name) { if (it == histograms_.end()) { return -1; } - return it->second.last; + return it->second.last_; } int NumHistogramSamples(const std::string& name) { @@ -70,13 +83,15 @@ int NumHistogramSamples(const std::string& name) { if (it == histograms_.end()) { return 0; } - return it->second.total; + return it->second.total_; } void ClearHistograms() { rtc::CritScope cs(&histogram_crit_); - histograms_.clear(); + for (auto& it : histograms_) { + it.second.last_ = -1; + it.second.total_ = 0; + } } } // namespace test } // namespace webrtc - diff --git a/webrtc/test/histogram.h b/webrtc/test/histogram.h index 44ce32b4f4..3c8e743aa1 100644 --- a/webrtc/test/histogram.h +++ b/webrtc/test/histogram.h @@ -23,7 +23,7 @@ int LastHistogramSample(const std::string& name); // Returns the number of added samples to a histogram. int NumHistogramSamples(const std::string& name); -// Removes all histograms. +// Removes all histogram samples. void ClearHistograms(); } // namespace test |