Consolidate serialization code in base::HistogramDeltasSerializer
Before patch code lived partially in base/ and content/
BUG=305019
Review URL: https://codereview.chromium.org/27460003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230268 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/base/base.gyp b/base/base.gyp
index fb37803..5dfe08dd 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -551,6 +551,7 @@
'metrics/bucket_ranges_unittest.cc',
'metrics/field_trial_unittest.cc',
'metrics/histogram_base_unittest.cc',
+ 'metrics/histogram_delta_serialization_unittest.cc',
'metrics/histogram_unittest.cc',
'metrics/sparse_histogram_unittest.cc',
'metrics/stats_table_unittest.cc',
diff --git a/base/base.gypi b/base/base.gypi
index 73e6ccc..b60ffa6 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -339,6 +339,8 @@
'metrics/histogram.h',
'metrics/histogram_base.cc',
'metrics/histogram_base.h',
+ 'metrics/histogram_delta_serialization.cc',
+ 'metrics/histogram_delta_serialization.h',
'metrics/histogram_flattener.h',
'metrics/histogram_samples.cc',
'metrics/histogram_samples.h',
diff --git a/base/metrics/histogram_base.cc b/base/metrics/histogram_base.cc
index 5b46d299..5c6e2d2 100644
--- a/base/metrics/histogram_base.cc
+++ b/base/metrics/histogram_base.cc
@@ -58,20 +58,6 @@
}
}
-void DeserializeHistogramAndAddSamples(PickleIterator* iter) {
- HistogramBase* histogram = DeserializeHistogramInfo(iter);
- if (!histogram)
- return;
-
- if (histogram->flags() & base::HistogramBase::kIPCSerializationSourceFlag) {
- DVLOG(1) << "Single process mode, histogram observed and not copied: "
- << histogram->histogram_name();
- return;
- }
- histogram->AddSamplesFromPickle(iter);
-}
-
-
const HistogramBase::Sample HistogramBase::kSampleType_MAX = INT_MAX;
HistogramBase::HistogramBase(const std::string& name)
diff --git a/base/metrics/histogram_base.h b/base/metrics/histogram_base.h
index 4663647..4248bd8 100644
--- a/base/metrics/histogram_base.h
+++ b/base/metrics/histogram_base.h
@@ -6,6 +6,7 @@
#define BASE_METRICS_HISTOGRAM_BASE_H_
#include <string>
+#include <vector>
#include "base/atomicops.h"
#include "base/base_export.h"
@@ -43,10 +44,6 @@
BASE_EXPORT_PRIVATE HistogramBase* DeserializeHistogramInfo(
PickleIterator* iter);
-// Create or find existing histogram and add the samples from pickle.
-// Silently returns when seeing any data problem in the pickle.
-BASE_EXPORT void DeserializeHistogramAndAddSamples(PickleIterator* iter);
-
////////////////////////////////////////////////////////////////////////////////
class BASE_EXPORT HistogramBase {
diff --git a/base/metrics/histogram_base_unittest.cc b/base/metrics/histogram_base_unittest.cc
index 0e19d56..4a2963a 100644
--- a/base/metrics/histogram_base_unittest.cc
+++ b/base/metrics/histogram_base_unittest.cc
@@ -61,40 +61,6 @@
EXPECT_EQ(HistogramBase::kUmaTargetedHistogramFlag, deserialized->flags());
}
-TEST_F(HistogramBaseTest, DeserializeHistogramAndAddSamples) {
- HistogramBase* histogram = Histogram::FactoryGet(
- "TestHistogram", 1, 1000, 10, HistogramBase::kIPCSerializationSourceFlag);
- histogram->Add(1);
- histogram->Add(10);
- histogram->Add(100);
- histogram->Add(1000);
-
- Pickle pickle;
- ASSERT_TRUE(histogram->SerializeInfo(&pickle));
- histogram->SnapshotSamples()->Serialize(&pickle);
-
- PickleIterator iter(pickle);
- DeserializeHistogramAndAddSamples(&iter);
-
- // The histogram has kIPCSerializationSourceFlag. So samples will be ignored.
- scoped_ptr<HistogramSamples> snapshot(histogram->SnapshotSamples());
- EXPECT_EQ(1, snapshot->GetCount(1));
- EXPECT_EQ(1, snapshot->GetCount(10));
- EXPECT_EQ(1, snapshot->GetCount(100));
- EXPECT_EQ(1, snapshot->GetCount(1000));
-
- // Clear kIPCSerializationSourceFlag to emulate multi-process usage.
- histogram->ClearFlags(HistogramBase::kIPCSerializationSourceFlag);
- PickleIterator iter2(pickle);
- DeserializeHistogramAndAddSamples(&iter2);
-
- scoped_ptr<HistogramSamples> snapshot2(histogram->SnapshotSamples());
- EXPECT_EQ(2, snapshot2->GetCount(1));
- EXPECT_EQ(2, snapshot2->GetCount(10));
- EXPECT_EQ(2, snapshot2->GetCount(100));
- EXPECT_EQ(2, snapshot2->GetCount(1000));
-}
-
TEST_F(HistogramBaseTest, DeserializeLinearHistogram) {
HistogramBase* histogram = LinearHistogram::FactoryGet(
"TestHistogram", 1, 1000, 10,
diff --git a/base/metrics/histogram_delta_serialization.cc b/base/metrics/histogram_delta_serialization.cc
new file mode 100644
index 0000000..924916d
--- /dev/null
+++ b/base/metrics/histogram_delta_serialization.cc
@@ -0,0 +1,111 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/metrics/histogram_delta_serialization.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_snapshot_manager.h"
+#include "base/pickle.h"
+#include "base/safe_numerics.h"
+#include "base/values.h"
+
+namespace base {
+
+namespace {
+
+// Create or find existing histogram and add the samples from pickle.
+// Silently returns when seeing any data problem in the pickle.
+void DeserializeHistogramAndAddSamples(PickleIterator* iter) {
+ HistogramBase* histogram = DeserializeHistogramInfo(iter);
+ if (!histogram)
+ return;
+
+ if (histogram->flags() & HistogramBase::kIPCSerializationSourceFlag) {
+ DVLOG(1) << "Single process mode, histogram observed and not copied: "
+ << histogram->histogram_name();
+ return;
+ }
+ histogram->AddSamplesFromPickle(iter);
+}
+
+} // namespace
+
+HistogramDeltaSerialization::HistogramDeltaSerialization(
+ const std::string& caller_name)
+ : histogram_snapshot_manager_(this),
+ serialized_deltas_(NULL) {
+ inconsistencies_histogram_ =
+ LinearHistogram::FactoryGet(
+ "Histogram.Inconsistencies" + caller_name, 1,
+ HistogramBase::NEVER_EXCEEDED_VALUE,
+ HistogramBase::NEVER_EXCEEDED_VALUE + 1,
+ HistogramBase::kUmaTargetedHistogramFlag);
+
+ inconsistencies_unique_histogram_ =
+ LinearHistogram::FactoryGet(
+ "Histogram.Inconsistencies" + caller_name + "Unique", 1,
+ HistogramBase::NEVER_EXCEEDED_VALUE,
+ HistogramBase::NEVER_EXCEEDED_VALUE + 1,
+ HistogramBase::kUmaTargetedHistogramFlag);
+
+ inconsistent_snapshot_histogram_ =
+ Histogram::FactoryGet(
+ "Histogram.InconsistentSnapshot" + caller_name, 1, 1000000, 50,
+ HistogramBase::kUmaTargetedHistogramFlag);
+}
+
+HistogramDeltaSerialization::~HistogramDeltaSerialization() {
+}
+
+void HistogramDeltaSerialization::PrepareAndSerializeDeltas(
+ std::vector<std::string>* serialized_deltas) {
+ serialized_deltas_ = serialized_deltas;
+ // Note: Before serializing, we set the kIPCSerializationSourceFlag for all
+ // the histograms, so that the receiving process can distinguish them from the
+ // local histograms.
+ histogram_snapshot_manager_.PrepareDeltas(
+ Histogram::kIPCSerializationSourceFlag, false);
+ serialized_deltas_ = NULL;
+}
+
+// static
+void HistogramDeltaSerialization::DeserializeAndAddSamples(
+ const std::vector<std::string>& serialized_deltas) {
+ for (std::vector<std::string>::const_iterator it = serialized_deltas.begin();
+ it != serialized_deltas.end(); ++it) {
+ Pickle pickle(it->data(), checked_numeric_cast<int>(it->size()));
+ PickleIterator iter(pickle);
+ DeserializeHistogramAndAddSamples(&iter);
+ }
+}
+
+void HistogramDeltaSerialization::RecordDelta(
+ const HistogramBase& histogram,
+ const HistogramSamples& snapshot) {
+ DCHECK_NE(0, snapshot.TotalCount());
+
+ Pickle pickle;
+ histogram.SerializeInfo(&pickle);
+ snapshot.Serialize(&pickle);
+ serialized_deltas_->push_back(
+ std::string(static_cast<const char*>(pickle.data()), pickle.size()));
+}
+
+void HistogramDeltaSerialization::InconsistencyDetected(
+ HistogramBase::Inconsistency problem) {
+ inconsistencies_histogram_->Add(problem);
+}
+
+void HistogramDeltaSerialization::UniqueInconsistencyDetected(
+ HistogramBase::Inconsistency problem) {
+ inconsistencies_unique_histogram_->Add(problem);
+}
+
+void HistogramDeltaSerialization::InconsistencyDetectedInLoggedCount(
+ int amount) {
+ inconsistent_snapshot_histogram_->Add(std::abs(amount));
+}
+
+} // namespace base
diff --git a/base/metrics/histogram_delta_serialization.h b/base/metrics/histogram_delta_serialization.h
new file mode 100644
index 0000000..ccadb12
--- /dev/null
+++ b/base/metrics/histogram_delta_serialization.h
@@ -0,0 +1,65 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_METRICS_HISTOGRAM_DELTA_SERIALIZATION_H_
+#define BASE_METRICS_HISTOGRAM_DELTA_SERIALIZATION_H_
+
+#include <string>
+#include <vector>
+
+#include "base/base_export.h"
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_flattener.h"
+#include "base/metrics/histogram_snapshot_manager.h"
+
+namespace base {
+
+class HistogramBase;
+
+// Serializes and restores histograms deltas.
+class BASE_EXPORT HistogramDeltaSerialization : public HistogramFlattener {
+ public:
+ // |caller_name| is string used in histograms for counting inconsistencies.
+ explicit HistogramDeltaSerialization(const std::string& caller_name);
+ virtual ~HistogramDeltaSerialization();
+
+ // Computes deltas in histogram bucket counts relative to the previous call to
+ // this method. Stores the deltas in serialized form into |serialized_deltas|.
+ // If |serialized_deltas| is NULL, no data is serialized, though the next call
+ // will compute the deltas relative to this one.
+ void PrepareAndSerializeDeltas(std::vector<std::string>* serialized_deltas);
+
+ // Deserialize deltas and add samples to corresponding histograms, creating
+ // them if necessary. Silently ignores errors in |serialized_deltas|.
+ static void DeserializeAndAddSamples(
+ const std::vector<std::string>& serialized_deltas);
+
+ private:
+ // HistogramFlattener implementation.
+ virtual void RecordDelta(const HistogramBase& histogram,
+ const HistogramSamples& snapshot) OVERRIDE;
+ virtual void InconsistencyDetected(
+ HistogramBase::Inconsistency problem) OVERRIDE;
+ virtual void UniqueInconsistencyDetected(
+ HistogramBase::Inconsistency problem) OVERRIDE;
+ virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE;
+
+ // Calculates deltas in histogram counters.
+ HistogramSnapshotManager histogram_snapshot_manager_;
+
+ // Output buffer for serialized deltas.
+ std::vector<std::string>* serialized_deltas_;
+
+ // Histograms to count inconsistencies in snapshots.
+ HistogramBase* inconsistencies_histogram_;
+ HistogramBase* inconsistencies_unique_histogram_;
+ HistogramBase* inconsistent_snapshot_histogram_;
+
+ DISALLOW_COPY_AND_ASSIGN(HistogramDeltaSerialization);
+};
+
+} // namespace base
+
+#endif // BASE_METRICS_HISTOGRAM_DELTA_SERIALIZATION_H_
diff --git a/base/metrics/histogram_delta_serialization_unittest.cc b/base/metrics/histogram_delta_serialization_unittest.cc
new file mode 100644
index 0000000..b53520c5
--- /dev/null
+++ b/base/metrics/histogram_delta_serialization_unittest.cc
@@ -0,0 +1,54 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/metrics/histogram_delta_serialization.h"
+
+#include <vector>
+
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/statistics_recorder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+TEST(HistogramDeltaSerializationTest, DeserializeHistogramAndAddSamples) {
+ StatisticsRecorder statistic_recorder;
+ HistogramDeltaSerialization serializer("HistogramDeltaSerializationTest");
+ std::vector<std::string> deltas;
+ // Nothing was changed yet.
+ serializer.PrepareAndSerializeDeltas(&deltas);
+ EXPECT_TRUE(deltas.empty());
+
+ HistogramBase* histogram = Histogram::FactoryGet(
+ "TestHistogram", 1, 1000, 10, HistogramBase::kIPCSerializationSourceFlag);
+ histogram->Add(1);
+ histogram->Add(10);
+ histogram->Add(100);
+ histogram->Add(1000);
+
+ serializer.PrepareAndSerializeDeltas(&deltas);
+ EXPECT_FALSE(deltas.empty());
+
+ HistogramDeltaSerialization::DeserializeAndAddSamples(deltas);
+
+ // The histogram has kIPCSerializationSourceFlag. So samples will be ignored.
+ scoped_ptr<HistogramSamples> snapshot(histogram->SnapshotSamples());
+ EXPECT_EQ(1, snapshot->GetCount(1));
+ EXPECT_EQ(1, snapshot->GetCount(10));
+ EXPECT_EQ(1, snapshot->GetCount(100));
+ EXPECT_EQ(1, snapshot->GetCount(1000));
+
+ // Clear kIPCSerializationSourceFlag to emulate multi-process usage.
+ histogram->ClearFlags(HistogramBase::kIPCSerializationSourceFlag);
+ HistogramDeltaSerialization::DeserializeAndAddSamples(deltas);
+
+ scoped_ptr<HistogramSamples> snapshot2(histogram->SnapshotSamples());
+ EXPECT_EQ(2, snapshot2->GetCount(1));
+ EXPECT_EQ(2, snapshot2->GetCount(10));
+ EXPECT_EQ(2, snapshot2->GetCount(100));
+ EXPECT_EQ(2, snapshot2->GetCount(1000));
+}
+
+} // namespace base
diff --git a/base/metrics/statistics_recorder.h b/base/metrics/statistics_recorder.h
index 9a55225..0fdfb769 100644
--- a/base/metrics/statistics_recorder.h
+++ b/base/metrics/statistics_recorder.h
@@ -85,6 +85,8 @@
friend class HistogramTest;
friend class SparseHistogramTest;
friend class StatisticsRecorderTest;
+ FRIEND_TEST_ALL_PREFIXES(HistogramDeltaSerializationTest,
+ DeserializeHistogramAndAddSamples);
// The constructor just initializes static members. Usually client code should
// use Initialize to do this. But in test code, you can friend this class and
diff --git a/content/browser/histogram_synchronizer.cc b/content/browser/histogram_synchronizer.cc
index 5a1e6f9..16617237 100644
--- a/content/browser/histogram_synchronizer.cc
+++ b/content/browser/histogram_synchronizer.cc
@@ -8,6 +8,7 @@
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_delta_serialization.h"
#include "base/pickle.h"
#include "base/threading/thread.h"
#include "base/threading/thread_restrictions.h"
@@ -268,16 +269,10 @@
const std::vector<std::string>& pickled_histograms) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ base::HistogramDeltaSerialization::DeserializeAndAddSamples(
+ pickled_histograms);
+
RequestContext* request = RequestContext::GetRequestContext(sequence_number);
-
- for (std::vector<std::string>::const_iterator it = pickled_histograms.begin();
- it < pickled_histograms.end();
- ++it) {
- Pickle pickle(it->data(), it->size());
- PickleIterator iter(pickle);
- base::DeserializeHistogramAndAddSamples(&iter);
- }
-
if (!request)
return;
diff --git a/content/child/child_histogram_message_filter.cc b/content/child/child_histogram_message_filter.cc
index 427dd0d..f05d111c 100644
--- a/content/child/child_histogram_message_filter.cc
+++ b/content/child/child_histogram_message_filter.cc
@@ -8,8 +8,7 @@
#include "base/bind.h"
#include "base/message_loop/message_loop.h"
-#include "base/metrics/statistics_recorder.h"
-#include "base/pickle.h"
+#include "base/metrics/histogram_delta_serialization.h"
#include "content/child/child_process.h"
#include "content/child/child_thread.h"
#include "content/common/child_process_messages.h"
@@ -18,8 +17,7 @@
ChildHistogramMessageFilter::ChildHistogramMessageFilter()
: channel_(NULL),
- io_message_loop_(ChildProcess::current()->io_message_loop_proxy()),
- histogram_snapshot_manager_(this) {
+ io_message_loop_(ChildProcess::current()->io_message_loop_proxy()) {
}
ChildHistogramMessageFilter::~ChildHistogramMessageFilter() {
@@ -54,53 +52,19 @@
}
void ChildHistogramMessageFilter::UploadAllHistograms(int sequence_number) {
- DCHECK_EQ(0u, pickled_histograms_.size());
+ if (!histogram_delta_serialization_) {
+ histogram_delta_serialization_.reset(
+ new base::HistogramDeltaSerialization("ChildProcess"));
+ }
- // Push snapshots into our pickled_histograms_ vector.
- // Note: Before serializing, we set the kIPCSerializationSourceFlag for all
- // the histograms, so that the receiving process can distinguish them from the
- // local histograms.
- histogram_snapshot_manager_.PrepareDeltas(
- base::Histogram::kIPCSerializationSourceFlag, false);
+ std::vector<std::string> deltas;
+ histogram_delta_serialization_->PrepareAndSerializeDeltas(&deltas);
+ channel_->Send(
+ new ChildProcessHostMsg_ChildHistogramData(sequence_number, deltas));
- channel_->Send(new ChildProcessHostMsg_ChildHistogramData(
- sequence_number, pickled_histograms_));
-
- pickled_histograms_.clear();
static int count = 0;
count++;
DHISTOGRAM_COUNTS("Histogram.ChildProcessHistogramSentCount", count);
}
-void ChildHistogramMessageFilter::RecordDelta(
- const base::HistogramBase& histogram,
- const base::HistogramSamples& snapshot) {
- DCHECK_NE(0, snapshot.TotalCount());
-
- Pickle pickle;
- histogram.SerializeInfo(&pickle);
- snapshot.Serialize(&pickle);
-
- pickled_histograms_.push_back(
- std::string(static_cast<const char*>(pickle.data()), pickle.size()));
-}
-
-void ChildHistogramMessageFilter::InconsistencyDetected(
- base::HistogramBase::Inconsistency problem) {
- UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcess",
- problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
-}
-
-void ChildHistogramMessageFilter::UniqueInconsistencyDetected(
- base::HistogramBase::Inconsistency problem) {
- UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesChildProcessUnique",
- problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
-}
-
-void ChildHistogramMessageFilter::InconsistencyDetectedInLoggedCount(
- int amount) {
- UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotChildProcess",
- std::abs(amount));
-}
-
} // namespace content
diff --git a/content/child/child_histogram_message_filter.h b/content/child/child_histogram_message_filter.h
index 85d2e2097..98a30a2 100644
--- a/content/child/child_histogram_message_filter.h
+++ b/content/child/child_histogram_message_filter.h
@@ -9,20 +9,16 @@
#include <vector>
#include "base/basictypes.h"
-#include "base/metrics/histogram_base.h"
-#include "base/metrics/histogram_flattener.h"
-#include "base/metrics/histogram_snapshot_manager.h"
#include "ipc/ipc_channel_proxy.h"
namespace base {
-class HistogramSamples;
+class HistogramDeltaSerialization;
class MessageLoopProxy;
} // namespace base
namespace content {
-class ChildHistogramMessageFilter : public base::HistogramFlattener,
- public IPC::ChannelProxy::MessageFilter {
+class ChildHistogramMessageFilter : public IPC::ChannelProxy::MessageFilter {
public:
ChildHistogramMessageFilter();
@@ -33,15 +29,6 @@
void SendHistograms(int sequence_number);
- // HistogramFlattener interface (override) methods.
- virtual void RecordDelta(const base::HistogramBase& histogram,
- const base::HistogramSamples& snapshot) OVERRIDE;
- virtual void InconsistencyDetected(
- base::HistogramBase::Inconsistency problem) OVERRIDE;
- virtual void UniqueInconsistencyDetected(
- base::HistogramBase::Inconsistency problem) OVERRIDE;
- virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE;
-
private:
typedef std::vector<std::string> HistogramPickledList;
@@ -58,11 +45,8 @@
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
- // Collection of histograms to send to the browser.
- HistogramPickledList pickled_histograms_;
-
- // |histogram_snapshot_manager_| prepares histogram deltas for transmission.
- base::HistogramSnapshotManager histogram_snapshot_manager_;
+ // Prepares histogram deltas for transmission.
+ scoped_ptr<base::HistogramDeltaSerialization> histogram_delta_serialization_;
DISALLOW_COPY_AND_ASSIGN(ChildHistogramMessageFilter);
};
