De-duplicate ranges information in persistent memory.

base/metrics/bucket_ranges.h

 // Copyright (c) 2012 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.
 //
 // BucketRanges stores the vector of ranges that delimit what samples are
 // tallied in the corresponding buckets of a histogram. Histograms that have
 // same ranges for all their corresponding buckets should share the same
 // BucketRanges object.
 //
 // E.g. A 5 buckets LinearHistogram with 1 as minimal value and 4 as maximal
 // value will need a BucketRanges with 6 ranges:
 // 0, 1, 2, 3, 4, INT_MAX
 //
 // TODO(kaiwang): Currently we keep all negative values in 0~1 bucket. Consider
 // changing 0 to INT_MIN.
 
 #ifndef BASE_METRICS_BUCKET_RANGES_H_
 #define BASE_METRICS_BUCKET_RANGES_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <vector>
 
 #include <limits.h>
 
+#include "base/atomicops.h"
 #include "base/base_export.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_base.h"
 
 namespace base {
 
 class BASE_EXPORT BucketRanges {
  public:
   typedef std::vector<HistogramBase::Sample> Ranges;
 
@@ skipping 14 matching lines @@
 
   // Checksum methods to verify whether the ranges are corrupted (e.g. bad
   // memory access).
   uint32_t CalculateChecksum() const;
   bool HasValidChecksum() const;
   void ResetChecksum();
 
   // Return true iff |other| object has same ranges_ as |this| object's ranges_.
   bool Equals(const BucketRanges* other) const;
 
+  // Set and get a reference into persistent memory where this bucket data
+  // can be found (and re-used). These calls are internally atomic with no
+  // safety against overwriting an existing value since though it is wasteful
+  // to have multiple identical persistent records, it is still safe.
+  void set_persistent_reference(uint32_t ref) const {
+    subtle::NoBarrier_Store(&persistent_reference_, ref);
+  }
+  uint32_t persistent_reference() const {
+    return subtle::NoBarrier_Load(&persistent_reference_);
+  }
+
  private:
   // A monotonically increasing list of values which determine which bucket to
   // put a sample into.  For each index, show the smallest sample that can be
   // added to the corresponding bucket.
   Ranges ranges_;
 
   // Checksum for the conntents of ranges_.  Used to detect random over-writes
   // of our data, and to quickly see if some other BucketRanges instance is
   // possibly Equal() to this instance.
   // TODO(kaiwang): Consider change this to uint64_t. Because we see a lot of
   // noise on UMA dashboard.
   uint32_t checksum_;
 
+  // A reference into a global PersistentMemoryAllocator where the ranges
+  // information is stored. This allows for the record to be created once and
+  // re-used simply by having all histograms with the same ranges use the
+  // same reference.
+  mutable subtle::Atomic32 persistent_reference_ = 0;

[Alexei Svitkine (slow), 2017/04/04 17:45:09]

I don't think this should be mutable. Can you just change the relevant functions
to not receive the param by const ptr - but by regular ptr?

[bcwhite, 2017/04/05 13:41:30]

The "const" goes all the way back to StatisticsRecorder's
RegisterOrDeleteDuplicateRanges method and I don't think that should be changed.

I think this is okay because though a reference is changing, the contents never
do.  Either the persistent contents don't exist or they match that of the const
ranges.  Even if the reference changes, it would always point to the same
values, just different copies of it.  In any case, the value can always be used
with complete safety after reading just as though it was immutable.

[Alexei Svitkine (slow), 2017/04/05 20:57:51]

Fair enough. I guess we still want to keep the other members constant, but the
way the API is designed they're not initialized at ctor time.

Ideally, I think it would be great if the BucketRanges object could be made to
have ranges_ and checksum_ fields const and initialized on construction and then
we can make the pointer to it non-const with this field being changeable.
However, that's a bigger change so I won't hold up this CL on it.

+
   DISALLOW_COPY_AND_ASSIGN(BucketRanges);
 };
 
 //////////////////////////////////////////////////////////////////////////////
 // Expose only for test.
 BASE_EXPORT extern const uint32_t kCrcTable[256];
 
 }  // namespace base
 
 #endif  // BASE_METRICS_BUCKET_RANGES_H_