From 36e0f57de3e540c00b3ab63adfc46d3a79ed9746 Mon Sep 17 00:00:00 2001 From: Qingsi Wang Date: Wed, 16 Jan 2019 18:37:21 -0800 Subject: [PATCH] Replace the usage of RTC_HISTOGRAM_COMMON_BLOCK with RTC_HISTOGRAM_COMMON_BLOCK_SLOW. The macro RTC_HISTOGRAM_COMMON_BLOCK uses a static variable for caching, which causes its usage to behave unlike that of a regular method, introducing subtble bugs. For example, if this macro is used inside a member function to log an instance-dependent metric, all instances of the same class will share the same cached histogram handle and thus incorrect reports. Before we move away from these macro-based implementation for metrics, we should avoid using the non-slow macro unless the caching in logging metrics become performance-critical. Bug: webrtc:10188, chromium:921023 Change-Id: I8835978498f3e67c5d97580fc916792f38817ff5 Reviewed-on: https://webrtc-review.googlesource.com/c/118022 Commit-Queue: Minyue Li Reviewed-by: Minyue Li Reviewed-by: Niels Moller Reviewed-by: Henrik Lundin Cr-Commit-Position: refs/heads/master@{#26334} --- system_wrappers/include/metrics.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/system_wrappers/include/metrics.h b/system_wrappers/include/metrics.h index 6b29018552..bad0f6592f 100644 --- a/system_wrappers/include/metrics.h +++ b/system_wrappers/include/metrics.h @@ -134,7 +134,7 @@ // TODO(qingsi): Refactor the default implementation given by RtcHistogram, // which is already sparse, and remove the boundary argument from the macro. #define RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, boundary) \ - RTC_HISTOGRAM_COMMON_BLOCK( \ + RTC_HISTOGRAM_COMMON_BLOCK_SLOW( \ name, sample, \ webrtc::metrics::SparseHistogramFactoryGetEnumeration(name, boundary)) @@ -149,7 +149,7 @@ // 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( \ + RTC_HISTOGRAM_COMMON_BLOCK_SLOW( \ name, sample, \ webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary)) @@ -176,7 +176,6 @@ } \ } 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) \