Add a field trial for using lower sqlite cache sizes.

chrome/browser/history/history_field_trial.cc

Index: chrome/browser/history/history_field_trial.cc
===================================================================
--- chrome/browser/history/history_field_trial.cc	(revision 0)
+++ chrome/browser/history/history_field_trial.cc	(revision 0)
@@ -0,0 +1,43 @@
+// Copyright (c) 2011 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 "chrome/browser/history/history_field_trial.h"
+
+#include "base/metrics/field_trial.h"
+
+namespace history {
+
+namespace {
+
+int g_low_mem_trial_group = 0;
+bool g_low_mem_trial = false;
+
+static const char kHistoryFieldTrialName[] = "History";

[Scott Hess - ex-Googler, 2011/10/26 21:44:59]

AFAICT this is your namespace for field trials.  Unless you're sure you'll only
ever want one, it might be reasonable to have it be History_CacheSize or
something.

+
+}  // namespace
+
+// static
+void HistoryFieldTrial::Activate() {
+  if (g_low_mem_trial_group)
+    return;  // Already initialized.

[Scott Hess - ex-Googler, 2011/11/23 23:10:08]

Note that FieldTrial::kDefaultGroupNumber is 0.  I think this only fires if
you're in the 5% group, otherwise you keep Activating... maybe needs a local
global to handle this.

I think this is being reverted for breaking browser tests...

jar says it should have failed when you created a new FieldTrial, but that may
be for another day.

+
+  scoped_refptr<base::FieldTrial> trial(new base::FieldTrial(
+      kHistoryFieldTrialName, 1000, "Inactive", 2012, 2, 1));

[Scott Hess - ex-Googler, 2011/10/26 21:44:59]

Since I don't see this stored anywhere, am I safe in assuming that
FieldTrialList keeps a reference and this ref is only here to prevent a race
condition before the end of the fn?

[jar (doing other things), 2011/10/27 00:23:55]

The real field trial is actually ref-counted, and registered globally.  This
uses a refptr to hold it briefly.  As it turns out, I'm pretty sure we
deliberately leak these objects (to be sure they are around long into the
shutdown actions).

On 2011/10/26 21:44:59, shess wrote:

+
+  g_low_mem_trial_group = trial->AppendGroup("LowMem", 50);  // 5%
+  if (trial->group() == g_low_mem_trial_group)
+    g_low_mem_trial = true;
+}
+
+// static
+bool HistoryFieldTrial::IsLowMemFieldTrial() {
+  return g_low_mem_trial;
+}
+
+// static
+std::string HistoryFieldTrial::GetGroupSuffix() {
+  return g_low_mem_trial ? std::string("_LowMem") : std::string();

[Scott Hess - ex-Googler, 2011/10/26 21:44:59]

Seems like it might be reasonable to have this construct the histogram name,
rather than having the caller combine this with the histogram name.  I mean
like:

std::string MakeName(const std::string& name_prefix) {
  if (!g_log_mem_trial_group)
    return name_prefix;
  return FieldTrial::MakeName(name_prefix, kLowMemGroupName);
}

With obvious tweak to AppendGroup() call.  I'm not sure if that's what jar was
getting at?

[jar (doing other things), 2011/10/27 00:23:55]

+1

The MakeName() function was crafted to be nicely used in a HISTOGRAM macro. 
That is the common pattern to see... and is probably safer/more readably
correct.

Common use is:

UMA_HISTOGRAM_STUFF(MakeName("Group.HistogramName", "FieldTrialName"), sample);
On 2011/10/26 21:44:59, shess wrote:

[brettw, 2011/10/28 22:30:40]

I feel like I'm missing something fundamental that's making us talk past each
other.

The way this works is that I'm taking 5% of the users and making them use
smaller caches. I then plan to look at the histograms to see if there is a
significant difference in performance from the regular users.

So I've got two histograms, one old with the same meaning and name it always had
for users not in the field trial (big cache), and one for users in the field
trial with _LowMem appended to the name.

You're asking me to keep the old histogram with the old meaning. This is what I
feel like I'm doing. MakeName doesn't do what I want because then I won't get
the old histogram, since everything will have some suffix on it.

If I added 3 more "levels" of cache size in the field trial, I would then have
one original histogram, and 3 different suffixes for the field trial. If I were
to count the "no change" as part of the field trial and keep the old histogram
like you were suggesting, it seems like this would just diplicate the no change
case for a particular version of Chrome.

[Scott Hess - ex-Googler, 2011/10/28 23:18:19]

Probably!  At least from my end, my suggestion was because I get uncomfortable
seeing ad-hoc solutions in areas like this where there are interesting things
done with the results (like caching in a global types of things).  I'll defer to
jar on it, though, because my level is just discomfort, but I think you're right
that for the non-field-trial case the histogram is unchanged, while the
field-trial case does change it.

So far as having more than one trial case, I guess in the end it's entirely up
to whether you want to approach this as multiple rollouts as you go, or try to
get more info up front.

+}
+
+}  // namespace history
Property changes on: chrome\browser\history\history_field_trial.cc
___________________________________________________________________
Added: svn:eol-style
   + LF