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 <[email protected]> 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 <[email protected]> Authored: Fri Sep 23 12:24:30 2016 -0700 Committer: Internal Jenkins <[email protected]> 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_);
