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"
@@ skipping 25 matching lines @@
 
 // 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
-#if !defined(OS_NACL)
-const size_t kFieldTrialAllocationSize = 4 << 10;  // 4 KiB = one page
-#endif
+const size_t kFieldTrialAllocationSize = 4 << 14;  // 64 KiB should be enough

[Alexei Svitkine (slow), 2016/11/08 18:15:23]

The "should be enough" comment is not useful. 

I suggest moving the comment to above the constant and expanding on it. In
particular, mentioning that this doesn't want memory since parts of the block
that are not touched don't get mapped.

If you want to keep the "should be enough" comment, you should justify it - e.g.
by referencing the observed amounts currently.

 
 // 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.
 struct FieldTrialEntry {
   bool activated;
 
   // Size of the pickled structure, NOT the total size of this entry.
   uint32_t size;
 
@@ skipping 662 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",

[Alexei Svitkine (slow), 2016/11/08 18:15:23]

Changing the macro without changing histogram name is not OK. It will renumber
all the bucket ranges and now you'll have different buckets reported by
different versions. So please rename the histogram, marking the old name as
<obsolete> in the config.

Can you also add a different histogram logging the percent used?

[Alexei Svitkine (slow), 2016/11/08 18:18:38]

For the rename, you can use "2" suffix to the name.

-                               field_trial_length);
+    UMA_HISTOGRAM_COUNTS_100000("UMA.FieldTrialAllocator.Size",
+                                global_->field_trial_allocator_->used());
     return;
   }
 #endif
 
   AddForceFieldTrialsFlag(cmd_line);
 }
 
 // static
 FieldTrial* FieldTrialList::CreateFieldTrial(
     const std::string& name,
@@ skipping 229 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