Bump field trial allocator size to 64kb and report usage

base/metrics/field_trial.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/field_trial.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/base_switches.h"
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/pickle.h"
 #include "base/process/memory.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 
 namespace base {
 
 namespace {
 
 // Define a separator character to use when creating a persistent form of an
 // instance.  This is intended for use as a command line argument, passed to a
 // second process to mimic our state (i.e., provide the same group name).
 const char kPersistentStringSeparator = '/';  // Currently a slash.
 
 // Define a marker character to be used as a prefix to a trial name on the
 // command line which forces its activation.
 const char kActivationMarker = '*';
 
 // Use shared memory to communicate field trial (experiment) state. Set to false
 // for now while the implementation is fleshed out (e.g. data format, single
 // shared memory segment). See https://codereview.chromium.org/2365273004/ and
 // crbug.com/653874
 const bool kUseSharedMemoryForFieldTrials = false;
 
 // Constants for the field trial allocator.
 const char kAllocatorName[] = "FieldTrialAllocator";
-const uint32_t kFieldTrialType = 0xABA17E13 + 1;  // SHA1(FieldTrialEntry) v1
+const uint32_t kFieldTrialType = 0xABA17E13 + 2;  // SHA1(FieldTrialEntry) v2
+
+// We allocate 64 KiB to hold all the field trial data. This should be enough,
+// as currently we use ~8KiB for the field trials, and ~10KiB for experiment
+// parameters (as of 9/11/2016). This also doesn't allocate all 64 KiB at once
+// -- the pages only get mapped to physical memory when they are touched. If the
+// size of the allocated field trials does get larger than 64 KiB, then we will
+// drop some field trials in child processes, leading to an inconsistent view
+// between browser and child processes and possibly causing crashes (see
+// crbug.com/661617).
 #if !defined(OS_NACL)
-const size_t kFieldTrialAllocationSize = 4 << 10;  // 4 KiB = one page
+const size_t kFieldTrialAllocationSize = 64 << 10;  // 64 KiB
 #endif
 
 // We create one FieldTrialEntry per field trial in shared memory, via
 // AddToAllocatorWhileLocked. The FieldTrialEntry is followed by a base::Pickle
-// object that we unpickle and read from.
+// object that we unpickle and read from. Any changes to this structure requires
+// a bump in kFieldTrialType id defined above.
 struct FieldTrialEntry {
-  bool activated;
+  // Whether or not this field trial is activated. This is really just a boolean
+  // but marked as a uint32_t for portability reasons.
+  uint32_t activated;
 
   // Size of the pickled structure, NOT the total size of this entry.
   uint32_t size;
 
   // Calling this is only valid when the entry is initialized. That is, it
   // resides in shared memory and has a pickle containing the trial name and
   // group name following it.
   bool GetTrialAndGroupName(StringPiece* trial_name,
                             StringPiece* group_name) const {
     char* src = reinterpret_cast<char*>(const_cast<FieldTrialEntry*>(this)) +
@@ skipping 656 matching lines @@
     // command-line flag. The child process will do the reverse conversions to
     // retrieve the handle. See http://stackoverflow.com/a/153077
     auto uintptr_handle =
         reinterpret_cast<uintptr_t>(global_->readonly_allocator_handle_);
     size_t field_trial_length =
         global_->field_trial_allocator_->shared_memory()->mapped_size();
     std::string field_trial_handle = std::to_string(uintptr_handle) + "," +
                                      std::to_string(field_trial_length);
 
     cmd_line->AppendSwitchASCII(field_trial_handle_switch, field_trial_handle);
-    UMA_HISTOGRAM_COUNTS_10000("UMA.FieldTrialAllocator.Size",
-                               field_trial_length);
+    global_->field_trial_allocator_->UpdateTrackingHistograms();
     return;
   }
 #endif
 
   AddForceFieldTrialsFlag(cmd_line);
 }
 
 // static
 FieldTrial* FieldTrialList::CreateFieldTrial(
     const std::string& name,
@@ skipping 154 matching lines @@
   // Don't do anything if the allocator hasn't been instantiated yet.
   if (allocator == nullptr)
     return;
 
   // Or if the allocator is read only, which means we are in a child process and
   // shouldn't be writing to it.
   if (allocator->IsReadonly())
     return;
 
   // Or if we've already added it.
   if (field_trial->ref_ != SharedPersistentMemoryAllocator::kReferenceNull)

[bcwhite, 2016/11/09 18:57:37]

It's acceptable to use just "!value" to check validity of things that could be
"null" instead of an explicit comparison.

[lawrencewu, 2016/11/09 19:15:00]

Done.

     return;
 
   FieldTrial::State trial_state;
   if (!field_trial->GetState(&trial_state))
     return;
 
   Pickle pickle;
   pickle.WriteString(trial_state.trial_name);
   pickle.WriteString(trial_state.group_name);
 
@@ skipping 67 matching lines @@
     return;
   }
   AutoLock auto_lock(global_->lock_);
   CHECK(!global_->PreLockedFind(trial->trial_name())) << trial->trial_name();
   trial->AddRef();
   trial->SetTrialRegistered();
   global_->registered_[trial->trial_name()] = trial;
 }
 
 }  // namespace base