From 668653600cd2b8b749c5242e367da94a7ab55b00 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Fri, 17 Jul 2020 02:12:47 +0300 Subject: [PATCH] Use SettingMaxThreads only in Settings, call getNumberOfPhysicalCPUCores() instead of SettingMaxThreads::getAuto(). --- src/Common/ThreadPool.cpp | 8 ++++ src/Common/ThreadPool.h | 3 ++ src/Common/getNumberOfPhysicalCPUCores.cpp | 50 ++++++++++++---------- src/Core/SettingsCollection.cpp | 3 +- src/Databases/DatabaseOnDisk.cpp | 2 +- src/Databases/DatabaseOrdinary.cpp | 2 +- src/Interpreters/DatabaseCatalog.cpp | 2 +- 7 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/Common/ThreadPool.cpp b/src/Common/ThreadPool.cpp index 63351a77544..49516d777fb 100644 --- a/src/Common/ThreadPool.cpp +++ b/src/Common/ThreadPool.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -24,6 +25,13 @@ namespace CurrentMetrics } +template +ThreadPoolImpl::ThreadPoolImpl() + : ThreadPoolImpl(getNumberOfPhysicalCPUCores()) +{ +} + + template ThreadPoolImpl::ThreadPoolImpl(size_t max_threads_) : ThreadPoolImpl(max_threads_, max_threads_, max_threads_) diff --git a/src/Common/ThreadPool.h b/src/Common/ThreadPool.h index c1304051ea7..55796905b73 100644 --- a/src/Common/ThreadPool.h +++ b/src/Common/ThreadPool.h @@ -29,6 +29,9 @@ class ThreadPoolImpl public: using Job = std::function; + /// Maximum number of threads is based on the number of physical cores. + ThreadPoolImpl(); + /// Size is constant. Up to num_threads are created on demand and then run until shutdown. explicit ThreadPoolImpl(size_t max_threads_); diff --git a/src/Common/getNumberOfPhysicalCPUCores.cpp b/src/Common/getNumberOfPhysicalCPUCores.cpp index 3808a367f8b..13485c634e8 100644 --- a/src/Common/getNumberOfPhysicalCPUCores.cpp +++ b/src/Common/getNumberOfPhysicalCPUCores.cpp @@ -9,29 +9,33 @@ unsigned getNumberOfPhysicalCPUCores() { -#if USE_CPUID - cpu_raw_data_t raw_data; - cpu_id_t data; + static const unsigned number = [] + { +# if USE_CPUID + cpu_raw_data_t raw_data; + cpu_id_t data; - /// On Xen VMs, libcpuid returns wrong info (zero number of cores). Fallback to alternative method. - /// Also, libcpuid does not support some CPUs like AMD Hygon C86 7151. - if (0 != cpuid_get_raw_data(&raw_data) || 0 != cpu_identify(&raw_data, &data) || data.num_logical_cpus == 0) + /// On Xen VMs, libcpuid returns wrong info (zero number of cores). Fallback to alternative method. + /// Also, libcpuid does not support some CPUs like AMD Hygon C86 7151. + if (0 != cpuid_get_raw_data(&raw_data) || 0 != cpu_identify(&raw_data, &data) || data.num_logical_cpus == 0) + return std::thread::hardware_concurrency(); + + unsigned res = data.num_cores * data.total_logical_cpus / data.num_logical_cpus; + + /// Also, libcpuid gives strange result on Google Compute Engine VMs. + /// Example: + /// num_cores = 12, /// number of physical cores on current CPU socket + /// total_logical_cpus = 1, /// total number of logical cores on all sockets + /// num_logical_cpus = 24. /// number of logical cores on current CPU socket + /// It means two-way hyper-threading (24 / 12), but contradictory, 'total_logical_cpus' == 1. + + if (res != 0) + return res; +# endif + + /// As a fallback (also for non-x86 architectures) assume there are no hyper-threading on the system. + /// (Actually, only Aarch64 is supported). return std::thread::hardware_concurrency(); - - unsigned res = data.num_cores * data.total_logical_cpus / data.num_logical_cpus; - - /// Also, libcpuid gives strange result on Google Compute Engine VMs. - /// Example: - /// num_cores = 12, /// number of physical cores on current CPU socket - /// total_logical_cpus = 1, /// total number of logical cores on all sockets - /// num_logical_cpus = 24. /// number of logical cores on current CPU socket - /// It means two-way hyper-threading (24 / 12), but contradictory, 'total_logical_cpus' == 1. - - if (res != 0) - return res; -#endif - - /// As a fallback (also for non-x86 architectures) assume there are no hyper-threading on the system. - /// (Actually, only Aarch64 is supported). - return std::thread::hardware_concurrency(); + }(); + return number; } diff --git a/src/Core/SettingsCollection.cpp b/src/Core/SettingsCollection.cpp index 32bf1f29c90..ed1a16eab0e 100644 --- a/src/Core/SettingsCollection.cpp +++ b/src/Core/SettingsCollection.cpp @@ -210,8 +210,7 @@ void SettingMaxThreads::setAuto() UInt64 SettingMaxThreads::getAutoValue() { - static auto res = getNumberOfPhysicalCPUCores(); - return res; + return getNumberOfPhysicalCPUCores(); } diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 43846ff6d64..799ed041bef 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -426,7 +426,7 @@ void DatabaseOnDisk::iterateMetadataFiles(const Context & context, const Iterati } /// Read and parse metadata in parallel - ThreadPool pool(SettingMaxThreads().getAutoValue()); + ThreadPool pool; for (const auto & file : metadata_files) { pool.scheduleOrThrowOnError([&]() diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 9e7d2b52199..1e82420298b 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -152,7 +152,7 @@ void DatabaseOrdinary::loadStoredObjects(Context & context, bool has_force_resto std::atomic tables_processed{0}; std::atomic dictionaries_processed{0}; - ThreadPool pool(SettingMaxThreads().getAutoValue()); + ThreadPool pool; /// Attach tables. for (const auto & name_with_query : file_names) diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index 545d586fa67..49b79ad0314 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -560,7 +560,7 @@ void DatabaseCatalog::loadMarkedAsDroppedTables() dropped_metadata.emplace(std::move(full_path), std::move(dropped_id)); } - ThreadPool pool(SettingMaxThreads().getAutoValue()); + ThreadPool pool; for (const auto & elem : dropped_metadata) { pool.scheduleOrThrowOnError([&]()