Add zram field trial

chrome/browser/chromeos/chrome_browser_main_chromeos.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 "chrome/browser/chromeos/chrome_browser_main_chromeos.h"
 
 #include <string>
 #include <vector>
 
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/linux_util.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_util.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/audio/audio_handler.h"
 #include "chrome/browser/chromeos/boot_times_loader.h"
@@ skipping 685 matching lines @@
 
   // Let the UserManager unregister itself as an observer of the CrosSettings
   // singleton before it is destroyed.
   UserManager::Get()->Shutdown();
 
   ChromeBrowserMainPartsLinux::PostMainMessageLoopRun();
 }
 
 void ChromeBrowserMainPartsChromeos::SetupPlatformFieldTrials() {
   SetupLowMemoryHeadroomFieldTrial();
+  if (base::chromeos::IsRunningOnChromeOS())
+    // Run this on ChromeOS only because it tries to access /home/chronos.
+    SetupZramFieldTrial();

[Ilya Sherman, 2013/01/08 20:16:16]

nit: Include curly braces for this if-stmt since it spans two textual lines with
the comment.

[Luigi Semenzato, 2013/01/08 21:28:19]

Thanks.  The if has been removed.

 }
 
 void ChromeBrowserMainPartsChromeos::SetupLowMemoryHeadroomFieldTrial() {
   chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
   // Only enable this experiment on Canary and Dev, since it's possible
   // that this will make the machine unstable.
   // Note that to have this code execute in a developer build,
   // then chrome::VersionInfo::CHANNEL_UNKNOWN needs to be added here.
   if (channel == chrome::VersionInfo::CHANNEL_CANARY ||
       channel == chrome::VersionInfo::CHANNEL_DEV) {
@@ skipping 28 matching lines @@
       LowMemoryObserver::SetLowMemoryMargin(100);
     } else if (trial->group() == margin_200mb) {
       LOG(WARNING) << "low_mem: Part of '200MB' experiment";
       LowMemoryObserver::SetLowMemoryMargin(200);
     } else {
       LOG(WARNING) << "low_mem: Part of 'default' experiment";
     }
   }
 }
 
+
+void ChromeBrowserMainPartsChromeos::SetupZramFieldTrial() {
+  // The dice for this experiment have been thrown at boot.  The selected group
+  // number is stored in a file.
+  const FilePath kZramGroupPath("/home/chronos/.swap_exp_enrolled");
+  std::string zram_group;
+  // If the file does not exist, the experiment has not started.  If the file
+  // exists but contains an "x", the user has a non-standard swap size and has
+  // been opted out.
+  if (!file_util::ReadFileToString(kZramGroupPath, &zram_group) ||
+      StartsWithASCII(zram_group, "x", true)) {
+    return;
+  }
+  const base::FieldTrial::Probability kDivisor = 1;  // on/off only
+  scoped_refptr<base::FieldTrial> trial =
+    base::FieldTrialList::FactoryGetFieldTrial(

[Alexei Svitkine (slow), 2013/01/08 20:30:47]

Nit: Indent 4.

+      "ZRAM", kDivisor, "default", 2013, 12, 31, NULL);

[Alexei Svitkine (slow), 2013/01/08 20:30:47]

Nit: Also indent 4 from line above.

+  // Assign probability of 1 to the group Chrome OS has picked.  Assign 0 to
+  // all other choices.
+  const char* const kGroups[] = { "0", "1", "2", "3", "4", "5", "6", "7", "8" };

[Alexei Svitkine (slow), 2013/01/08 20:38:02]

Note: I saw your earlier concern about whether these are good group names to
use. They are for the following reason. We have a script that parses source code
to extract names used in field trials so that they can be made available to the
Finch dashboard.

It doesn't work for group names created in a loop, such as here. However, we
special case simple numeric values, so that they are always available. So while
the script won't be able to parse the loop, the values you chose should still
work with the dashboard without additional intervention.

[Luigi Semenzato, 2013/01/08 21:28:19]

Good to know, thanks.

+  for (unsigned int i = 0; i < arraysize(kGroups); i++) {

[Ilya Sherman, 2013/01/08 20:16:16]

nit: Still ++i ;)

[Luigi Semenzato, 2013/01/08 21:28:19]

Aw!

+    bool match = StartsWithASCII(zram_group, kGroups[i], true);

[Alexei Svitkine (slow), 2013/01/08 20:30:47]

I don't know the details of how that file is created, but is StartsWith() the
right thing to use here? What if it the file contains "10"? Or is that
impossible?

[Alexei Svitkine (slow), 2013/01/08 20:38:02]

I notice this was somewhat covered by earlier discussion. I would suggest adding
a DCHECK() to ensure the file contents are as expected. (Do we run with DCHECKs
enabled on actual ChromeOS hardware? e.g. on dev channel?)

[Greg Spencer (Chromium), 2013/01/08 20:41:03]

No, we don't run Debug builds on any channels: the resulting build doesn't fit
on the partition.

AFAIK, Debug builds are pretty much only run by developers when debugging.

[Alexei Svitkine (slow), 2013/01/08 20:44:05]

DCHECKs can be enabled in release builds though, with via dcheck_always_on=1 gyp
flag. I was wondering if that was on for e.g. dev channel.

+    trial->AppendGroup(kGroups[i], match ? 1 : 0);
+    if (match)
+      LOG(WARNING) << "zram field trial: group " << groups[i];
+  }
+}
+
 }  //  namespace chromeos