Commit 25c0e682 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[base] Reduce string copies in HistogramTester

This change reduces string copies in base::HistogramTester by changing
its APIs to take base::StringPieces instead of const std::string&. The
majority of callsites pass string literals to the HistogramTester APIs,
resulting in a deep copy with the old implementation.

Furthermore, this change updates the underlying std::maps to use the
heterogeneous std::less<> comparator, allowing the direct comparison of
base::StringPiece with std::string.

Bug: 384011
Change-Id: I6a8dd3e76280162b6046eac63a3d1ac84ed2ee6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1887612Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710454}
parent 13da6b87
...@@ -30,7 +30,7 @@ HistogramTester::HistogramTester() { ...@@ -30,7 +30,7 @@ HistogramTester::HistogramTester() {
HistogramTester::~HistogramTester() = default; HistogramTester::~HistogramTester() = default;
void HistogramTester::ExpectUniqueSample( void HistogramTester::ExpectUniqueSample(
const std::string& name, StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
HistogramBase::Count expected_count) const { HistogramBase::Count expected_count) const {
HistogramBase* histogram = StatisticsRecorder::FindHistogram(name); HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
...@@ -45,7 +45,7 @@ void HistogramTester::ExpectUniqueSample( ...@@ -45,7 +45,7 @@ void HistogramTester::ExpectUniqueSample(
} }
void HistogramTester::ExpectBucketCount( void HistogramTester::ExpectBucketCount(
const std::string& name, StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
HistogramBase::Count expected_count) const { HistogramBase::Count expected_count) const {
HistogramBase* histogram = StatisticsRecorder::FindHistogram(name); HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
...@@ -58,7 +58,7 @@ void HistogramTester::ExpectBucketCount( ...@@ -58,7 +58,7 @@ void HistogramTester::ExpectBucketCount(
} }
} }
void HistogramTester::ExpectTotalCount(const std::string& name, void HistogramTester::ExpectTotalCount(StringPiece name,
HistogramBase::Count count) const { HistogramBase::Count count) const {
HistogramBase* histogram = StatisticsRecorder::FindHistogram(name); HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
if (histogram) { if (histogram) {
...@@ -70,14 +70,13 @@ void HistogramTester::ExpectTotalCount(const std::string& name, ...@@ -70,14 +70,13 @@ void HistogramTester::ExpectTotalCount(const std::string& name,
} }
} }
void HistogramTester::ExpectTimeBucketCount(const std::string& name, void HistogramTester::ExpectTimeBucketCount(StringPiece name,
TimeDelta sample, TimeDelta sample,
HistogramBase::Count count) const { HistogramBase::Count count) const {
ExpectBucketCount(name, sample.InMilliseconds(), count); ExpectBucketCount(name, sample.InMilliseconds(), count);
} }
std::vector<Bucket> HistogramTester::GetAllSamples( std::vector<Bucket> HistogramTester::GetAllSamples(StringPiece name) const {
const std::string& name) const {
std::vector<Bucket> samples; std::vector<Bucket> samples;
std::unique_ptr<HistogramSamples> snapshot = std::unique_ptr<HistogramSamples> snapshot =
GetHistogramSamplesSinceCreation(name); GetHistogramSamplesSinceCreation(name);
...@@ -93,7 +92,7 @@ std::vector<Bucket> HistogramTester::GetAllSamples( ...@@ -93,7 +92,7 @@ std::vector<Bucket> HistogramTester::GetAllSamples(
} }
HistogramBase::Count HistogramTester::GetBucketCount( HistogramBase::Count HistogramTester::GetBucketCount(
const std::string& name, StringPiece name,
HistogramBase::Sample sample) const { HistogramBase::Sample sample) const {
HistogramBase* histogram = StatisticsRecorder::FindHistogram(name); HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
EXPECT_NE(nullptr, histogram) EXPECT_NE(nullptr, histogram)
...@@ -107,7 +106,7 @@ HistogramBase::Count HistogramTester::GetBucketCount( ...@@ -107,7 +106,7 @@ HistogramBase::Count HistogramTester::GetBucketCount(
} }
void HistogramTester::GetBucketCountForSamples( void HistogramTester::GetBucketCountForSamples(
const std::string& name, StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
const HistogramSamples& samples, const HistogramSamples& samples,
HistogramBase::Count* count) const { HistogramBase::Count* count) const {
...@@ -118,8 +117,8 @@ void HistogramTester::GetBucketCountForSamples( ...@@ -118,8 +117,8 @@ void HistogramTester::GetBucketCountForSamples(
} }
HistogramTester::CountsMap HistogramTester::GetTotalCountsForPrefix( HistogramTester::CountsMap HistogramTester::GetTotalCountsForPrefix(
const std::string& prefix) const { StringPiece prefix) const {
EXPECT_TRUE(prefix.find('.') != std::string::npos) EXPECT_TRUE(prefix.find('.') != StringPiece::npos)
<< "|prefix| ought to contain at least one period, to avoid matching too" << "|prefix| ought to contain at least one period, to avoid matching too"
<< " many histograms."; << " many histograms.";
...@@ -143,7 +142,7 @@ HistogramTester::CountsMap HistogramTester::GetTotalCountsForPrefix( ...@@ -143,7 +142,7 @@ HistogramTester::CountsMap HistogramTester::GetTotalCountsForPrefix(
std::unique_ptr<HistogramSamples> std::unique_ptr<HistogramSamples>
HistogramTester::GetHistogramSamplesSinceCreation( HistogramTester::GetHistogramSamplesSinceCreation(
const std::string& histogram_name) const { StringPiece histogram_name) const {
HistogramBase* histogram = StatisticsRecorder::FindHistogram(histogram_name); HistogramBase* histogram = StatisticsRecorder::FindHistogram(histogram_name);
// Whether the histogram exists or not may not depend on the current test // Whether the histogram exists or not may not depend on the current test
// calling this method, but rather on which tests ran before and possibly // calling this method, but rather on which tests ran before and possibly
...@@ -192,7 +191,7 @@ std::string HistogramTester::GetAllHistogramsRecorded() const { ...@@ -192,7 +191,7 @@ std::string HistogramTester::GetAllHistogramsRecorded() const {
return output; return output;
} }
void HistogramTester::CheckBucketCount(const std::string& name, void HistogramTester::CheckBucketCount(StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
HistogramBase::Count expected_count, HistogramBase::Count expected_count,
const HistogramSamples& samples) const { const HistogramSamples& samples) const {
...@@ -206,7 +205,7 @@ void HistogramTester::CheckBucketCount(const std::string& name, ...@@ -206,7 +205,7 @@ void HistogramTester::CheckBucketCount(const std::string& name,
<< ")."; << ").";
} }
void HistogramTester::CheckTotalCount(const std::string& name, void HistogramTester::CheckTotalCount(StringPiece name,
HistogramBase::Count expected_count, HistogramBase::Count expected_count,
const HistogramSamples& samples) const { const HistogramSamples& samples) const {
int actual_count = samples.TotalCount(); int actual_count = samples.TotalCount();
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef BASE_TEST_METRICS_HISTOGRAM_TESTER_H_ #ifndef BASE_TEST_METRICS_HISTOGRAM_TESTER_H_
#define BASE_TEST_METRICS_HISTOGRAM_TESTER_H_ #define BASE_TEST_METRICS_HISTOGRAM_TESTER_H_
#include <functional>
#include <map> #include <map>
#include <memory> #include <memory>
#include <ostream> #include <ostream>
...@@ -15,6 +16,7 @@ ...@@ -15,6 +16,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/metrics/histogram.h" #include "base/metrics/histogram.h"
#include "base/metrics/histogram_base.h" #include "base/metrics/histogram_base.h"
#include "base/strings/string_piece.h"
#include "base/time/time.h" #include "base/time/time.h"
namespace base { namespace base {
...@@ -33,7 +35,7 @@ class HistogramSamples; ...@@ -33,7 +35,7 @@ class HistogramSamples;
// should be used to achieve that. // should be used to achieve that.
class HistogramTester { class HistogramTester {
public: public:
using CountsMap = std::map<std::string, HistogramBase::Count>; using CountsMap = std::map<std::string, HistogramBase::Count, std::less<>>;
// Takes a snapshot of all current histograms counts. // Takes a snapshot of all current histograms counts.
HistogramTester(); HistogramTester();
...@@ -42,11 +44,11 @@ class HistogramTester { ...@@ -42,11 +44,11 @@ class HistogramTester {
// We know the exact number of samples in a bucket, and that no other bucket // We know the exact number of samples in a bucket, and that no other bucket
// should have samples. Measures the diff from the snapshot taken when this // should have samples. Measures the diff from the snapshot taken when this
// object was constructed. // object was constructed.
void ExpectUniqueSample(const std::string& name, void ExpectUniqueSample(StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
HistogramBase::Count expected_count) const; HistogramBase::Count expected_count) const;
template <typename T> template <typename T>
void ExpectUniqueSample(const std::string& name, void ExpectUniqueSample(StringPiece name,
T sample, T sample,
HistogramBase::Count expected_count) const { HistogramBase::Count expected_count) const {
ExpectUniqueSample(name, static_cast<HistogramBase::Sample>(sample), ExpectUniqueSample(name, static_cast<HistogramBase::Sample>(sample),
...@@ -56,11 +58,11 @@ class HistogramTester { ...@@ -56,11 +58,11 @@ class HistogramTester {
// We know the exact number of samples in a bucket, but other buckets may // We know the exact number of samples in a bucket, but other buckets may
// have samples as well. Measures the diff from the snapshot taken when this // have samples as well. Measures the diff from the snapshot taken when this
// object was constructed. // object was constructed.
void ExpectBucketCount(const std::string& name, void ExpectBucketCount(StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
HistogramBase::Count expected_count) const; HistogramBase::Count expected_count) const;
template <typename T> template <typename T>
void ExpectBucketCount(const std::string& name, void ExpectBucketCount(StringPiece name,
T sample, T sample,
HistogramBase::Count expected_count) const { HistogramBase::Count expected_count) const {
ExpectBucketCount(name, static_cast<HistogramBase::Sample>(sample), ExpectBucketCount(name, static_cast<HistogramBase::Sample>(sample),
...@@ -70,12 +72,11 @@ class HistogramTester { ...@@ -70,12 +72,11 @@ class HistogramTester {
// We don't know the values of the samples, but we know how many there are. // We don't know the values of the samples, but we know how many there are.
// This measures the diff from the snapshot taken when this object was // This measures the diff from the snapshot taken when this object was
// constructed. // constructed.
void ExpectTotalCount(const std::string& name, void ExpectTotalCount(StringPiece name, HistogramBase::Count count) const;
HistogramBase::Count count) const;
// We know exact number of samples for buckets corresponding to a time // We know exact number of samples for buckets corresponding to a time
// interval. Other intervals may have samples too. // interval. Other intervals may have samples too.
void ExpectTimeBucketCount(const std::string& name, void ExpectTimeBucketCount(StringPiece name,
TimeDelta sample, TimeDelta sample,
HistogramBase::Count count) const; HistogramBase::Count count) const;
...@@ -95,10 +96,10 @@ class HistogramTester { ...@@ -95,10 +96,10 @@ class HistogramTester {
// slightly less helpful failure message: // slightly less helpful failure message:
// EXPECT_EQ(expected_buckets, // EXPECT_EQ(expected_buckets,
// histogram_tester.GetAllSamples("HistogramName")); // histogram_tester.GetAllSamples("HistogramName"));
std::vector<Bucket> GetAllSamples(const std::string& name) const; std::vector<Bucket> GetAllSamples(StringPiece name) const;
// Returns the value of the |sample| bucket for ths histogram |name|. // Returns the value of the |sample| bucket for ths histogram |name|.
HistogramBase::Count GetBucketCount(const std::string& name, HistogramBase::Count GetBucketCount(StringPiece name,
HistogramBase::Sample sample) const; HistogramBase::Sample sample) const;
// Finds histograms whose names start with |prefix|, and returns them along // Finds histograms whose names start with |prefix|, and returns them along
...@@ -119,12 +120,12 @@ class HistogramTester { ...@@ -119,12 +120,12 @@ class HistogramTester {
// expected_counts["MyMetric.B"] = 1; // expected_counts["MyMetric.B"] = 1;
// EXPECT_THAT(histogram_tester.GetTotalCountsForPrefix("MyMetric."), // EXPECT_THAT(histogram_tester.GetTotalCountsForPrefix("MyMetric."),
// testing::ContainerEq(expected_counts)); // testing::ContainerEq(expected_counts));
CountsMap GetTotalCountsForPrefix(const std::string& prefix) const; CountsMap GetTotalCountsForPrefix(StringPiece prefix) const;
// Access a modified HistogramSamples containing only what has been logged // Access a modified HistogramSamples containing only what has been logged
// to the histogram since the creation of this object. // to the histogram since the creation of this object.
std::unique_ptr<HistogramSamples> GetHistogramSamplesSinceCreation( std::unique_ptr<HistogramSamples> GetHistogramSamplesSinceCreation(
const std::string& histogram_name) const; StringPiece histogram_name) const;
// Dumps all histograms that have had new samples added to them into a string, // Dumps all histograms that have had new samples added to them into a string,
// for debugging purposes. Note: this will dump the entire contents of any // for debugging purposes. Note: this will dump the entire contents of any
...@@ -135,7 +136,7 @@ class HistogramTester { ...@@ -135,7 +136,7 @@ class HistogramTester {
// Verifies and asserts that value in the |sample| bucket matches the // Verifies and asserts that value in the |sample| bucket matches the
// |expected_count|. The bucket's current value is determined from |samples| // |expected_count|. The bucket's current value is determined from |samples|
// and is modified based on the snapshot stored for histogram |name|. // and is modified based on the snapshot stored for histogram |name|.
void CheckBucketCount(const std::string& name, void CheckBucketCount(StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
Histogram::Count expected_count, Histogram::Count expected_count,
const HistogramSamples& samples) const; const HistogramSamples& samples) const;
...@@ -143,21 +144,22 @@ class HistogramTester { ...@@ -143,21 +144,22 @@ class HistogramTester {
// Verifies that the total number of values recorded for the histogram |name| // Verifies that the total number of values recorded for the histogram |name|
// is |expected_count|. This is checked against |samples| minus the snapshot // is |expected_count|. This is checked against |samples| minus the snapshot
// that was taken for |name|. // that was taken for |name|.
void CheckTotalCount(const std::string& name, void CheckTotalCount(StringPiece name,
Histogram::Count expected_count, Histogram::Count expected_count,
const HistogramSamples& samples) const; const HistogramSamples& samples) const;
// Sets the value for |count| to be the value in the |sample| bucket. The // Sets the value for |count| to be the value in the |sample| bucket. The
// bucket's current value is determined from |samples| and is modified based // bucket's current value is determined from |samples| and is modified based
// on the snapshot stored for histogram |name|. // on the snapshot stored for histogram |name|.
void GetBucketCountForSamples(const std::string& name, void GetBucketCountForSamples(StringPiece name,
HistogramBase::Sample sample, HistogramBase::Sample sample,
const HistogramSamples& samples, const HistogramSamples& samples,
HistogramBase::Count* count) const; HistogramBase::Count* count) const;
// Used to determine the histogram changes made during this instance's // Used to determine the histogram changes made during this instance's
// lifecycle. // lifecycle.
std::map<std::string, std::unique_ptr<HistogramSamples>> histograms_snapshot_; std::map<std::string, std::unique_ptr<HistogramSamples>, std::less<>>
histograms_snapshot_;
DISALLOW_COPY_AND_ASSIGN(HistogramTester); DISALLOW_COPY_AND_ASSIGN(HistogramTester);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment