Use subtle::NoBarrier* statements for Histogram flags_ field.

base/metrics/histogram_base.cc

 // 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.
 
 #include "base/metrics/histogram_base.h"
 
 #include <climits>
 
 #include "base/json/json_string_value_serializer.h"
 #include "base/logging.h"
@@ skipping 54 matching lines @@
     : histogram_name_(name),
       flags_(kNoFlags) {}
 
 HistogramBase::~HistogramBase() {}
 
 void HistogramBase::CheckName(const StringPiece& name) const {
   DCHECK_EQ(histogram_name(), name);
 }
 
 void HistogramBase::SetFlags(int32 flags) {
-  flags_ |= flags;
+  HistogramBase::Count old_flags = subtle::NoBarrier_Load(&flags_);
+  subtle::NoBarrier_Store(&flags_, old_flags | flags);

[Alexander Potapenko, 2015/11/18 11:23:28]

The question is whether there is any data that is being concurrently read or
written together with the flags.
For example, if there's a data pointer that's written by one thread before
SetFlags(kSomeFlag) and dereferenced by another thread after checking flags() &
kSomeFlag, there must be a barrier between the accesses to flags_ and to the
pointer (i.e. accesses to flags_ must have acquire/release semantics).
If your flags won't associated data it's fine to keep the accesses nobarrier.

[Alexei Svitkine (slow), 2015/11/18 16:37:12]

I don't think there's a case where there is associated data that's not already
guarded by a lock.

The only case where there is an associate data is the callback flag, but the
callback itself is accessed after grabbing a lock on StatisticsRecorder. The
flag is just used to ensure that 99.999% cases that don't have a callback set
don't need to do any synchronization. (And inconsistencies between the flag and
the callback are safe - i.e. it's not critical if one doesn't match the other
due to caching for some period of time until a barrier happens elsewhere.)

 }
 
 void HistogramBase::ClearFlags(int32 flags) {
-  flags_ &= ~flags;
+  HistogramBase::Count old_flags = subtle::NoBarrier_Load(&flags_);
+  subtle::NoBarrier_Store(&flags_, old_flags & ~flags);
 }
 
 void HistogramBase::AddTime(const TimeDelta& time) {
   Add(static_cast<Sample>(time.InMilliseconds()));
 }
 
 void HistogramBase::AddBoolean(bool value) {
   Add(value ? 1 : 0);
 }
 
@@ skipping 22 matching lines @@
   root.SetInteger("count", count);
   root.SetDouble("sum", static_cast<double>(sum));
   root.SetInteger("flags", flags());
   root.Set("params", parameters.Pass());
   root.Set("buckets", buckets.Pass());
   root.SetInteger("pid", GetCurrentProcId());
   serializer.Serialize(root);
 }
 
 void HistogramBase::FindAndRunCallback(HistogramBase::Sample sample) const {
-  if ((flags_ & kCallbackExists) == 0)
+  if ((flags() & kCallbackExists) == 0)
     return;
 
   StatisticsRecorder::OnSampleCallback cb =
       StatisticsRecorder::FindCallback(histogram_name());
   if (!cb.is_null())
     cb.Run(sample);
 }
 
 void HistogramBase::WriteAsciiBucketGraph(double current_size,
                                           double max_size,
@@ skipping 20 matching lines @@
   return result;
 }
 
 void HistogramBase::WriteAsciiBucketValue(Count current,
                                           double scaled_sum,
                                           std::string* output) const {
   StringAppendF(output, " (%d = %3.1f%%)", current, current/scaled_sum);
 }
 
 }  // namespace base