Repository: incubator-impala Updated Branches: refs/heads/master 08eff2bc0 -> f413e236a
IMPALA-3546: don't die if sysconf() reports bogus cache info With RHEL5 on AWS EC2 for example, sysconf() returns bad info about cache line sizes. We should tolerate this instead of bringing down impalad. Change-Id: Id4d61b05fe213028a7e9aaabe98adc2792b90e07 Reviewed-on: http://gerrit.cloudera.org:8080/3111 Reviewed-by: Tim Armstrong <[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/38416eee Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/38416eee Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/38416eee Branch: refs/heads/master Commit: 38416eeeb95942b26b239c05de53dd805e0590f0 Parents: 08eff2b Author: Tim Armstrong <[email protected]> Authored: Tue May 17 15:49:57 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Mon May 23 08:40:15 2016 -0700 ---------------------------------------------------------------------- be/src/util/cpu-info.cc | 78 +++++++++++++++++++++++--------------------- be/src/util/cpu-info.h | 21 ++++-------- 2 files changed, 47 insertions(+), 52 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/38416eee/be/src/util/cpu-info.cc ---------------------------------------------------------------------- diff --git a/be/src/util/cpu-info.cc b/be/src/util/cpu-info.cc index 5a7d2bc..bb038e1 100644 --- a/be/src/util/cpu-info.cc +++ b/be/src/util/cpu-info.cc @@ -47,8 +47,6 @@ namespace impala { bool CpuInfo::initialized_ = false; int64_t CpuInfo::hardware_flags_ = 0; int64_t CpuInfo::original_hardware_flags_; -long CpuInfo::cache_sizes_[L3_CACHE + 1]; -long CpuInfo::cache_line_sizes_[L3_CACHE + 1]; int64_t CpuInfo::cycles_per_ms_; int CpuInfo::num_cores_ = 1; string CpuInfo::model_name_ = "unknown"; @@ -87,8 +85,6 @@ void CpuInfo::Init() { float max_mhz = 0; int num_cores = 0; - memset(&cache_sizes_, 0, sizeof(cache_sizes_)); - // Read from /proc/cpuinfo ifstream cpuinfo("/proc/cpuinfo", ios::in); while (cpuinfo) { @@ -117,33 +113,6 @@ void CpuInfo::Init() { } if (cpuinfo.is_open()) cpuinfo.close(); -#ifdef __APPLE__ - // On Mac OS X use sysctl() to get the cache sizes - size_t len = 0; - sysctlbyname("hw.cachesize", NULL, &len, NULL, 0); - uint64_t* data = static_cast<uint64_t*>(malloc(len)); - sysctlbyname("hw.cachesize", data, &len, NULL, 0); - DCHECK(len / sizeof(uint64_t) >= 3); - for (size_t i = 0; i < 3; ++i) { - cache_sizes_[i] = data[i]; - } - size_t linesize; - size_t sizeof_linesize = sizeof(linesize); - sysctlbyname("hw.cachelinesize", &linesize, &sizeof_linesize, NULL, 0); - for (size_t i = 0; i < 3; ++i) cache_line_sizes_[i] = linesize; -#else - // Call sysconf to query for the cache sizes - cache_sizes_[0] = sysconf(_SC_LEVEL1_DCACHE_SIZE); - cache_sizes_[1] = sysconf(_SC_LEVEL2_CACHE_SIZE); - cache_sizes_[2] = sysconf(_SC_LEVEL3_CACHE_SIZE); - - cache_line_sizes_[0] = sysconf(_SC_LEVEL1_DCACHE_LINESIZE); - // See bloom-filter.cc for one dependency. - DCHECK_EQ(cache_line_sizes_[0], 64) << "Impala expects 64-byte L1 cache lines"; - cache_line_sizes_[1] = sysconf(_SC_LEVEL2_CACHE_LINESIZE); - cache_line_sizes_[2] = sysconf(_SC_LEVEL3_CACHE_LINESIZE); -#endif - if (max_mhz != 0) { cycles_per_ms_ = max_mhz * 1000; } else { @@ -151,7 +120,6 @@ void CpuInfo::Init() { } original_hardware_flags_ = hardware_flags_; - if (num_cores > 0) { num_cores_ = num_cores; } else { @@ -181,18 +149,52 @@ void CpuInfo::EnableFeature(long flag, bool enable) { } } +void CpuInfo::GetCacheInfo(long cache_sizes[NUM_CACHE_LEVELS], + long cache_line_sizes[NUM_CACHE_LEVELS]) { +#ifdef __APPLE__ + // On Mac OS X use sysctl() to get the cache sizes + size_t len = 0; + sysctlbyname("hw.cachesize", NULL, &len, NULL, 0); + uint64_t* data = static_cast<uint64_t*>(malloc(len)); + sysctlbyname("hw.cachesize", data, &len, NULL, 0); + DCHECK(len / sizeof(uint64_t) >= 3); + for (size_t i = 0; i < NUM_CACHE_LEVELS; ++i) { + cache_sizes[i] = data[i]; + } + size_t linesize; + size_t sizeof_linesize = sizeof(linesize); + sysctlbyname("hw.cachelinesize", &linesize, &sizeof_linesize, NULL, 0); + for (size_t i = 0; i < NUM_CACHE_LEVELS; ++i) cache_line_sizes[i] = linesize; +#else + // Call sysconf to query for the cache sizes + // Note: on some systems (e.g. RHEL 5 on AWS EC2), this returns 0 instead of the + // actual cache line size. + cache_sizes[L1_CACHE] = sysconf(_SC_LEVEL1_DCACHE_SIZE); + cache_sizes[L2_CACHE] = sysconf(_SC_LEVEL2_CACHE_SIZE); + cache_sizes[L3_CACHE] = sysconf(_SC_LEVEL3_CACHE_SIZE); + + cache_line_sizes[L1_CACHE] = sysconf(_SC_LEVEL1_DCACHE_LINESIZE); + cache_line_sizes[L2_CACHE] = sysconf(_SC_LEVEL2_CACHE_LINESIZE); + cache_line_sizes[L3_CACHE] = sysconf(_SC_LEVEL3_CACHE_LINESIZE); +#endif +} + string CpuInfo::DebugString() { DCHECK(initialized_); stringstream stream; + long cache_sizes[NUM_CACHE_LEVELS]; + long cache_line_sizes[NUM_CACHE_LEVELS]; + GetCacheInfo(cache_sizes, cache_line_sizes); + string L1 = Substitute("L1 Cache: $0 (Line: $1)", - PrettyPrinter::Print(CacheSize(L1_CACHE), TUnit::BYTES), - PrettyPrinter::Print(CacheLineSize(L1_CACHE), TUnit::BYTES)); + PrettyPrinter::Print(cache_sizes[L1_CACHE], TUnit::BYTES), + PrettyPrinter::Print(cache_line_sizes[L1_CACHE], TUnit::BYTES)); string L2 = Substitute("L1 Cache: $0 (Line: $1)", - PrettyPrinter::Print(CacheSize(L2_CACHE), TUnit::BYTES), - PrettyPrinter::Print(CacheLineSize(L2_CACHE), TUnit::BYTES)); + PrettyPrinter::Print(cache_sizes[L2_CACHE], TUnit::BYTES), + PrettyPrinter::Print(cache_line_sizes[L2_CACHE], TUnit::BYTES)); string L3 = Substitute("L1 Cache: $0 (Line: $1)", - PrettyPrinter::Print(CacheSize(L3_CACHE), TUnit::BYTES), - PrettyPrinter::Print(CacheLineSize(L3_CACHE), TUnit::BYTES)); + PrettyPrinter::Print(cache_sizes[L3_CACHE], TUnit::BYTES), + PrettyPrinter::Print(cache_line_sizes[L3_CACHE], TUnit::BYTES)); stream << "Cpu Info:" << endl << " Model: " << model_name_ << endl << " Cores: " << num_cores_ << endl http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/38416eee/be/src/util/cpu-info.h ---------------------------------------------------------------------- diff --git a/be/src/util/cpu-info.h b/be/src/util/cpu-info.h index c3597f3..e47829c 100644 --- a/be/src/util/cpu-info.h +++ b/be/src/util/cpu-info.h @@ -40,6 +40,7 @@ class CpuInfo { L2_CACHE = 1, L3_CACHE = 2, }; + static const int NUM_CACHE_LEVELS = L3_CACHE + 1; /// Initialize CpuInfo. static void Init(); @@ -64,18 +65,6 @@ class CpuInfo { /// that the underlying hardware cannot support. This is useful for testing. static void EnableFeature(long flag, bool enable); - /// Returns the size of the cache in KB at this cache level - static long CacheSize(CacheLevel level) { - DCHECK(initialized_); - return cache_sizes_[level]; - } - - /// Returns the size of a line in the cache at this level. - static long CacheLineSize(CacheLevel level) { - DCHECK(initialized_); - return cache_line_sizes_[level]; - } - /// Returns the number of cpu cycles per millisecond static int64_t cycles_per_ms() { DCHECK(initialized_); @@ -97,11 +86,15 @@ class CpuInfo { static std::string DebugString(); private: + /// Populates the arguments with information about this machine's caches. + /// The values returned are not reliable in some environments, e.g. RHEL5 on EC2, so + /// so we will keep this as a private method. + static void GetCacheInfo(long cache_sizes[NUM_CACHE_LEVELS], + long cache_line_sizes[NUM_CACHE_LEVELS]); + static bool initialized_; static int64_t hardware_flags_; static int64_t original_hardware_flags_; - static long cache_sizes_[L3_CACHE + 1]; - static long cache_line_sizes_[L3_CACHE + 1]; static int64_t cycles_per_ms_; static int num_cores_; static std::string model_name_;
