PerfProvider: Run a randomly selected perf command

chrome/browser/metrics/perf/perf_provider_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/metrics/perf/perf_provider_chromeos.h"
 
+#include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/histogram.h"
 #include "base/rand_util.h"
+#include "base/strings/string_split.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/metrics/perf/windowed_incognito_observer.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon_client.h"
 
 namespace metrics {
 
+namespace internal {

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

Nit: Move this below the anon namespace.

[dhsharp, 2015/10/13 23:36:22]

Done.

+
+// Hopefully we never need a space in a command argument.
+const char* kPerfCommandDelimiter = " ";

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

Nit: const char kPerfCommandDelimiter[] =

(Preferred style for Chromium and generates more efficient machine code.)

Please fix throughout.

[dhsharp, 2015/10/13 23:36:22]

Done.

+
+const char* kPerfRecordCyclesCmd =

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

These should just be in the anon namespace.

Only put things in internal:: if they need to be exposed outside of the file.

[dhsharp, 2015/10/13 23:36:22]

Done.

+  "perf record -a -e cycles -c 1000003";
+
+const char* kPerfRecordCallgraphCmd =
+  "perf record -a -e cycles -g -c 4000037";
+
+const char* kPerfRecordLBRCmd =
+  "perf record -a -e r2c4 -b -c 20011";
+
+const char* kPerfRecordInstructionTLBMissesCmd =
+  "perf record -a -e iTLB-misses -c 2003";
+
+const char* kPerfRecordDataTLBMissesCmd =
+  "perf record -a -e dTLB-misses -c 2003";
+
+const char* kPerfStatMemoryBandwidthCmd =
+  "perf stat -a -e cycles -e instructions "
+  "-e uncore_imc/data_reads/ -e uncore_imc/data_writes/ "
+  "-e cpu/event=0xD0,umask=0x11,name=MEM_UOPS_RETIRED-STLB_MISS_LOADS/ "
+  "-e cpu/event=0xD0,umask=0x12,name=MEM_UOPS_RETIRED-STLB_MISS_STORES/";
+
+const std::vector<RandomSelector::WeightAndValue> GetDefaultCommands_x86_64(
+    const CPUIdentity& cpuid) {
+  using WeightAndValue = RandomSelector::WeightAndValue;

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

Nit: Inline using declarations are not recommended.

[dhsharp, 2015/10/13 23:36:22]

This is a type alias, not a using-declaration. It's the same as a typedef, just
a little easier to read (and it can be templated, but I'm not using that here).
It's an allowed C++11 feature.

(Also: "You may use a using-declaration anywhere in a .cc file, and in
functions, methods or classes in .h files.")

[Alexei Svitkine (slow), 2015/10/14 15:53:58]

Fair enough. Metrics code generally avoids these, but I guess this use is
harmless enough. :)

+  std::vector<WeightAndValue> cmds;
+  DCHECK(cpuid.arch == "x86_64");

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

Nit: DCHECK_EQ

Also, please make file-level constants for these.

[dhsharp, 2015/10/13 23:36:22]

Done.
I don't see the point of that when they serve as their own identifier.

[Alexei Svitkine (slow), 2015/10/14 15:53:58]

Well, my reasoning was that you use it in two places. But I guess given the
second is a DCHECK based on the first, in this particular case a typo would
actually cause it to trigger, which should be good enough to leave as-is, as you
say.

+  const std::string intel_uarch = GetIntelUarch(cpuid);
+  if (intel_uarch == "IvyBridge" ||
+      intel_uarch == "Haswell" ||
+      intel_uarch == "Broadwell") {
+    cmds.push_back(WeightAndValue(60.0, kPerfRecordCyclesCmd));
+    cmds.push_back(WeightAndValue(20.0, kPerfRecordCallgraphCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordLBRCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordInstructionTLBMissesCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordDataTLBMissesCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfStatMemoryBandwidthCmd));
+    return cmds;
+  }
+  if (intel_uarch == "SandyBridge") {
+    cmds.push_back(WeightAndValue(65.0, kPerfRecordCyclesCmd));
+    cmds.push_back(WeightAndValue(20.0, kPerfRecordCallgraphCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordLBRCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordInstructionTLBMissesCmd));
+    cmds.push_back(WeightAndValue(5.0, kPerfRecordDataTLBMissesCmd));
+    return cmds;
+  }
+  // Other 64-bit x86
+  cmds.push_back(WeightAndValue(70.0, kPerfRecordCyclesCmd));
+  cmds.push_back(WeightAndValue(20.0, kPerfRecordCallgraphCmd));
+  cmds.push_back(WeightAndValue(5.0, kPerfRecordInstructionTLBMissesCmd));
+  cmds.push_back(WeightAndValue(5.0, kPerfRecordDataTLBMissesCmd));
+  return cmds;
+}
+
+const std::vector<RandomSelector::WeightAndValue> GetDefaultCommandsForCpu(
+    const CPUIdentity& cpuid) {
+  using WeightAndValue = RandomSelector::WeightAndValue;
+  std::vector<WeightAndValue> cmds;
+  if (cpuid.arch == "x86" ||     // 32-bit x86, or...
+      cpuid.arch == "armv7l") {  // ARM
+    cmds.push_back(WeightAndValue(80.0, kPerfRecordCyclesCmd));
+    cmds.push_back(WeightAndValue(20.0, kPerfRecordCallgraphCmd));
+    return cmds;
+  }
+  if (cpuid.arch == "x86_64") {  // 64-bit x86

[Alexei Svitkine (slow), 2015/10/13 17:31:19]

Nit: No {}'s

[dhsharp, 2015/10/13 23:36:22]

Done.

+    return GetDefaultCommands_x86_64(cpuid);
+  }
+  // Unknown CPUs
+  cmds.push_back(WeightAndValue(1.0, kPerfRecordCyclesCmd));
+  return cmds;
+}
+
+}  // namespace internal
+
 namespace {
 
 // Limit the total size of protobufs that can be cached, so they don't take up
 // too much memory. If the size of cached protobufs exceeds this value, stop
 // collecting further perf data. The current value is 4 MB.
 const size_t kCachedPerfDataProtobufSizeThreshold = 4 * 1024 * 1024;
 
 // This is used to space out session restore collections in the face of several
 // notifications in a short period of time. There should be no less than this
 // much time between collections.
@@ skipping 62 matching lines @@
       /* max_collection_delay = */ base::TimeDelta::FromSeconds(5)),
   /* restore_session = */ PerfProvider::CollectionParams::TriggerParams(
       /* sampling_factor = */ 10,
       /* max_collection_delay = */ base::TimeDelta::FromSeconds(10)));
 
 PerfProvider::PerfProvider()
     : collection_params_(kDefaultParameters),
       login_observer_(this),
       next_profiling_interval_start_(base::TimeTicks::Now()),
       weak_factory_(this) {
+  command_selector_.SetOdds(
+      internal::GetDefaultCommandsForCpu(GetCPUIdentity())),
+
   // Register the login observer with LoginState.
   chromeos::LoginState::Get()->AddObserver(&login_observer_);
 
   // Register as an observer of power manager events.
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
       AddObserver(this);
 
   // Register as an observer of session restore.
   on_session_restored_callback_subscription_ =
       SessionRestore::RegisterOnSessionRestoredCallback(
@@ skipping 219 matching lines @@
     AddToPerfHistogram(INCOGNITO_ACTIVE);
     return;
   }
 
   scoped_ptr<WindowedIncognitoObserver> incognito_observer(
       new WindowedIncognitoObserver);
 
   chromeos::DebugDaemonClient* client =
       chromeos::DBusThreadManager::Get()->GetDebugDaemonClient();
 
+  std::vector<std::string> command = base::SplitString(
+      command_selector_.Select(), internal::kPerfCommandDelimiter,
+      base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
   client->GetPerfOutput(
-      collection_params_.collection_duration().InSeconds(),
+      collection_params_.collection_duration().InSeconds(), command,
       base::Bind(&PerfProvider::ParseOutputProtoIfValid,
                  weak_factory_.GetWeakPtr(), base::Passed(&incognito_observer),
                  base::Passed(&sampled_profile)));
 }
 
 void PerfProvider::DoPeriodicCollection() {
   scoped_ptr<SampledProfile> sampled_profile(new SampledProfile);
   sampled_profile->set_trigger_event(SampledProfile::PERIODIC_COLLECTION);
 
   CollectIfNecessary(sampled_profile.Pass());
@@ skipping 18 matching lines @@
   scoped_ptr<SampledProfile> sampled_profile(new SampledProfile);
   sampled_profile->set_trigger_event(SampledProfile::RESTORE_SESSION);
   sampled_profile->set_ms_after_restore(time_after_restore.InMilliseconds());
   sampled_profile->set_num_tabs_restored(num_tabs_restored);
 
   CollectIfNecessary(sampled_profile.Pass());
   last_session_restore_collection_time_ = base::TimeTicks::Now();
 }
 
 }  // namespace metrics