Skip to content

Commit

Permalink
CI and clang-tidy updates (#50)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jrmadsen authored Jul 26, 2024
1 parent f892a93 commit 381ff12
Show file tree
Hide file tree
Showing 22 changed files with 131 additions and 93 deletions.
5 changes: 5 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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,\
Expand All @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/linux-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -72,7 +73,7 @@ jobs:
os-release: ['20.04', '22.04']

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4

- name: Install Conda
run:
Expand Down Expand Up @@ -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 &&
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/macos-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@
*~
*.bak

# clangd
/.cache
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.0.0
3.0.1
2 changes: 1 addition & 1 deletion cmake/Modules/PTLFormatting.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions examples/common/Timer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "Timer.hh"

#include <chrono>
#include <stdexcept>

using namespace PTL;
Expand Down Expand Up @@ -89,7 +90,7 @@ Timer::GetRealElapsed() const
throw std::runtime_error("Timer::GetRealElapsed() - "
"Timer not stopped or times not recorded!");
}
std::chrono::duration<double> diff = fEndRealTime - fStartRealTime;
const std::chrono::duration<double> diff = fEndRealTime - fStartRealTime;
return diff.count();
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion source/PTL/AutoLock.hh
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ private:

//========================================================================//
// standard locking
inline void _lock_deferred()
void _lock_deferred()
{
try
{
Expand Down
6 changes: 3 additions & 3 deletions source/PTL/GetEnv.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions source/PTL/Task.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

//======================================================================================//
Expand Down
20 changes: 10 additions & 10 deletions source/PTL/TaskGroup.hh
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ TaskGroup<Tp, Arg, MaxDepth>::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) !=
Expand All @@ -392,7 +392,7 @@ TaskGroup<Tp, Arg, MaxDepth>::wait()
// only want to process if within a task
if((!_is_main || tpool->size() < 2) && _within_task)
{
int bin = static_cast<int>(taskq->GetThreadBin());
const int bin = static_cast<int>(taskq->GetThreadBin());
// const auto nitr = (tpool) ? tpool->size() :
// Thread::hardware_concurrency();
while(this->pending() > 0)
Expand Down Expand Up @@ -438,8 +438,8 @@ TaskGroup<Tp, Arg, MaxDepth>::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())
{
Expand Down Expand Up @@ -480,12 +480,12 @@ TaskGroup<Tp, Arg, MaxDepth>::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();
}
Expand All @@ -502,7 +502,7 @@ TaskGroup<Tp, Arg, MaxDepth>::get_scope_destructor()
auto _count = --(_counter);
if(_count < 1)
{
AutoLock _lk{ _task_lock };
auto _lk = AutoLock{ _task_lock };
_task_cond.notify_all();
}
} };
Expand All @@ -512,15 +512,15 @@ template <typename Tp, typename Arg, intmax_t MaxDepth>
void
TaskGroup<Tp, Arg, MaxDepth>::notify()
{
AutoLock _lk{ m_task_lock };
auto _lk = AutoLock{ m_task_lock };
m_task_cond.notify_one();
}

template <typename Tp, typename Arg, intmax_t MaxDepth>
void
TaskGroup<Tp, Arg, MaxDepth>::notify_all()
{
AutoLock _lk{ m_task_lock };
auto _lk = AutoLock{ m_task_lock };
m_task_cond.notify_all();
}

Expand Down
10 changes: 4 additions & 6 deletions source/PTL/TaskManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 13 additions & 11 deletions source/PTL/ThreadPool.hh
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public:
{
static affinity_func_t _v = [](intmax_t) {
static std::atomic<intmax_t> assigned;
intmax_t _assign = assigned++;
const intmax_t _assign = assigned++;
return _assign % Thread::hardware_concurrency();
};
return _v;
Expand Down Expand Up @@ -149,13 +149,15 @@ public:
};

public:
// NOLINTBEGIN(performance-noexcept-move-constructor)
// Constructor and Destructors
explicit ThreadPool(const Config&);
~ThreadPool();
ThreadPool(const ThreadPool&) = delete;
ThreadPool(ThreadPool&&) = default;
ThreadPool& operator=(const ThreadPool&) = delete;
ThreadPool& operator=(ThreadPool&&) = default;
// NOLINTEND(performance-noexcept-move-constructor)

public:
// Public functions
Expand Down Expand Up @@ -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();
}
}
Expand All @@ -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();
}
//--------------------------------------------------------------------------------------//
Expand 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)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<size_t>(_ncore, 8);
const size_t _dmax = std::max<size_t>(_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<void()> _init_task;
_init_task = [&]() {
Expand Down Expand Up @@ -602,9 +604,9 @@ ThreadPool::execute_on_specific_threads(const std::set<std::thread::id>& _tids,
// executed the _exec function above
std::atomic<size_t> _total_exec{ 0 };
// number of cores
size_t _ncore = GetNumberOfCores();
const size_t _ncore = GetNumberOfCores();
// maximum depth for recursion
size_t _dmax = std::max<size_t>(_ncore, 8);
const size_t _dmax = std::max<size_t>(_ncore, 8);
// how many threads we need to initialize
size_t _num = _tids.size();
// create a task arena
Expand Down
2 changes: 1 addition & 1 deletion source/PTL/detail/CxxBackports.hh
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ struct Itup_cat<Index_tuple<Ind1...>, Index_tuple<Ind2...>>
template <size_t NumT>
struct Build_index_tuple
: Itup_cat<typename Build_index_tuple<NumT / 2>::__type,
typename Build_index_tuple<NumT - NumT / 2>::__type>
typename Build_index_tuple<NumT - (NumT / 2)>::__type>
{};

template <>
Expand Down
Loading

0 comments on commit 381ff12

Please sign in to comment.