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 24 matching lines @@
 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

[bcwhite, 2016/11/08 20:30:02]

Blank line after.

[lawrencewu, 2016/11/09 18:06:46]

Done.

+// We allocate 64 KiB to hold all the field trial data. This should be enough,

[bcwhite, 2016/11/08 20:30:02]

Include the date when this was true.  Also, comment what will happen if this
isn't large enough.

[lawrencewu, 2016/11/09 18:06:46]

Done.

+// as currently we use ~8KiB for the field trials, and ~10KiB for experiment
+// parameters. This also doesn't allocate all 64 KiB at once -- the pages only
+// get mapped to physical memory when they are touched.
 #if !defined(OS_NACL)
-const size_t kFieldTrialAllocationSize = 4 << 10;  // 4 KiB = one page
+const size_t kFieldTrialAllocationSize = 4 << 14;

[bcwhite, 2016/11/08 20:14:32]

Please do this as

64 << 10 // 64 KiB

[lawrencewu, 2016/11/09 18:06:46]

Done.

 #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.

[bcwhite, 2016/11/08 20:30:02]

Comment that any changes to this structure require a bump in the TypeId version
number defined at the top of this file.

[lawrencewu, 2016/11/09 18:06:46]

Done.

 struct FieldTrialEntry {
   bool activated;

[bcwhite, 2016/11/08 20:30:02]

This should be a fixed size (i.e. uint32_t) so that this structure is guaranteed
to be identical across both 32/64 bit architectures.  Otherwise, data written by
one arch will not read correctly if read by a different one.

"bool" is not strictly defined in its size.

[lawrencewu, 2016/11/09 18:06:46]

Done.

 
   // 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 107 matching lines @@
 #if defined(OS_WIN)
 HANDLE CreateReadOnlyHandle(SharedPersistentMemoryAllocator* allocator) {
   HANDLE src = allocator->shared_memory()->handle().GetHandle();
   ProcessHandle process = GetCurrentProcess();
   DWORD access = SECTION_MAP_READ | SECTION_QUERY;
   HANDLE dst;
   if (!::DuplicateHandle(process, src, process, &dst, access, true, 0))
     return nullptr;
   return dst;
 }
+
+void ReportFieldTrialAllocatorUsage(

[bcwhite, 2016/11/08 20:14:32]

Don't need this.

[lawrencewu, 2016/11/09 18:06:46]

Removed.

+    SharedPersistentMemoryAllocator* allocator) {
+  size_t used = allocator->used();
+  UMA_HISTOGRAM_COUNTS_100000("UMA.FieldTrialAllocator.Used", used);
+  int used_percent = used / kFieldTrialAllocationSize;
+  UMA_HISTOGRAM_PERCENTAGE("UMA.FieldTrialAllocator.UsedPct",
+                           used_percent * 100);
+}
 #endif
 
 }  // namespace
 
 // statics
 const int FieldTrial::kNotFinalized = -1;
 const int FieldTrial::kDefaultGroupNumber = 0;
 bool FieldTrial::enable_benchmarking_ = false;
 
 int FieldTrialList::kNoExpirationYear = 0;
@@ skipping 529 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);
+    ReportFieldTrialAllocatorUsage(global_->field_trial_allocator_.get());

[bcwhite, 2016/11/08 20:14:32]

When you create the allocator, call

allocator->CreateTrackingHistograms(allocator->Name());

Here, call

allocator->UpdateTrackingHistograms();


See here for how the histograms are named:
https://cs.chromium.org/chromium/src/base/metrics/persistent_memory_allocator...

[lawrencewu, 2016/11/09 18:06:46]

Done. But this will get called on every subprocess launch. Is that okay?

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

Not ideal but acceptable.  If you can call it once after everything has been
loaded, that would be best but it's not overly important since we're really just
looking for the maximum value it reaches.

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

Acknowledged.

     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