From 580d367b1482b2472f6c220a5c30d3942524f36c Mon Sep 17 00:00:00 2001 From: "asapersson@webrtc.org" Date: Thu, 23 Oct 2014 12:57:56 +0000 Subject: [PATCH] Add macros and APIs for webrtc histograms. BUG=crbug/419657 Code that links system_wrappers.gyp:system_wrappers should either: - provide implementations for the APIs, or - link with default implementations in system_wrappers.gyp:system_wrappers_default. R=andresp@webrtc.org, kjellander@webrtc.org, mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/22809004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7508 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/libjingle.gyp | 2 +- .../main/acm2/audio_coding_module.gypi | 4 +- .../audio_coding/neteq/neteq_tests.gypi | 2 +- .../remote_bitrate_estimator.gypi | 4 +- .../codecs/tools/video_codecs_tools.gypi | 2 +- .../main/source/video_coding_test.gypi | 2 +- webrtc/system_wrappers/BUILD.gn | 37 ++++++ webrtc/system_wrappers/interface/metrics.h | 116 ++++++++++++++++++ .../system_wrappers/source/metrics_default.cc | 29 +++++ .../source/system_wrappers.gyp | 17 +++ webrtc/test/BUILD.gn | 1 + webrtc/test/test.gyp | 2 + webrtc/tools/tools.gyp | 4 +- .../test/auto_test/vie_auto_test.gypi | 2 +- webrtc/voice_engine/voice_engine.gyp | 4 +- webrtc/webrtc_examples.gyp | 2 +- webrtc/webrtc_tests.gypi | 4 +- 17 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 webrtc/system_wrappers/interface/metrics.h create mode 100644 webrtc/system_wrappers/source/metrics_default.cc diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp index 335a78891b..c552b22447 100755 --- a/talk/libjingle.gyp +++ b/talk/libjingle.gyp @@ -401,7 +401,7 @@ '<(webrtc_root)/voice_engine/voice_engine.gyp:voice_engine', '<(webrtc_root)/sound/sound.gyp:rtc_sound', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/libjingle/xmllite/xmllite.gyp:rtc_xmllite', 'libjingle', ], diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module.gypi b/webrtc/modules/audio_coding/main/acm2/audio_coding_module.gypi index d746a80b03..8dfdb5638c 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module.gypi +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module.gypi @@ -162,7 +162,7 @@ '<(DEPTH)/testing/gtest.gyp:gtest', '<(webrtc_root)/test/test.gyp:test_support', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', ], 'sources': [ @@ -180,7 +180,7 @@ '<(DEPTH)/testing/gtest.gyp:gtest', '<(webrtc_root)/test/test.gyp:test_support', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', ], 'sources': [ diff --git a/webrtc/modules/audio_coding/neteq/neteq_tests.gypi b/webrtc/modules/audio_coding/neteq/neteq_tests.gypi index d134dcd49d..48cd9ebef0 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_tests.gypi +++ b/webrtc/modules/audio_coding/neteq/neteq_tests.gypi @@ -87,7 +87,7 @@ 'neteq_unittest_tools', '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', ], 'sources': [ 'tools/rtp_analyze.cc', diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi index f290a34c53..f88a16a254 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi @@ -45,7 +45,7 @@ ], 'dependencies': [ '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', 'bwe_tools_util', 'rtp_rtcp', ], @@ -68,7 +68,7 @@ ], 'dependencies': [ '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', 'bwe_tools_util', 'rtp_rtcp', ], diff --git a/webrtc/modules/video_coding/codecs/tools/video_codecs_tools.gypi b/webrtc/modules/video_coding/codecs/tools/video_codecs_tools.gypi index 8f15b28504..3e77a5be11 100644 --- a/webrtc/modules/video_coding/codecs/tools/video_codecs_tools.gypi +++ b/webrtc/modules/video_coding/codecs/tools/video_codecs_tools.gypi @@ -17,7 +17,7 @@ 'video_codecs_test_framework', 'webrtc_video_coding', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/test/metrics.gyp:metrics', '<(webrtc_vp8_dir)/vp8.gyp:webrtc_vp8', ], diff --git a/webrtc/modules/video_coding/main/source/video_coding_test.gypi b/webrtc/modules/video_coding/main/source/video_coding_test.gypi index 64cb6024f2..0c5641d7d9 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_test.gypi +++ b/webrtc/modules/video_coding/main/source/video_coding_test.gypi @@ -20,7 +20,7 @@ '<(webrtc_root)/test/test.gyp:test_support', '<(webrtc_root)/test/metrics.gyp:metrics', '<(webrtc_root)/common_video/common_video.gyp:common_video', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/test/webrtc_test_common.gyp:webrtc_test_common', ], 'sources': [ diff --git a/webrtc/system_wrappers/BUILD.gn b/webrtc/system_wrappers/BUILD.gn index c193c28b77..c4d219269c 100644 --- a/webrtc/system_wrappers/BUILD.gn +++ b/webrtc/system_wrappers/BUILD.gn @@ -35,6 +35,7 @@ static_library("system_wrappers") { "interface/file_wrapper.h", "interface/fix_interlocked_exchange_pointer_win.h", "interface/logging.h", + "interface/metrics.h", "interface/ref_count.h", "interface/rtp_to_ntp.h", "interface/rw_lock_wrapper.h", @@ -216,6 +217,42 @@ source_set("field_trial_default") { ] } +source_set("metrics_default") { + sources = [ + "source/metrics_default.cc", + ] + + configs += [ "..:common_config" ] + public_configs = [ "..:common_inherited_config" ] + + if (is_clang) { + # Suppress warnings from Chrome's Clang plugins. + # See http://code.google.com/p/webrtc/issues/detail?id=163 for details. + configs -= [ "//build/config/clang:find_bad_constructs" ] + } + + deps = [ + ":system_wrappers", + ] +} + +source_set("system_wrappers_default") { + + configs += [ "..:common_config" ] + public_configs = [ "..:common_inherited_config" ] + + if (is_clang) { + # Suppress warnings from Chrome's Clang plugins. + # See http://code.google.com/p/webrtc/issues/detail?id=163 for details. + configs -= [ "//build/config/clang:find_bad_constructs" ] + } + + deps = [ + ":field_trial_default", + ":metrics_default", + ] +} + if (is_android) { source_set("cpu_features_android") { sources = [ diff --git a/webrtc/system_wrappers/interface/metrics.h b/webrtc/system_wrappers/interface/metrics.h new file mode 100644 index 0000000000..be9564a0c2 --- /dev/null +++ b/webrtc/system_wrappers/interface/metrics.h @@ -0,0 +1,116 @@ +// +// Copyright (c) 2014 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. +// + +#ifndef WEBRTC_SYSTEM_WRAPPERS_INTERFACE_METRICS_H_ +#define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_METRICS_H_ + +#include + +#include "webrtc/common_types.h" + +// Macros for allowing WebRTC clients (e.g. Chrome) to gather and aggregate +// statistics. +// +// Histogram for counters. +// RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count); +// +// Histogram for enumerators. +// The boundary should be above the max enumerator sample. +// RTC_HISTOGRAM_ENUMERATION(name, sample, boundary); +// +// +// The macros use the methods HistogramFactoryGetCounts, +// HistogramFactoryGetEnumeration and HistogramAdd. +// +// Therefore, WebRTC clients must either: +// +// - provide implementations of +// Histogram* webrtc::metrics::HistogramFactoryGetCounts( +// const std::string& name, int sample, int min, int max, +// int bucket_count); +// Histogram* webrtc::metrics::HistogramFactoryGetEnumeration( +// const std::string& name, int sample, int boundary); +// void webrtc::metrics::HistogramAdd( +// Histogram* histogram_pointer, const std::string& name, int sample); +// +// - or link with the default implementations (i.e. +// system_wrappers/source/system_wrappers.gyp:metrics_default). +// +// +// Example usage: +// +// RTC_HISTOGRAM_COUNTS("WebRTC.Video.NacksSent", nacks_sent, 1, 100000, 100); +// +// enum Types { +// kTypeX, +// kTypeY, +// kBoundary, +// }; +// +// RTC_HISTOGRAM_ENUMERATION("WebRTC.Types", kTypeX, kBoundary); + + +// 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. +#define RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count) \ + RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \ + webrtc::metrics::HistogramFactoryGetCounts( \ + name, min, max, bucket_count)) + +// Histogram for enumerators. +#define RTC_HISTOGRAM_ENUMERATION(name, sample, boundary) \ + RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \ + webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary)) + +#define RTC_HISTOGRAM_COMMON_BLOCK(constant_name, sample, \ + factory_get_invocation) \ + do { \ + webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \ + webrtc::metrics::HistogramAdd(histogram_pointer, constant_name, sample); \ + } while (0) + + +namespace webrtc { +namespace metrics { + +class Histogram; + +// Functions for getting pointer to histogram (constructs or finds the named +// histogram). + +// Get histogram for counters. +Histogram* HistogramFactoryGetCounts( + const std::string& name, int min, int max, int bucket_count); + +// Get histogram for enumerators. +// |boundary| should be above the max enumerator sample. +Histogram* HistogramFactoryGetEnumeration( + const std::string& name, int boundary); + +// 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); + +} // namespace metrics +} // namespace webrtc + +#endif // WEBRTC_SYSTEM_WRAPPERS_INTERFACE_METRICS_H_ + diff --git a/webrtc/system_wrappers/source/metrics_default.cc b/webrtc/system_wrappers/source/metrics_default.cc new file mode 100644 index 0000000000..af950b4d91 --- /dev/null +++ b/webrtc/system_wrappers/source/metrics_default.cc @@ -0,0 +1,29 @@ +// Copyright (c) 2014 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 "webrtc/system_wrappers/interface/metrics.h" + +// Default implementation of histogram methods for WebRTC clients that do not +// want to provide their own implementation. + +namespace webrtc { +namespace metrics { + +Histogram* HistogramFactoryGetCounts(const std::string& name, int min, int max, + int bucket_count) { return NULL; } + +Histogram* HistogramFactoryGetEnumeration(const std::string& name, + int boundary) { return NULL; } + +void HistogramAdd( + Histogram* histogram_pointer, const std::string& name, int sample) {} + +} // namespace metrics +} // namespace webrtc + diff --git a/webrtc/system_wrappers/source/system_wrappers.gyp b/webrtc/system_wrappers/source/system_wrappers.gyp index 870d88a40e..2cdd23de6f 100644 --- a/webrtc/system_wrappers/source/system_wrappers.gyp +++ b/webrtc/system_wrappers/source/system_wrappers.gyp @@ -44,6 +44,7 @@ '../interface/fix_interlocked_exchange_pointer_win.h', '../interface/logcat_trace_context.h', '../interface/logging.h', + '../interface/metrics.h', '../interface/ref_count.h', '../interface/rtp_to_ntp.h', '../interface/rw_lock_wrapper.h', @@ -204,6 +205,22 @@ 'dependencies': [ 'system_wrappers', ] + }, { + 'target_name': 'metrics_default', + 'type': 'static_library', + 'sources': [ + 'metrics_default.cc', + ], + 'dependencies': [ + 'system_wrappers', + ] + }, { + 'target_name': 'system_wrappers_default', + 'type': 'static_library', + 'dependencies': [ + 'field_trial_default', + 'metrics_default', + ] }, ], # targets 'conditions': [ diff --git a/webrtc/test/BUILD.gn b/webrtc/test/BUILD.gn index 7bb11a14f4..db17c31e44 100644 --- a/webrtc/test/BUILD.gn +++ b/webrtc/test/BUILD.gn @@ -95,6 +95,7 @@ source_set("test_support_main") { deps = [ ":field_trial", ":test_support", + "../system_wrappers:metrics_default", "//testing/gmock", "//testing/gtest", "//third_party/gflags", diff --git a/webrtc/test/test.gyp b/webrtc/test/test.gyp index 154ba2cd5b..92f007bdfc 100644 --- a/webrtc/test/test.gyp +++ b/webrtc/test/test.gyp @@ -84,6 +84,7 @@ 'field_trial', '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:metrics_default', ], }, { @@ -140,6 +141,7 @@ '<(DEPTH)/testing/gmock.gyp:gmock', '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:metrics_default', ], 'sources': [ 'run_all_unittests.cc', diff --git a/webrtc/tools/tools.gyp b/webrtc/tools/tools.gyp index 38a19fc85b..04acac3c23 100644 --- a/webrtc/tools/tools.gyp +++ b/webrtc/tools/tools.gyp @@ -91,7 +91,7 @@ 'type': 'executable', 'dependencies': [ '<(webrtc_root)/voice_engine/voice_engine.gyp:voice_engine', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', ], 'sources': [ 'force_mic_volume_max/force_mic_volume_max.cc', @@ -107,7 +107,7 @@ 'dependencies': [ '<(webrtc_root)/test/test.gyp:channel_transport', '<(webrtc_root)/voice_engine/voice_engine.gyp:voice_engine', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', ], diff --git a/webrtc/video_engine/test/auto_test/vie_auto_test.gypi b/webrtc/video_engine/test/auto_test/vie_auto_test.gypi index a415b3b3a3..1fb7e2f3a1 100644 --- a/webrtc/video_engine/test/auto_test/vie_auto_test.gypi +++ b/webrtc/video_engine/test/auto_test/vie_auto_test.gypi @@ -13,7 +13,7 @@ 'type': 'executable', 'dependencies': [ '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:metrics_default', '<(webrtc_root)/modules/modules.gyp:video_capture_module_internal_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_internal_impl', '<(webrtc_root)/voice_engine/voice_engine.gyp:voice_engine', diff --git a/webrtc/voice_engine/voice_engine.gyp b/webrtc/voice_engine/voice_engine.gyp index deed467a3a..252afde2ce 100644 --- a/webrtc/voice_engine/voice_engine.gyp +++ b/webrtc/voice_engine/voice_engine.gyp @@ -148,7 +148,7 @@ '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/test/test.gyp:channel_transport', '<(webrtc_root)/test/test.gyp:test_support', ], @@ -216,7 +216,7 @@ '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/gflags/gflags.gyp:gflags', '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/test/test.gyp:channel_transport', '<(webrtc_root)/test/test.gyp:test_support', ], diff --git a/webrtc/webrtc_examples.gyp b/webrtc/webrtc_examples.gyp index 0c7293f8e8..4d63110021 100644 --- a/webrtc/webrtc_examples.gyp +++ b/webrtc/webrtc_examples.gyp @@ -18,7 +18,7 @@ '<(DEPTH)/third_party/icu/icu.gyp:icuuc', '<(webrtc_root)/modules/modules.gyp:video_capture_module_internal_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_internal_impl', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', '<(webrtc_root)/test/test.gyp:channel_transport', '<(webrtc_root)/video_engine/video_engine.gyp:video_engine_core', '<(webrtc_root)/voice_engine/voice_engine.gyp:voice_engine', diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi index aca0a6946f..847feffd0d 100644 --- a/webrtc/webrtc_tests.gypi +++ b/webrtc/webrtc_tests.gypi @@ -54,7 +54,7 @@ 'test/webrtc_test_common.gyp:webrtc_test_renderer', '<(webrtc_root)/modules/modules.gyp:video_capture_module_internal_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_impl', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', 'webrtc', ], }, @@ -81,7 +81,7 @@ 'test/webrtc_test_common.gyp:webrtc_test_renderer', '<(webrtc_root)/modules/modules.gyp:video_capture_module_impl', '<(webrtc_root)/modules/modules.gyp:video_render_module_impl', - '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:field_trial_default', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers_default', 'webrtc', ], },