Use better group names for 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"
@@ skipping 756 matching lines @@
   }
 }
 
 
 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_file_content;
   // If the file does not exist, the experiment has not started.
   if (!file_util::ReadFileToString(kZramGroupPath, &zram_file_content))

[James Cook, 2013/01/09 18:17:17]

Does this need a check for zram_file_content.empty() ?  I think the line
zram_file_content[0] will crash if the file is empty.

[Luigi Semenzato, 2013/01/09 18:52:43]

Ah, I was assuming empty file == null-terminated empty string, but this is a
std::string...

Interestingly, this code works:

    std::string s("");
    std::cout << s[0];

but the reference doesn't say anything about this, so better play it safe.

     return;
   // The file contains a single significant character, possibly followed by
   // newline.  "x" means the user has opted out.  "0" through "8" are the valid
   // group names.  (See src/platform/init/swap-exp.conf in chromiumos repo for
   // group meanings.)
-  std::string zram_group = zram_file_content.substr(0, 1);
-  if (zram_group.compare("x") == 0)
+  char zram_group = zram_file_content[0];

[Alexei Svitkine (slow), 2013/01/09 19:00:36]

Can you check that !zram_file_content.empty() somewhere before this?

[Alexei Svitkine (slow), 2013/01/09 19:01:24]

Sorry, I wrote this before you addressed James' comment.

+  if (zram_group == 'x')
     return;
+  if (zram_group < '0' || zram_group > '8') {
+    LOG(WARNING) << "zram field trial: invalid group \"" << zram_group << "\"";
+    return;
+  }
+  LOG(WARNING) << "zram field trial: group " << zram_group;

[Alexei Svitkine (slow), 2013/01/09 19:00:36]

field_trial.cc has VLOGs that can tell you this already, so you don't need a
separate one (unless vlog doesn't work for you).

[Luigi Semenzato, 2013/01/09 19:15:49]

We discussed this in the previous review.  That's a DVLOG(1) but for ease of
analyzing crashes etc. we want to always have this information in the Chrome OS
logs, since it's only one line per Chrome start.  This only affects Chrome OS,
not even the chromeos linux tests.

   const base::FieldTrial::Probability kDivisor = 1;  // on/off only
   scoped_refptr<base::FieldTrial> trial =
       base::FieldTrialList::FactoryGetFieldTrial(
           "ZRAM", kDivisor, "default", 2013, 12, 31, NULL);
   // 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" };
-  bool matched = false;
-  for (size_t i = 0; i < arraysize(kGroups); ++i) {
-    bool match = zram_group.compare(kGroups[i]) == 0;
-    trial->AppendGroup(kGroups[i], match ? 1 : 0);
-    if (match) {
-      matched = true;
-      LOG(WARNING) << "zram field trial: group " << kGroups[i];
-    }
-  }
-  if (!matched)
-    LOG(WARNING) << "zram field trial: invalid group \"" << zram_group << "\"";
+  trial->AppendGroup("2GB RAM, no swap", zram_group == '0' ? 1 : 0);
+  trial->AppendGroup("2GB RAM, 2GB swap", zram_group == '1' ? 1 : 0);
+  trial->AppendGroup("2GB RAM, 3GB swap", zram_group == '2' ? 1 : 0);
+  trial->AppendGroup("4GB RAM, no swap", zram_group == '3' ? 1 : 0);
+  trial->AppendGroup("4GB RAM, 4GB swap", zram_group == '4' ? 1 : 0);
+  trial->AppendGroup("4GB RAM, 6GB swap", zram_group == '5' ? 1 : 0);
+  trial->AppendGroup("snow, no swap", zram_group == '6' ? 1 : 0);
+  trial->AppendGroup("snow, 1GB swap", zram_group == '7' ? 1 : 0);
+  trial->AppendGroup("snow, 2GB swap", zram_group == '8' ? 1 : 0);
 }
 
 }  //  namespace chromeos