Skip to content

Commit

Permalink
Add LOCAL_ prefix to non-UMA histogram macros.
Browse files Browse the repository at this point in the history
This makes it harder to accidently use the wrong
macro.

Also, removes the D* variants of the macros and
associated DebugNow() function. These were rarely
used and removing them removes clutter from the
header file. Existing uses converted to be behind
NDEBUG ifdefs.

No functional changes except for a fix to the
code in content_based_thumbnailing_algorithm.cc
which was incorrectly using a ternary operator for
the histogram name (which doesn't work since the
macros cache the histogram object) and removal
of local histograms Spellcheck.SuggestTime and
Spellcheck.InitTime per groby@.

Since this is an API rename, TBR'ing downstream
owners.

BUG=311349
TBR=groby@chromium.org,zea@chromium.org,jeremy@chromium.org,reveman@chromium.org,agl@chromium.org,jam@chromium.org

Review URL: https://codereview.chromium.org/484603006

Cr-Commit-Position: refs/heads/master@{#291840}
  • Loading branch information
asvitkine-chromium authored and Commit bot committed Aug 26, 2014
1 parent 663b9f4 commit c0fb802
Show file tree
Hide file tree
Showing 39 changed files with 130 additions and 261 deletions.
2 changes: 1 addition & 1 deletion base/metrics/field_trial.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
// the impact of some pruning algorithm.
// We assume that we already have a histogram of memory usage, such as:

// HISTOGRAM_COUNTS("Memory.RendererTotal", count);
// UMA_HISTOGRAM_COUNTS("Memory.RendererTotal", count);

// Somewhere in main thread initialization code, we'd probably define an
// instance of a FieldTrial, with code such as:
Expand Down
8 changes: 0 additions & 8 deletions base/metrics/histogram.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,6 @@ HistogramBase* Histogram::FactoryTimeGet(const string& name,
bucket_count, flags);
}

TimeTicks Histogram::DebugNow() {
#ifndef NDEBUG
return TimeTicks::Now();
#else
return TimeTicks();
#endif
}

// Calculate what range of values are held in each bucket.
// We have to be careful that we don't pick a ratio between starting points in
// consecutive buckets that is sooo small, that the integer bounds are the same
Expand Down
103 changes: 16 additions & 87 deletions base/metrics/histogram.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Lock;
// The following code is generally what a thread-safe static pointer
// initialization looks like for a histogram (after a macro is expanded). This
// sample is an expansion (with comments) of the code for
// HISTOGRAM_CUSTOM_COUNTS().
// LOCAL_HISTOGRAM_CUSTOM_COUNTS().

/*
do {
Expand Down Expand Up @@ -165,27 +165,27 @@ class Lock;
// Provide easy general purpose histogram in a macro, just like stats counters.
// The first four macros use 50 buckets.

#define HISTOGRAM_TIMES(name, sample) HISTOGRAM_CUSTOM_TIMES( \
#define LOCAL_HISTOGRAM_TIMES(name, sample) LOCAL_HISTOGRAM_CUSTOM_TIMES( \
name, sample, base::TimeDelta::FromMilliseconds(1), \
base::TimeDelta::FromSeconds(10), 50)

// For folks that need real specific times, use this to select a precise range
// of times you want plotted, and the number of buckets you want used.
#define HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
#define LOCAL_HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
STATIC_HISTOGRAM_POINTER_BLOCK(name, AddTime(sample), \
base::Histogram::FactoryTimeGet(name, min, max, bucket_count, \
base::HistogramBase::kNoFlags))

#define HISTOGRAM_COUNTS(name, sample) HISTOGRAM_CUSTOM_COUNTS( \
#define LOCAL_HISTOGRAM_COUNTS(name, sample) LOCAL_HISTOGRAM_CUSTOM_COUNTS( \
name, sample, 1, 1000000, 50)

#define HISTOGRAM_COUNTS_100(name, sample) HISTOGRAM_CUSTOM_COUNTS( \
name, sample, 1, 100, 50)
#define LOCAL_HISTOGRAM_COUNTS_100(name, sample) \
LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 100, 50)

#define HISTOGRAM_COUNTS_10000(name, sample) HISTOGRAM_CUSTOM_COUNTS( \
name, sample, 1, 10000, 50)
#define LOCAL_HISTOGRAM_COUNTS_10000(name, sample) \
LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 10000, 50)

#define HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
#define LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
base::Histogram::FactoryGet(name, min, max, bucket_count, \
base::HistogramBase::kNoFlags))
Expand All @@ -196,10 +196,10 @@ class Lock;
base::LinearHistogram::FactoryGet(name, 1, boundary, boundary + 1, \
flag))

#define HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)
#define LOCAL_HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
LOCAL_HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)

#define HISTOGRAM_BOOLEAN(name, sample) \
#define LOCAL_HISTOGRAM_BOOLEAN(name, sample) \
STATIC_HISTOGRAM_POINTER_BLOCK(name, AddBoolean(sample), \
base::BooleanHistogram::FactoryGet(name, base::Histogram::kNoFlags))

Expand All @@ -209,7 +209,7 @@ class Lock;
// also that, despite explicitly setting the minimum bucket value to |1| below,
// it is fine for enumerated histograms to be 0-indexed -- this is because
// enumerated histograms should never have underflow.
#define HISTOGRAM_ENUMERATION(name, sample, boundary_value) \
#define LOCAL_HISTOGRAM_ENUMERATION(name, sample, boundary_value) \
STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
base::LinearHistogram::FactoryGet(name, 1, boundary_value, \
boundary_value + 1, base::HistogramBase::kNoFlags))
Expand All @@ -219,81 +219,14 @@ class Lock;
// CustomRanges::FactoryGet about the requirement of |custom_ranges|.
// You can use the helper function CustomHistogram::ArrayToCustomRanges to
// transform a C-style array of valid sample values to a std::vector<int>.
#define HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
#define LOCAL_HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
base::CustomHistogram::FactoryGet(name, custom_ranges, \
base::HistogramBase::kNoFlags))

#define HISTOGRAM_MEMORY_KB(name, sample) HISTOGRAM_CUSTOM_COUNTS( \
#define LOCAL_HISTOGRAM_MEMORY_KB(name, sample) LOCAL_HISTOGRAM_CUSTOM_COUNTS( \
name, sample, 1000, 500000, 50)

//------------------------------------------------------------------------------
// Define Debug vs non-debug flavors of macros.
#ifndef NDEBUG

#define DHISTOGRAM_TIMES(name, sample) HISTOGRAM_TIMES(name, sample)
#define DHISTOGRAM_COUNTS(name, sample) HISTOGRAM_COUNTS(name, sample)
#define DHISTOGRAM_PERCENTAGE(name, under_one_hundred) HISTOGRAM_PERCENTAGE(\
name, under_one_hundred)
#define DHISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count)
#define DHISTOGRAM_CLIPPED_TIMES(name, sample, min, max, bucket_count) \
HISTOGRAM_CLIPPED_TIMES(name, sample, min, max, bucket_count)
#define DHISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count)
#define DHISTOGRAM_ENUMERATION(name, sample, boundary_value) \
HISTOGRAM_ENUMERATION(name, sample, boundary_value)
#define DHISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges)

#else // NDEBUG
// Keep a mention of passed variables to avoid unused variable warnings in
// release build if these variables are only used in macros.
#define DISCARD_2_ARGUMENTS(a, b) \
while (0) { \
static_cast<void>(a); \
static_cast<void>(b); \
}
#define DISCARD_3_ARGUMENTS(a, b, c) \
while (0) { \
static_cast<void>(a); \
static_cast<void>(b); \
static_cast<void>(c); \
}
#define DISCARD_5_ARGUMENTS(a, b, c, d ,e) \
while (0) { \
static_cast<void>(a); \
static_cast<void>(b); \
static_cast<void>(c); \
static_cast<void>(d); \
static_cast<void>(e); \
}
#define DHISTOGRAM_TIMES(name, sample) \
DISCARD_2_ARGUMENTS(name, sample)

#define DHISTOGRAM_COUNTS(name, sample) \
DISCARD_2_ARGUMENTS(name, sample)

#define DHISTOGRAM_PERCENTAGE(name, under_one_hundred) \
DISCARD_2_ARGUMENTS(name, under_one_hundred)

#define DHISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
DISCARD_5_ARGUMENTS(name, sample, min, max, bucket_count)

#define DHISTOGRAM_CLIPPED_TIMES(name, sample, min, max, bucket_count) \
DISCARD_5_ARGUMENTS(name, sample, min, max, bucket_count)

#define DHISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
DISCARD_5_ARGUMENTS(name, sample, min, max, bucket_count)

#define DHISTOGRAM_ENUMERATION(name, sample, boundary_value) \
DISCARD_3_ARGUMENTS(name, sample, boundary_value)

#define DHISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
DISCARD_3_ARGUMENTS(name, sample, custom_ranges)

#endif // NDEBUG

//------------------------------------------------------------------------------
// The following macros provide typical usage scenarios for callers that wish
// to record histogram data, and have the data submitted/uploaded via UMA.
Expand Down Expand Up @@ -408,10 +341,6 @@ class BASE_EXPORT Histogram : public HistogramBase {
size_t bucket_count,
int32 flags);

// Time call for use with DHISTOGRAM*.
// Returns TimeTicks::Now() in debug and TimeTicks() in release build.
static TimeTicks DebugNow();

static void InitializeBucketRanges(Sample minimum,
Sample maximum,
BucketRanges* ranges);
Expand Down Expand Up @@ -660,7 +589,7 @@ class BASE_EXPORT CustomHistogram : public Histogram {
virtual HistogramType GetHistogramType() const OVERRIDE;

// Helper method for transforming an array of valid enumeration values
// to the std::vector<int> expected by HISTOGRAM_CUSTOM_ENUMERATION.
// to the std::vector<int> expected by UMA_HISTOGRAM_CUSTOM_ENUMERATION.
// This function ensures that a guard bucket exists right after any
// valid sample value (unless the next higher sample is also a valid value),
// so that invalid samples never fall into the same bucket as valid samples.
Expand Down
13 changes: 5 additions & 8 deletions base/metrics/histogram_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,17 @@ TEST_F(HistogramTest, BasicTest) {
EXPECT_TRUE(custom_histogram);

// Use standard macros (but with fixed samples)
HISTOGRAM_TIMES("Test2Histogram", TimeDelta::FromDays(1));
HISTOGRAM_COUNTS("Test3Histogram", 30);
LOCAL_HISTOGRAM_TIMES("Test2Histogram", TimeDelta::FromDays(1));
LOCAL_HISTOGRAM_COUNTS("Test3Histogram", 30);

DHISTOGRAM_TIMES("Test4Histogram", TimeDelta::FromDays(1));
DHISTOGRAM_COUNTS("Test5Histogram", 30);

HISTOGRAM_ENUMERATION("Test6Histogram", 129, 130);
LOCAL_HISTOGRAM_ENUMERATION("Test6Histogram", 129, 130);
}

// Check that the macro correctly matches histograms by name and records their
// data together.
TEST_F(HistogramTest, NameMatchTest) {
HISTOGRAM_PERCENTAGE("DuplicatedHistogram", 10);
HISTOGRAM_PERCENTAGE("DuplicatedHistogram", 10);
LOCAL_HISTOGRAM_PERCENTAGE("DuplicatedHistogram", 10);
LOCAL_HISTOGRAM_PERCENTAGE("DuplicatedHistogram", 10);
HistogramBase* histogram = LinearHistogram::FactoryGet(
"DuplicatedHistogram", 1, 101, 102, HistogramBase::kNoFlags);

Expand Down
32 changes: 3 additions & 29 deletions base/metrics/sparse_histogram.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,13 @@

namespace base {

// The common code for different SparseHistogram macros.
#define HISTOGRAM_SPARSE_COMMON(name, sample, flag) \
#define UMA_HISTOGRAM_SPARSE_SLOWLY(name, sample) \
do { \
base::HistogramBase* histogram( \
base::SparseHistogram::FactoryGet(name, flag)); \
DCHECK_EQ(histogram->histogram_name(), name); \
base::HistogramBase* histogram = base::SparseHistogram::FactoryGet( \
name, base::HistogramBase::kUmaTargetedHistogramFlag); \
histogram->Add(sample); \
} while (0)

#define HISTOGRAM_SPARSE_SLOWLY(name, sample) \
HISTOGRAM_SPARSE_COMMON(name, sample, base::HistogramBase::kNoFlags)

#define UMA_HISTOGRAM_SPARSE_SLOWLY(name, sample) \
HISTOGRAM_SPARSE_COMMON(name, sample, \
base::HistogramBase::kUmaTargetedHistogramFlag)

//------------------------------------------------------------------------------
// Define debug only version of macros.
#ifndef NDEBUG

#define DHISTOGRAM_SPARSE_SLOWLY(name, sample) \
HISTOGRAM_SPARSE_SLOWLY(name, sample)

#else // NDEBUG

#define DHISTOGRAM_SPARSE_SLOWLY(name, sample) \
while (0) { \
static_cast<void>(name); \
static_cast<void>(sample); \
}

#endif // NDEBUG

class HistogramSamples;

class BASE_EXPORT_PRIVATE SparseHistogram : public HistogramBase {
Expand Down
23 changes: 5 additions & 18 deletions base/metrics/sparse_histogram_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ TEST_F(SparseHistogramTest, BasicTest) {
}

TEST_F(SparseHistogramTest, MacroBasicTest) {
HISTOGRAM_SPARSE_SLOWLY("Sparse", 100);
HISTOGRAM_SPARSE_SLOWLY("Sparse", 200);
HISTOGRAM_SPARSE_SLOWLY("Sparse", 100);
UMA_HISTOGRAM_SPARSE_SLOWLY("Sparse", 100);
UMA_HISTOGRAM_SPARSE_SLOWLY("Sparse", 200);
UMA_HISTOGRAM_SPARSE_SLOWLY("Sparse", 100);

StatisticsRecorder::Histograms histograms;
StatisticsRecorder::GetHistograms(&histograms);
Expand All @@ -76,28 +76,15 @@ TEST_F(SparseHistogramTest, MacroBasicTest) {

EXPECT_EQ(SPARSE_HISTOGRAM, sparse_histogram->GetHistogramType());
EXPECT_EQ("Sparse", sparse_histogram->histogram_name());
EXPECT_EQ(HistogramBase::kNoFlags, sparse_histogram->flags());
EXPECT_EQ(HistogramBase::kUmaTargetedHistogramFlag,
sparse_histogram->flags());

scoped_ptr<HistogramSamples> samples = sparse_histogram->SnapshotSamples();
EXPECT_EQ(3, samples->TotalCount());
EXPECT_EQ(2, samples->GetCount(100));
EXPECT_EQ(1, samples->GetCount(200));
}

TEST_F(SparseHistogramTest, MacroUmaTest) {
UMA_HISTOGRAM_SPARSE_SLOWLY("Uma", 100);

StatisticsRecorder::Histograms histograms;
StatisticsRecorder::GetHistograms(&histograms);

ASSERT_EQ(1U, histograms.size());
HistogramBase* sparse_histogram = histograms[0];

EXPECT_EQ("Uma", sparse_histogram->histogram_name());
EXPECT_EQ(HistogramBase::kUmaTargetedHistogramFlag,
sparse_histogram->flags());
}

TEST_F(SparseHistogramTest, MacroInLoopTest) {
// Unlike the macros in histogram.h, SparseHistogram macros can have a
// variable as histogram name.
Expand Down
25 changes: 7 additions & 18 deletions base/metrics/statistics_recorder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,29 +222,18 @@ TEST_F(StatisticsRecorderTest, RegisterHistogramWithMacros) {
"TestHistogramCounts", 1, 1000000, 50, HistogramBase::kNoFlags);

// The histogram we got from macro is the same as from FactoryGet.
HISTOGRAM_COUNTS("TestHistogramCounts", 30);
LOCAL_HISTOGRAM_COUNTS("TestHistogramCounts", 30);
registered_histograms.clear();
StatisticsRecorder::GetHistograms(&registered_histograms);
ASSERT_EQ(1u, registered_histograms.size());
EXPECT_EQ(histogram, registered_histograms[0]);

HISTOGRAM_TIMES("TestHistogramTimes", TimeDelta::FromDays(1));
HISTOGRAM_ENUMERATION("TestHistogramEnumeration", 20, 200);
LOCAL_HISTOGRAM_TIMES("TestHistogramTimes", TimeDelta::FromDays(1));
LOCAL_HISTOGRAM_ENUMERATION("TestHistogramEnumeration", 20, 200);

registered_histograms.clear();
StatisticsRecorder::GetHistograms(&registered_histograms);
EXPECT_EQ(3u, registered_histograms.size());

// Debugging only macros.
DHISTOGRAM_TIMES("TestHistogramDebugTimes", TimeDelta::FromDays(1));
DHISTOGRAM_COUNTS("TestHistogramDebugCounts", 30);
registered_histograms.clear();
StatisticsRecorder::GetHistograms(&registered_histograms);
#ifndef NDEBUG
EXPECT_EQ(5u, registered_histograms.size());
#else
EXPECT_EQ(3u, registered_histograms.size());
#endif
}

TEST_F(StatisticsRecorderTest, BucketRangesSharing) {
Expand All @@ -266,10 +255,10 @@ TEST_F(StatisticsRecorderTest, BucketRangesSharing) {
}

TEST_F(StatisticsRecorderTest, ToJSON) {
HISTOGRAM_COUNTS("TestHistogram1", 30);
HISTOGRAM_COUNTS("TestHistogram1", 40);
HISTOGRAM_COUNTS("TestHistogram2", 30);
HISTOGRAM_COUNTS("TestHistogram2", 40);
LOCAL_HISTOGRAM_COUNTS("TestHistogram1", 30);
LOCAL_HISTOGRAM_COUNTS("TestHistogram1", 40);
LOCAL_HISTOGRAM_COUNTS("TestHistogram2", 30);
LOCAL_HISTOGRAM_COUNTS("TestHistogram2", 40);

std::string json(StatisticsRecorder::ToJSON(std::string()));

Expand Down
2 changes: 1 addition & 1 deletion cc/resources/tile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class RasterTaskImpl : public RasterTask {
if (rendering_stats_->record_rendering_stats()) {
base::TimeDelta current_rasterize_time =
rendering_stats_->impl_thread_rendering_stats().rasterize_time;
HISTOGRAM_CUSTOM_COUNTS(
LOCAL_HISTOGRAM_CUSTOM_COUNTS(
"Renderer4.PictureRasterTimeUS",
(current_rasterize_time - prev_rasterize_time).InMicroseconds(),
0,
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/chrome_browser_main_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ CatSixtyFour CatSixtyFourValue() {
void RecordCatSixtyFour() {
CatSixtyFour cat_sixty_four = CatSixtyFourValue();

// Set this higher than the highest value in the CatSixtyFour enum to
// provide some headroom and then leave it alone. See HISTOGRAM_ENUMERATION
// in base/metrics/histogram.h.
// Set this higher than the highest value in the CatSixtyFour enum to provide
// some headroom and then leave it alone. See UMA_HISTOGRAM_ENUMERATION in
// base/metrics/histogram.h.
const int kMaxCatsAndSixtyFours = 32;
COMPILE_ASSERT(kMaxCatsAndSixtyFours >= CAT_SIXTY_FOUR_MAX,
CatSixtyFour_enum_grew_too_large);
Expand Down
Loading

0 comments on commit c0fb802

Please sign in to comment.