[2/3] incubator-impala git commit: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings

[he...@apache.org]

IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings

Change-Id: I5e879cb35cf736f6112c1caed829722a38849794
Reviewed-on: http://gerrit.cloudera.org:8080/4528
Reviewed-by: Jim Apple <jb...@cloudera.com>
Tested-by: Internal Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/435aec54
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/435aec54
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/435aec54

Branch: refs/heads/master
Commit: 435aec5489338d35c64e6aaa748fd6cad8283215
Parents: 9313dcd
Author: Lars Volker <lv...@cloudera.com>
Authored: Fri Sep 23 12:24:30 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Sat Sep 24 03:58:38 2016 +0000

----------------------------------------------------------------------
 be/src/util/benchmark.cc |  2 ++
 be/src/util/cpu-info.cc  | 39 +++++++++++++++++++++++++++++++++++++--
 be/src/util/cpu-info.h   |  7 +++++++
 3 files changed, 46 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/435aec54/be/src/util/benchmark.cc
----------------------------------------------------------------------
diff --git a/be/src/util/benchmark.cc b/be/src/util/benchmark.cc
index 6f7901f..0001398 100644
--- a/be/src/util/benchmark.cc
+++ b/be/src/util/benchmark.cc
@@ -68,6 +68,8 @@ Benchmark::Benchmark(const string& name) : name_(name) {
 #ifndef NDEBUG
   LOG(ERROR) << "WARNING: Running benchmark in DEBUG mode.";
 #endif
+  CpuInfo::VerifyPerformanceGovernor();
+  CpuInfo::VerifyTurboDisabled();
 }
 
 int Benchmark::AddBenchmark(const string& name, BenchmarkFunction fn, void* args,

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/435aec54/be/src/util/cpu-info.cc
----------------------------------------------------------------------
diff --git a/be/src/util/cpu-info.cc b/be/src/util/cpu-info.cc
index 2a98102..ec667c4 100644
--- a/be/src/util/cpu-info.cc
+++ b/be/src/util/cpu-info.cc
@@ -45,6 +45,21 @@ DEFINE_int32(num_cores, 0, "(Advanced) If > 0, it sets the number of cores avail
     " Impala. Setting it to 0 means Impala will use all available cores on the machine"
     " according to /proc/cpuinfo.");
 
+namespace {
+// Helper function to warn if a given file does not contain an expected string as its
+// first line. If the file cannot be opened, no error is reported.
+void WarnIfFileNotEqual(
+    const string& filename, const string& expected, const string& warning_text) {
+  ifstream file(filename);
+  if (!file) return;
+  string line;
+  getline(file, line);
+  if (line != expected) {
+    LOG(ERROR) << "Expected " << expected << ", actual " << line << endl << warning_text;
+  }
+}
+} // end anonymous namespace
+
 namespace impala {
 
 bool CpuInfo::initialized_ = false;
@@ -90,7 +105,7 @@ void CpuInfo::Init() {
   int num_cores = 0;
 
   // Read from /proc/cpuinfo
-  ifstream cpuinfo("/proc/cpuinfo", ios::in);
+  ifstream cpuinfo("/proc/cpuinfo");
   while (cpuinfo) {
     getline(cpuinfo, line);
     size_t colon = line.find(':');
@@ -115,7 +130,6 @@ void CpuInfo::Init() {
       }
     }
   }
-  if (cpuinfo.is_open()) cpuinfo.close();
 
   if (max_mhz != 0) {
     cycles_per_ms_ = max_mhz * 1000;
@@ -142,6 +156,27 @@ void CpuInfo::VerifyCpuRequirements() {
   }
 }
 
+void CpuInfo::VerifyPerformanceGovernor() {
+  for (int cpu_id = 0; cpu_id < CpuInfo::num_cores(); ++cpu_id) {
+    const string governor_file =
+        Substitute("/sys/devices/system/cpu/cpu$0/cpufreq/scaling_governor", cpu_id);
+    const string warning_text = Substitute(
+        "WARNING: CPU $0 is not using 'performance' governor. Note that changing the "
+        "governor to 'performance' will reset the no_turbo setting to 0.",
+        cpu_id);
+    WarnIfFileNotEqual(governor_file, "performance", warning_text);
+  }
+}
+
+void CpuInfo::VerifyTurboDisabled() {
+  WarnIfFileNotEqual("/sys/devices/system/cpu/intel_pstate/no_turbo", "1",
+      "WARNING: CPU turbo is enabled. This setting can change the clock frequency of CPU "
+      "cores during the benchmark run, which can lead to inaccurate results. You can "
+      "disable CPU turbo by writing a 1 to "
+      "/sys/devices/system/cpu/intel_pstate/no_turbo. Note that changing the governor to "
+      "'performance' will reset this to 0.");
+}
+
 void CpuInfo::EnableFeature(long flag, bool enable) {
   DCHECK(initialized_);
   if (!enable) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/435aec54/be/src/util/cpu-info.h
----------------------------------------------------------------------
diff --git a/be/src/util/cpu-info.h b/be/src/util/cpu-info.h
index ac7f0c1..cb577c2 100644
--- a/be/src/util/cpu-info.h
+++ b/be/src/util/cpu-info.h
@@ -53,6 +53,13 @@ class CpuInfo {
   /// and terminate.
   static void VerifyCpuRequirements();
 
+  /// Determine if the CPU scaling governor is set to 'performance' and if not, issue an
+  /// error.
+  static void VerifyPerformanceGovernor();
+
+  /// Determine if CPU turbo is disabled and if not, issue an error.
+  static void VerifyTurboDisabled();
+
   /// Returns all the flags for this cpu
   static int64_t hardware_flags() {
     DCHECK(initialized_);