From 381ff121174825044ad511435c607650333d7996 Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Fri, 26 Jul 2024 15:44:48 -0500 Subject: [PATCH] CI and clang-tidy updates (#50) * Update workflows - Update macos-ci.yml - remove macos-11 - add macos-13 - bump versions for actions/checkout and actions/setup-python * Misc clang-tidy fixes * Bump patch version * Bump clang-format to version 11 * Remove noexcept from ThreadPool move ctor/assign --- .clang-tidy | 5 +++ .github/workflows/formatting.yml | 10 ++--- .github/workflows/linux-ci.yml | 4 +- .github/workflows/macos-ci.yml | 4 +- .gitignore | 2 + CMakeLists.txt | 3 ++ VERSION | 2 +- cmake/Modules/PTLFormatting.cmake | 2 +- examples/common/Timer.cc | 7 ++-- source/PTL/AutoLock.hh | 2 +- source/PTL/GetEnv.hh | 6 +-- source/PTL/Task.hh | 7 ++-- source/PTL/TaskGroup.hh | 20 +++++----- source/PTL/TaskManager.hh | 10 ++--- source/PTL/ThreadPool.hh | 24 ++++++------ source/PTL/detail/CxxBackports.hh | 2 +- source/TaskGroup.cc | 3 ++ source/TaskRunManager.cc | 3 ++ source/ThreadPool.cc | 64 +++++++++++++++++-------------- source/Threading.cc | 10 +++-- source/UserTaskQueue.cc | 32 +++++++++------- source/VUserTaskQueue.cc | 2 +- 22 files changed, 131 insertions(+), 93 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 29d24e8..7dc5fe0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -20,9 +20,13 @@ modernize-*,\ -modernize-use-nodiscard,\ -modernize-make-unique,\ performance-*,\ +-performance-enum-size,\ +-performance-avoid-endl,\ readability-*,\ +-readability-avoid-return-with-void-value,\ -readability-function-size,\ -readability-identifier-naming,\ +-readability-identifier-length,\ -readability-implicit-bool-cast,\ -readability-inconsistent-declaration-parameter-name,\ -readability-named-parameter,\ @@ -40,6 +44,7 @@ readability-*,\ -readability-const-return-type,\ -readability-function-cognitive-complexity,\ -readability-redundant-access-specifiers,\ +-readability-use-anyofallof,\ " HeaderFilterRegex: 'source/PTL/.*\.(hh|icc)$' CheckOptions: diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 01b119d..f7ff697 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -16,18 +16,18 @@ jobs: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install dependencies run: | sudo apt-get update - sudo apt-get install -y clang-format-9 cmake + sudo apt-get install -y clang-format-11 cmake - name: clang-format run: | cmake -B build-PTL -DPTL_USE_TBB=OFF . cmake --build build-PTL --target format-source rm -rf build-PTL if [ $(git diff | wc -l) -gt 0 ]; then - echo -e "\nError! Source code not formatted. Run clang-format-9...\n" + echo -e "\nError! Source code not formatted. Run clang-format-11...\n" echo -e "\nFiles:\n" git diff --name-only echo -e "\nFull diff:\n" @@ -42,9 +42,9 @@ jobs: python-version: [3.8] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install dependencies diff --git a/.github/workflows/linux-ci.yml b/.github/workflows/linux-ci.yml index d3a890f..933b1ca 100644 --- a/.github/workflows/linux-ci.yml +++ b/.github/workflows/linux-ci.yml @@ -46,6 +46,7 @@ jobs: compiler: 'clang-12' extra_args: '--num-tasks=256 --tbb --sanitizer --sanitizer-type=thread --static-analysis' extra_cmake: '-DPTL_USE_LOCKS=ON' + extra_ctest: '' extra_packages: 'clang-tidy-12' standard: '17' os-release: '20.04' @@ -72,7 +73,7 @@ jobs: os-release: ['20.04', '22.04'] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install Conda run: @@ -141,6 +142,7 @@ jobs: -- -DCMAKE_INSTALL_PREFIX=${HOME}/ptl-install -DCMAKE_CXX_STANDARD=${{ matrix.standard }} ${{ matrix.extra_cmake }} - name: Install + if: ${{ !contains(matrix.extra_args, 'sanitizer') }} run: export PATH="/opt/conda/bin:${PATH}" && source activate && diff --git a/.github/workflows/macos-ci.yml b/.github/workflows/macos-ci.yml index 579a8c6..5c1bf24 100644 --- a/.github/workflows/macos-ci.yml +++ b/.github/workflows/macos-ci.yml @@ -19,10 +19,10 @@ jobs: compiler: ["clang-14", "clang-15"] extra_args: ["--num-tasks=256 --tbb"] extra_packages: ["clangdev tbb-devel"] - os-release: ["macos-12", "macos-11"] + os-release: ["macos-12", "macos-13"] standard: ["11", "17"] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install Conda run: | diff --git a/.gitignore b/.gitignore index 253645e..2b8db1f 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,5 @@ *~ *.bak +# clangd +/.cache diff --git a/CMakeLists.txt b/CMakeLists.txt index 815c5c4..00884cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,6 +99,9 @@ include(PTLPackageConfigHelpers) if(EXISTS ${CMAKE_CURRENT_LIST_DIR}/examples) ptl_add_option(PTL_BUILD_EXAMPLES "Build examples" OFF) if(PTL_BUILD_EXAMPLES) + if(CMAKE_CXX_CLANG_TIDY) + set(CMAKE_CXX_CLANG_TIDY) + endif() set(PTL_DIR ${CMAKE_BINARY_DIR}) add_subdirectory(examples) endif() diff --git a/VERSION b/VERSION index 4a36342..cb2b00e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.0.0 +3.0.1 diff --git a/cmake/Modules/PTLFormatting.cmake b/cmake/Modules/PTLFormatting.cmake index 6ea3499..787e9b3 100644 --- a/cmake/Modules/PTLFormatting.cmake +++ b/cmake/Modules/PTLFormatting.cmake @@ -3,7 +3,7 @@ if(PTL_CLANG_FORMATTER MATCHES ".*-6") unset(PTL_CLANG_FORMATTER CACHE) endif() -find_program(PTL_CLANG_FORMATTER NAMES clang-format-9 clang-format-mp-9.0 clang-format) +find_program(PTL_CLANG_FORMATTER NAMES clang-format-11 clang-format-mp-11.0 clang-format) mark_as_advanced(PTL_CLANG_FORMATTER) find_program(PTL_CMAKE_FORMATTER NAMES cmake-format) diff --git a/examples/common/Timer.cc b/examples/common/Timer.cc index 473a707..b5d4f69 100644 --- a/examples/common/Timer.cc +++ b/examples/common/Timer.cc @@ -24,6 +24,7 @@ #include "Timer.hh" +#include #include using namespace PTL; @@ -89,7 +90,7 @@ Timer::GetRealElapsed() const throw std::runtime_error("Timer::GetRealElapsed() - " "Timer not stopped or times not recorded!"); } - std::chrono::duration diff = fEndRealTime - fStartRealTime; + const std::chrono::duration diff = fEndRealTime - fStartRealTime; return diff.count(); } @@ -103,7 +104,7 @@ Timer::GetSystemElapsed() const throw std::runtime_error("Timer::GetSystemElapsed() - " "Timer not stopped or times not recorded!"); } - double diff = fEndTimes.tms_stime - fStartTimes.tms_stime; + const double diff = fEndTimes.tms_stime - fStartTimes.tms_stime; return diff / sysconf(_SC_CLK_TCK); } @@ -117,7 +118,7 @@ Timer::GetUserElapsed() const throw std::runtime_error("Timer::GetUserElapsed() - " "Timer not stopped or times not recorded!"); } - double diff = fEndTimes.tms_utime - fStartTimes.tms_utime; + const double diff = fEndTimes.tms_utime - fStartTimes.tms_utime; return diff / sysconf(_SC_CLK_TCK); } diff --git a/source/PTL/AutoLock.hh b/source/PTL/AutoLock.hh index 7580766..42973a1 100644 --- a/source/PTL/AutoLock.hh +++ b/source/PTL/AutoLock.hh @@ -401,7 +401,7 @@ private: //========================================================================// // standard locking - inline void _lock_deferred() + void _lock_deferred() { try { diff --git a/source/PTL/GetEnv.hh b/source/PTL/GetEnv.hh index f1a4162..fd98db0 100644 --- a/source/PTL/GetEnv.hh +++ b/source/PTL/GetEnv.hh @@ -49,9 +49,9 @@ GetEnv(const std::string& env_id, Tp _default = Tp()) char* env_var = std::getenv(env_id.c_str()); if(env_var) { - std::string str_var = std::string(env_var); - std::istringstream iss(str_var); - Tp var = Tp(); + const auto str_var = std::string{ env_var }; + auto iss = std::istringstream{ str_var }; + auto var = Tp{}; iss >> var; return var; } diff --git a/source/PTL/Task.hh b/source/PTL/Task.hh index 54e3bac..c8cfd92 100644 --- a/source/PTL/Task.hh +++ b/source/PTL/Task.hh @@ -68,9 +68,10 @@ public: public: // execution operator - virtual future_type get_future() = 0; - virtual void wait() = 0; - virtual RetT get() = 0; + void operator()() override = 0; + virtual future_type get_future() = 0; + virtual void wait() = 0; + virtual RetT get() = 0; }; //======================================================================================// diff --git a/source/PTL/TaskGroup.hh b/source/PTL/TaskGroup.hh index 1526e33..a0354d1 100644 --- a/source/PTL/TaskGroup.hh +++ b/source/PTL/TaskGroup.hh @@ -377,8 +377,8 @@ TaskGroup::wait() ThreadPool* tpool = (m_pool) ? m_pool : data->thread_pool; VUserTaskQueue* taskq = (tpool) ? tpool->get_queue() : data->current_queue; - bool _is_main = data->is_main; - bool _within_task = data->within_task; + bool _is_main = data->is_main; + const bool _within_task = data->within_task; auto is_active_state = [&]() { return (tpool->state()->load(std::memory_order_relaxed) != @@ -392,7 +392,7 @@ TaskGroup::wait() // only want to process if within a task if((!_is_main || tpool->size() < 2) && _within_task) { - int bin = static_cast(taskq->GetThreadBin()); + const int bin = static_cast(taskq->GetThreadBin()); // const auto nitr = (tpool) ? tpool->size() : // Thread::hardware_concurrency(); while(this->pending() > 0) @@ -438,8 +438,8 @@ TaskGroup::wait() } } - intmax_t wake_size = 2; - AutoLock _lock(m_task_lock, std::defer_lock); + const intmax_t wake_size = 2; + AutoLock _lock(m_task_lock, std::defer_lock); while(is_active_state()) { @@ -480,12 +480,12 @@ TaskGroup::wait() if(_lock.owns_lock()) _lock.unlock(); - intmax_t ntask = this->task_count().load(); + const intmax_t ntask = this->task_count().load(); if(ntask > 0) { std::stringstream ss; ss << "\nWarning! Join operation issue! " << ntask << " tasks still " - << "are running!" << std::endl; + << "are running!\n"; std::cerr << ss.str(); this->wait(); } @@ -502,7 +502,7 @@ TaskGroup::get_scope_destructor() auto _count = --(_counter); if(_count < 1) { - AutoLock _lk{ _task_lock }; + auto _lk = AutoLock{ _task_lock }; _task_cond.notify_all(); } } }; @@ -512,7 +512,7 @@ template void TaskGroup::notify() { - AutoLock _lk{ m_task_lock }; + auto _lk = AutoLock{ m_task_lock }; m_task_cond.notify_one(); } @@ -520,7 +520,7 @@ template void TaskGroup::notify_all() { - AutoLock _lk{ m_task_lock }; + auto _lk = AutoLock{ m_task_lock }; m_task_cond.notify_all(); } diff --git a/source/PTL/TaskManager.hh b/source/PTL/TaskManager.hh index 0225050..b238da6 100644 --- a/source/PTL/TaskManager.hh +++ b/source/PTL/TaskManager.hh @@ -70,15 +70,15 @@ public: public: //------------------------------------------------------------------------// // return the thread pool - inline ThreadPool* thread_pool() const { return m_pool; } + ThreadPool* thread_pool() const { return m_pool; } //------------------------------------------------------------------------// // return the number of threads in the thread pool - inline size_type size() const { return (m_pool) ? m_pool->size() : 0; } + size_type size() const { return (m_pool) ? m_pool->size() : 0; } //------------------------------------------------------------------------// // kill all the threads - inline void finalize() + void finalize() { if(m_is_finalized) return; @@ -237,9 +237,7 @@ PTL::TaskManager::GetInstance() { if(!fgInstance()) { - auto nthreads = std::thread::hardware_concurrency(); - std::cout << "Allocating mad::TaskManager with " << nthreads << " thread(s)..." - << std::endl; + // auto nthreads = std::thread::hardware_concurrency(); new TaskManager(TaskRunManager::GetMasterRunManager()->GetThreadPool()); } return fgInstance(); diff --git a/source/PTL/ThreadPool.hh b/source/PTL/ThreadPool.hh index 2b643b5..b1dd756 100644 --- a/source/PTL/ThreadPool.hh +++ b/source/PTL/ThreadPool.hh @@ -116,7 +116,7 @@ public: { static affinity_func_t _v = [](intmax_t) { static std::atomic assigned; - intmax_t _assign = assigned++; + const intmax_t _assign = assigned++; return _assign % Thread::hardware_concurrency(); }; return _v; @@ -149,6 +149,7 @@ public: }; public: + // NOLINTBEGIN(performance-noexcept-move-constructor) // Constructor and Destructors explicit ThreadPool(const Config&); ~ThreadPool(); @@ -156,6 +157,7 @@ public: ThreadPool(ThreadPool&&) = default; ThreadPool& operator=(const ThreadPool&) = delete; ThreadPool& operator=(ThreadPool&&) = default; + // NOLINTEND(performance-noexcept-move-constructor) public: // Public functions @@ -303,7 +305,7 @@ ThreadPool::notify() // wake up one thread that is waiting for a task to be available if(m_thread_awake->load() < m_pool_size) { - AutoLock l(*m_task_lock); + auto l = AutoLock{ *m_task_lock }; m_task_cond->notify_one(); } } @@ -312,7 +314,7 @@ inline void ThreadPool::notify_all() { // wake all threads - AutoLock l(*m_task_lock); + auto l = AutoLock{ *m_task_lock }; m_task_cond->notify_all(); } //--------------------------------------------------------------------------------------// @@ -325,7 +327,7 @@ ThreadPool::notify(size_type ntasks) // wake up as many threads that tasks just added if(m_thread_awake->load() < m_pool_size) { - AutoLock l(*m_task_lock); + auto l = AutoLock{ *m_task_lock }; if(ntasks < this->size()) { for(size_type i = 0; i < ntasks; ++i) @@ -354,10 +356,10 @@ ThreadPool::get_task_arena() // create a task arena if(!m_tbb_task_arena) { - auto _sz = (tbb_global_control()) - ? tbb_global_control()->active_value( + auto _sz = (tbb_global_control()) + ? tbb_global_control()->active_value( tbb::global_control::max_allowed_parallelism) - : size(); + : size(); m_tbb_task_arena = new tbb_task_arena_t(::tbb::task_arena::attach{}); m_tbb_task_arena->initialize(_sz, 1); } @@ -492,9 +494,9 @@ ThreadPool::execute_on_all_threads(FuncT&& _func) // number of cores size_t _ncore = GetNumberOfCores(); // maximum depth for recursion - size_t _dmax = std::max(_ncore, 8); + const size_t _dmax = std::max(_ncore, 8); // how many threads we need to initialize - size_t _num = std::min(_maxp, std::min(_sz, _ncore)); + const size_t _num = std::min({ _maxp, _sz, _ncore }); // this is the task passed to the task-group std::function _init_task; _init_task = [&]() { @@ -602,9 +604,9 @@ ThreadPool::execute_on_specific_threads(const std::set& _tids, // executed the _exec function above std::atomic _total_exec{ 0 }; // number of cores - size_t _ncore = GetNumberOfCores(); + const size_t _ncore = GetNumberOfCores(); // maximum depth for recursion - size_t _dmax = std::max(_ncore, 8); + const size_t _dmax = std::max(_ncore, 8); // how many threads we need to initialize size_t _num = _tids.size(); // create a task arena diff --git a/source/PTL/detail/CxxBackports.hh b/source/PTL/detail/CxxBackports.hh index 07014c4..9beefd8 100644 --- a/source/PTL/detail/CxxBackports.hh +++ b/source/PTL/detail/CxxBackports.hh @@ -61,7 +61,7 @@ struct Itup_cat, Index_tuple> template struct Build_index_tuple : Itup_cat::__type, - typename Build_index_tuple::__type> + typename Build_index_tuple::__type> {}; template <> diff --git a/source/TaskGroup.cc b/source/TaskGroup.cc index 33c5d72..7698633 100644 --- a/source/TaskGroup.cc +++ b/source/TaskGroup.cc @@ -33,6 +33,9 @@ #include "PTL/TaskRunManager.hh" #include "PTL/ThreadData.hh" +#include +#include + //======================================================================================// namespace PTL diff --git a/source/TaskRunManager.cc b/source/TaskRunManager.cc index 6cd190a..0325247 100644 --- a/source/TaskRunManager.cc +++ b/source/TaskRunManager.cc @@ -23,6 +23,9 @@ #include "PTL/TaskManager.hh" #include "PTL/ThreadPool.hh" +#include +#include + namespace PTL { //======================================================================================// diff --git a/source/ThreadPool.cc b/source/ThreadPool.cc index 161aa0d..dccca3b 100644 --- a/source/ThreadPool.cc +++ b/source/ThreadPool.cc @@ -33,14 +33,22 @@ #include "PTL/ScopeDestructor.hh" #include "PTL/ThreadData.hh" #include "PTL/Threading.hh" +#include "PTL/Types.hh" #include "PTL/UserTaskQueue.hh" #include "PTL/VUserTaskQueue.hh" +#include #include +#include +#include +#include +#include #include #include +#include #include #include +#include //======================================================================================// @@ -82,13 +90,13 @@ ThreadPool::start_thread(ThreadPool* tp, thread_data_t* _data, intmax_t _idx) { if(tp->get_verbose() > 0) { - AutoLock lock(TypeMutex()); - std::cerr << "[PTL::ThreadPool] Starting thread " << _idx << "..." << std::endl; + auto lock = AutoLock{ TypeMutex() }; + std::cerr << "[PTL::ThreadPool] Starting thread " << _idx << "...\n"; } auto _thr_data = std::make_shared(tp); { - AutoLock lock(TypeMutex(), std::defer_lock); + auto lock = AutoLock{ TypeMutex(), std::defer_lock }; if(!lock.owns_lock()) lock.lock(); if(_idx < 0) @@ -104,9 +112,8 @@ ThreadPool::start_thread(ThreadPool* tp, thread_data_t* _data, intmax_t _idx) if(tp->get_verbose() > 0) { - AutoLock lock(TypeMutex()); - std::cerr << "[PTL::ThreadPool] Thread " << _idx << " terminating..." - << std::endl; + auto lock = AutoLock{ TypeMutex() }; + std::cerr << "[PTL::ThreadPool] Thread " << _idx << " terminating...\n"; } } @@ -183,8 +190,8 @@ ThreadPool::ThreadPool(const Config& _cfg) auto master_id = get_this_thread_id(); if(master_id != 0 && m_verbose > 1) { - AutoLock lock(TypeMutex()); - std::cerr << "[PTL::ThreadPool] ThreadPool created on worker thread" << std::endl; + auto lock = AutoLock{ TypeMutex() }; + std::cerr << "[PTL::ThreadPool] ThreadPool created on worker thread\n"; } thread_data() = new ThreadData(this); @@ -202,8 +209,7 @@ ThreadPool::~ThreadPool() { std::cerr << "Warning! ThreadPool was not properly destroyed! Call " "destroy_threadpool() before deleting the ThreadPool object to " - "eliminate this message." - << std::endl; + "eliminate this message.\n"; m_pool_state->store(thread_pool::state::STOPPED); m_task_lock->lock(); m_task_cond->notify_all(); @@ -252,11 +258,11 @@ ThreadPool::set_affinity(intmax_t i, Thread& _thread) const { try { - NativeThread native_thread = _thread.native_handle(); - intmax_t _pin = m_affinity_func(i); + NativeThread native_thread = _thread.native_handle(); + const intmax_t _pin = m_affinity_func(i); if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] Setting pin affinity for thread " << get_thread_id(_thread.get_id()) << " to " << _pin << std::endl; } @@ -278,7 +284,7 @@ ThreadPool::set_priority(int _prio, Thread& _thread) const NativeThread native_thread = _thread.native_handle(); if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] Setting thread " << get_thread_id(_thread.get_id()) << " priority to " << _prio << std::endl; @@ -286,7 +292,7 @@ ThreadPool::set_priority(int _prio, Thread& _thread) const SetThreadPriority(_prio, native_thread); } catch(std::runtime_error& e) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] Error setting thread priority: " << e.what() << std::endl; } @@ -324,11 +330,13 @@ ThreadPool::initialize_threadpool(size_type proposed_size) if(!_global_control) { + // NOLINTBEGIN(misc-include-cleaner) _global_control = new tbb_global_control_t( tbb::global_control::max_allowed_parallelism, proposed_size + 1); + // NOLINTEND(misc-include-cleaner) if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] ThreadPool [TBB] initialized with " << m_pool_size << " threads." << std::endl; } @@ -357,7 +365,7 @@ ThreadPool::initialize_threadpool(size_type proposed_size) ; if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] ThreadPool initialized with " << m_pool_size << " threads." << std::endl; } @@ -376,7 +384,7 @@ ThreadPool::initialize_threadpool(size_type proposed_size) { if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "ThreadPool initialized with " << m_pool_size << " threads." << std::endl; } @@ -392,7 +400,7 @@ ThreadPool::initialize_threadpool(size_type proposed_size) //--------------------------------------------------------------------// // reserve enough space to prevent realloc later { - AutoLock _task_lock(*m_task_lock); + auto _task_lock = AutoLock{ *m_task_lock }; m_is_joined.reserve(proposed_size); } @@ -425,20 +433,20 @@ ThreadPool::initialize_threadpool(size_type proposed_size) m_threads.emplace_back(std::move(thr)); } catch(std::runtime_error& e) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] " << e.what() << std::endl; // issue creating thread continue; } catch(std::bad_alloc& e) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] " << e.what() << std::endl; continue; } } //------------------------------------------------------------------------// - AutoLock _task_lock(*m_task_lock); + auto _task_lock = AutoLock{ *m_task_lock }; // thread pool size doesn't match with join vector // this will screw up joining later @@ -454,7 +462,7 @@ ThreadPool::initialize_threadpool(size_type proposed_size) if(m_verbose > 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] ThreadPool initialized with " << m_pool_size << " threads." << std::endl; } @@ -499,7 +507,7 @@ ThreadPool::destroy_threadpool() delete _global_control; _global_control = nullptr; m_tbb_tp = false; - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; if(m_verbose > 0) { std::cerr << "[PTL::ThreadPool] ThreadPool [TBB] destroyed" << std::endl; @@ -580,12 +588,12 @@ ThreadPool::destroy_threadpool() { if(_active == 0) { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] ThreadPool destroyed" << std::endl; } else { - AutoLock lock(TypeMutex()); + auto lock = AutoLock{ TypeMutex() }; std::cerr << "[PTL::ThreadPool] ThreadPool destroyed but " << _active << " threads might still be active (and cause a termination error)" << std::endl; @@ -623,7 +631,7 @@ ThreadPool::stop_thread() ; // lock up the task queue - AutoLock _task_lock(*m_task_lock); + auto _task_lock = AutoLock{ *m_task_lock }; while(!m_stop_threads.empty()) { @@ -668,7 +676,7 @@ ThreadPool::execute_thread(VUserTaskQueue* _task_queue) // initialization function m_init_func(); // finalization function (executed when scope is destroyed) - ScopeDestructor _fini{ [this]() { m_fini_func(); } }; + auto _fini = ScopeDestructor{ [this]() { m_fini_func(); } }; ThreadId tid = ThisThread::get_id(); ThreadData* data = thread_data(); diff --git a/source/Threading.cc b/source/Threading.cc index 373d471..2b4d31f 100644 --- a/source/Threading.cc +++ b/source/Threading.cc @@ -24,8 +24,9 @@ #include "PTL/Threading.hh" -#include "PTL/ConsumeParameters.hh" +#include "PTL/ConsumeParameters.hh" // NOLINT(misc-include-cleaner) #include "PTL/Macros.hh" +#include "PTL/Types.hh" #if defined(PTL_WINDOWS) # include @@ -37,10 +38,12 @@ #if defined(PTL_LINUX) # include +# include +# include # include #endif -#include +#include #include //======================================================================================// @@ -93,7 +96,7 @@ GetNumberOfPhysicalCpus() break; if(line.find("core id") != std::string::npos) { - for(std::string itr : { "core id", ":", " ", "\t" }) + for(const std::string& itr : { "core id", ":", " ", "\t" }) { static auto _npos = std::string::npos; auto _pos = _npos; @@ -166,6 +169,7 @@ SetPinAffinity(int _cpu, NativeThread& _t) cpu_set_t _cpu_set{}; CPU_ZERO(&_cpu_set); CPU_SET(_cpu, &_cpu_set); + // NOLINTNEXTLINE(misc-include-cleaner) return (pthread_setaffinity_np(static_cast(_t), sizeof(cpu_set_t), &_cpu_set) == 0); #else // Not available for Mac, WIN,... diff --git a/source/UserTaskQueue.cc b/source/UserTaskQueue.cc index 9ea4318..d0af18e 100644 --- a/source/UserTaskQueue.cc +++ b/source/UserTaskQueue.cc @@ -30,15 +30,21 @@ #include "PTL/TaskGroup.hh" #include "PTL/ThreadData.hh" #include "PTL/ThreadPool.hh" +#include "PTL/Types.hh" +#include #include #include +#include #include #include #include +#include +#include #include #include #include +#include namespace PTL { @@ -88,7 +94,7 @@ UserTaskQueue::resize(intmax_t n) { if(!m_mutex) throw std::runtime_error("nullptr to mutex"); - AutoLock lk(m_mutex); + auto lk = AutoLock{ m_mutex }; if(m_workers < n) { while(m_workers < n) @@ -121,7 +127,7 @@ intmax_t UserTaskQueue::GetThreadBin() const { // get a thread id number - static thread_local intmax_t tl_bin = + static thread_local const intmax_t tl_bin = (m_thread_bin + ThreadPool::get_this_thread_id()) % (m_workers + 1); return tl_bin; } @@ -139,9 +145,9 @@ UserTaskQueue::GetInsertBin() const UserTaskQueue::task_pointer UserTaskQueue::GetThreadBinTask() { - intmax_t tbin = GetThreadBin(); - TaskSubQueue* task_subq = (*m_subqueues)[tbin % (m_workers + 1)]; - task_pointer _task = nullptr; + const intmax_t tbin = GetThreadBin(); + TaskSubQueue* task_subq = (*m_subqueues)[tbin % (m_workers + 1)]; + task_pointer _task = nullptr; //------------------------------------------------------------------------// auto get_task = [&]() { @@ -234,8 +240,8 @@ UserTaskQueue::InsertTask(task_pointer&& task, ThreadData* data, intmax_t subq) // increment number of tasks ++(*m_ntasks); - bool spin = m_hold->load(std::memory_order_relaxed); - intmax_t tbin = GetThreadBin(); + const bool spin = m_hold->load(std::memory_order_relaxed); + const intmax_t tbin = GetThreadBin(); if(data && data->within_task) { @@ -326,8 +332,8 @@ UserTaskQueue::ExecuteOnAllThreads(ThreadPool* tp, function_type func) //--------------------------------------------------------------------// auto thread_specific_func = [&]() { - ScopeDestructor _dtor = tg.get_scope_destructor(); - static Mutex _mtx; + auto _dtor = tg.get_scope_destructor(); + static Mutex _mtx; _mtx.lock(); bool& _executed = thread_execute_map[GetThreadBin()]; _mtx.unlock(); @@ -345,7 +351,7 @@ UserTaskQueue::ExecuteOnAllThreads(ThreadPool* tp, function_type func) } tp->notify_all(); - int nexecuted = tg.join(); + const int nexecuted = tg.join(); if(nexecuted != m_workers) { std::stringstream msg; @@ -384,8 +390,8 @@ UserTaskQueue::ExecuteOnSpecificThreads(ThreadIdSet tid_set, ThreadPool* tp, // wrap the function so that it will only be executed if the thread // has an ID in the set auto thread_specific_func = [&]() { - ScopeDestructor _dtor = tg.get_scope_destructor(); - static Mutex _mtx; + auto _dtor = tg.get_scope_destructor(); + static Mutex _mtx; _mtx.lock(); bool& _executed = thread_execute_map[GetThreadBin()]; _mtx.unlock(); @@ -411,7 +417,7 @@ UserTaskQueue::ExecuteOnSpecificThreads(ThreadIdSet tid_set, ThreadPool* tp, InsertTask(tg.wrap(thread_specific_func), ThreadData::GetInstance(), i); } tp->notify_all(); - decltype(tid_set.size()) nexecuted = tg.join(); + const decltype(tid_set.size()) nexecuted = tg.join(); if(nexecuted != tid_set.size()) { std::stringstream msg; diff --git a/source/VUserTaskQueue.cc b/source/VUserTaskQueue.cc index 4785d23..1fb7d78 100644 --- a/source/VUserTaskQueue.cc +++ b/source/VUserTaskQueue.cc @@ -41,7 +41,7 @@ VUserTaskQueue::VUserTaskQueue(intmax_t nworkers) { TaskRunManager* rm = TaskRunManager::GetMasterRunManager(); m_workers = (rm) ? rm->GetNumberOfThreads() + 1 // number of threads + 1 - : (2 * std::thread::hardware_concurrency()) + 1; + : (2 * std::thread::hardware_concurrency()) + 1; // hyperthreads + 1 } }