Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Unified Diff: base/metrics/histogram_persistence.cc

Issue 1537743006: Persist setup metrics and have Chrome report them during UMA upload. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@shared-histograms
Patch Set: test needs to clear out statistics-recorder before releasing histogram memory Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/metrics/histogram_persistence.cc
diff --git a/base/metrics/histogram_persistence.cc b/base/metrics/histogram_persistence.cc
index 19ca3a2fe9bd6f4dabf1b4e64452c3e058265ba4..b129bd757e0912fe8fcd3c7f3627f2f2964d42e4 100644
--- a/base/metrics/histogram_persistence.cc
+++ b/base/metrics/histogram_persistence.cc
@@ -50,6 +50,9 @@ enum CreateHistogramResultType {
CREATE_HISTOGRAM_MAX
};
+// Name of histogram for storing results of local operations.
+const char kResultHistogram[] = "UMA.CreatePersistentHistogram.Result";
+
// Type identifiers used when storing in persistent memory so they can be
// identified during extraction; the first 4 bytes of the SHA1 of the name
// is used as a unique integer. A "version number" is added to the base
@@ -149,9 +152,14 @@ HistogramBase* GetCreateHistogramResultHistogram() {
static bool initialized = false;
if (!initialized) {
initialized = true;
+ if (g_allocator) {
+ DLOG(WARNING) << "Creating the results-histogram inside persistent"
+ << " memory can cause future allocations to crash if"
+ << " that memory is ever released (for testing).";
+ }
+
histogram_pointer = LinearHistogram::FactoryGet(
- "UMA.CreatePersistentHistogram.Result",
- 1, CREATE_HISTOGRAM_MAX, CREATE_HISTOGRAM_MAX + 1,
+ kResultHistogram, 1, CREATE_HISTOGRAM_MAX, CREATE_HISTOGRAM_MAX + 1,
HistogramBase::kUmaTargetedHistogramFlag);
base::subtle::Release_Store(
&atomic_histogram_pointer,
@@ -173,13 +181,7 @@ void SetPersistentHistogramMemoryAllocator(
// Releasing or changing an allocator is extremely dangerous because it
// likely has histograms stored within it. If the backing memory is also
// also released, future accesses to those histograms will seg-fault.
- // It's not a fatal CHECK() because tests do this knowing that all
- // such persistent histograms have already been forgotten.
- if (g_allocator) {
- LOG(WARNING) << "Active PersistentMemoryAllocator has been released."
- << " Some existing histogram pointers may be invalid.";
- delete g_allocator;
- }
+ CHECK(!g_allocator);
g_allocator = allocator;
}
@@ -187,8 +189,35 @@ PersistentMemoryAllocator* GetPersistentHistogramMemoryAllocator() {
return g_allocator;
}
-PersistentMemoryAllocator* ReleasePersistentHistogramMemoryAllocator() {
+PersistentMemoryAllocator*
+ReleasePersistentHistogramMemoryAllocatorForTesting() {
PersistentMemoryAllocator* allocator = g_allocator;
+ if (!allocator)
+ return nullptr;
+
+ // Before releasing the memory, it's necessary to have the Statistics
+ // Recorder forget about the histograms contained within otherwise some
+ // operations will try to access it.
+ PersistentMemoryAllocator::Iterator iter;
+ PersistentMemoryAllocator::Reference ref;
+ uint32_t type_id;
+ allocator->CreateIterator(&iter);
+ while ((ref = allocator->GetNextIterable(&iter, &type_id)) != 0) {
+ if (type_id == kTypeIdHistogram) {
+ PersistentHistogramData* histogram_data =
+ allocator->GetAsObject<PersistentHistogramData>(
+ ref, kTypeIdHistogram);
+ DCHECK(histogram_data);
+ std::string name(histogram_data->name);
+ StatisticsRecorder::ForgetHistogramForTesting(name);
+ DLOG(WARNING) << "Dropped persistent histogram \"" << name << "\"";
+
+ // Fix by calling GetCreateHistogramResultHistogram() before setting
+ // the (temporary) persistent memory allocator.
+ DCHECK_NE(kResultHistogram, name);
+ }
+ }
+
g_allocator = nullptr;
return allocator;
};
@@ -444,7 +473,7 @@ HistogramBase* AllocatePersistentHistogram(
void ImportPersistentHistograms() {
// The lock protects against concurrent access to the iterator and is created
// in a thread-safe manner when needed.
- static base::LazyInstance<base::Lock> lock = LAZY_INSTANCE_INITIALIZER;
+ static base::LazyInstance<base::Lock>::Leaky lock = LAZY_INSTANCE_INITIALIZER;
if (g_allocator) {
base::AutoLock auto_lock(lock.Get());

Powered by Google App Engine
This is Rietveld 408576698