From 11e45ba6e60544b8ffd2298a3d5e8a96d7651441 Mon Sep 17 00:00:00 2001 From: Kostis Sagonas Date: Tue, 29 Nov 2022 16:55:01 +0100 Subject: [PATCH] Name constants with a leading "k" and mixed case A C++ programming convention, also suggested by the [Google style guide](https://google.github.io/styleguide/cppguide.html#Constant_Names) and enforced by `cpplint`, is to name variables declared `constexpr` or `const`, and whose value is fixed for the duration of the program, by a leading "k" followed by mixed case. There are only two files that require changes, one in the main source, which may require some discussion, and one in the test suite, which is uncontroversial, I think. This commit fixes both of them. --- .github/workflows/cpplint.yml | 2 +- .../first_touch_distribution.hpp | 42 +++++++++---------- tests/lock.cpp | 26 ++++++------ 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/.github/workflows/cpplint.yml b/.github/workflows/cpplint.yml index 1adda990..6f8ae148 100644 --- a/.github/workflows/cpplint.yml +++ b/.github/workflows/cpplint.yml @@ -7,4 +7,4 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 - run: pip install cpplint - - run: cpplint --quiet --root=../ --filter=-build/c++11,-build/include,-build/namespaces,-runtime/array,-runtime/string,-whitespace/braces,-whitespace/indent,-whitespace/line_length,-whitespace/tab --recursive src tests + - run: cpplint --quiet --root=../ --filter=-build/c++11,-build/include,-build/namespaces,-runtime/string,-whitespace/braces,-whitespace/indent,-whitespace/line_length,-whitespace/tab --recursive src tests diff --git a/src/data_distribution/first_touch_distribution.hpp b/src/data_distribution/first_touch_distribution.hpp index 091bf80e..7ca1b90b 100644 --- a/src/data_distribution/first_touch_distribution.hpp +++ b/src/data_distribution/first_touch_distribution.hpp @@ -25,7 +25,7 @@ constexpr std::size_t single_entry_size = 1; * in the MPI backend. Changes to this must be applied in both locations * and should probably be centralized at some point. */ -constexpr std::size_t page_info_size = single_entry_size * 3; +constexpr std::size_t kPageInfoSize = single_entry_size * 3; /** * @note backend.hpp is not included here as it includes global_ptr.hpp, @@ -105,12 +105,12 @@ class first_touch_distribution : public base_distribution { public: virtual node_id_t peek_homenode(char* const ptr) { - std::size_t page_info[page_info_size]; + std::size_t page_info[kPageInfoSize]; node_id_t homenode = invalid_node_id; static const std::size_t rank = argo::backend::node_id(); static const std::size_t global_null = base_distribution::total_size + 1; const std::size_t addr = (ptr - base_distribution::start_address) / granularity * granularity; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); std::unique_lock def_lock(owners_mutex, std::defer_lock); def_lock.lock(); @@ -118,7 +118,7 @@ class first_touch_distribution : public base_distribution { update_dirs(addr, false); /* spin in case the values other than `ownership` have not been reflected to the local window */ do { - argo::backend::atomic::_load_local_owners_dir(&page_info, page_info_size, rank, owners_dir_window_index); + argo::backend::atomic::_load_local_owners_dir(&page_info, kPageInfoSize, rank, owners_dir_window_index); } while (page_info[2] != global_null && page_info[0] == global_null); def_lock.unlock(); @@ -140,7 +140,7 @@ class first_touch_distribution : public base_distribution { static const std::size_t rank = argo::backend::node_id(); static const std::size_t global_null = base_distribution::total_size + 1; const std::size_t addr = (ptr - base_distribution::start_address) / granularity * granularity; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); std::unique_lock def_lock(owners_mutex, std::defer_lock); def_lock.lock(); @@ -162,13 +162,13 @@ class first_touch_distribution : public base_distribution { } virtual std::size_t peek_local_offset(char* const ptr) { - std::size_t page_info[page_info_size]; + std::size_t page_info[kPageInfoSize]; std::size_t offset = invalid_offset; static const std::size_t rank = argo::backend::node_id(); static const std::size_t global_null = base_distribution::total_size + 1; const std::size_t drift = (ptr - base_distribution::start_address) % granularity; const std::size_t addr = (ptr - base_distribution::start_address) / granularity * granularity; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); std::unique_lock def_lock(owners_mutex, std::defer_lock); def_lock.lock(); @@ -176,7 +176,7 @@ class first_touch_distribution : public base_distribution { update_dirs(addr, false); /* spin in case the values other than `ownership` have not been reflected to the local window */ do { - argo::backend::atomic::_load_local_owners_dir(&page_info, page_info_size, rank, owners_dir_window_index); + argo::backend::atomic::_load_local_owners_dir(&page_info, kPageInfoSize, rank, owners_dir_window_index); } while (page_info[2] != global_null && page_info[1] == global_null); def_lock.unlock(); @@ -199,7 +199,7 @@ class first_touch_distribution : public base_distribution { static const std::size_t global_null = base_distribution::total_size + 1; const std::size_t drift = (ptr - base_distribution::start_address) % granularity; const std::size_t addr = (ptr - base_distribution::start_address) / granularity * granularity; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); std::unique_lock def_lock(owners_mutex, std::defer_lock); def_lock.lock(); @@ -228,7 +228,7 @@ void first_touch_distribution::update_dirs(const std::size_t& addr, std::size_t ownership; static const std::size_t rank = argo::backend::node_id(); static const std::size_t global_null = base_distribution::total_size + 1; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); const std::size_t cas_node = (addr / granularity) % base_distribution::nodes; /* fetch the ownership value for the page from the local window */ @@ -237,17 +237,17 @@ void first_touch_distribution::update_dirs(const std::size_t& addr, /* check if no info is found locally regarding the page */ if (ownership == global_null) { /* load page info from the public cas_node window */ - std::size_t page_info[page_info_size]; - argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), page_info_size, cas_node, owners_dir_window_index); + std::size_t page_info[kPageInfoSize]; + argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), kPageInfoSize, cas_node, owners_dir_window_index); /* check if any page info is found on the cas_node window */ if (!is_all_equal_to(page_info, global_null)) { /* make sure that all the remote values are read correctly */ if (rank != cas_node) { while (is_one_equal_to(page_info, global_null)) { - argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), page_info_size, cas_node, owners_dir_window_index); + argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), kPageInfoSize, cas_node, owners_dir_window_index); } /* store page info in the local window */ - argo::backend::atomic::_store_local_owners_dir(page_info, page_info_size, rank, owners_dir_window_index); + argo::backend::atomic::_store_local_owners_dir(page_info, kPageInfoSize, rank, owners_dir_window_index); } } else { if(do_first_touch) { @@ -264,7 +264,7 @@ void first_touch_distribution::first_touch(const std::size_t& addr) { std::size_t offset, homenode, result; static const std::size_t rank = argo::backend::node_id(); static const std::size_t global_null = base_distribution::total_size + 1; - const std::size_t owners_dir_window_index = page_info_size * (addr / granularity); + const std::size_t owners_dir_window_index = kPageInfoSize * (addr / granularity); /* decentralize CAS */ const std::size_t cas_node = (addr / granularity) % base_distribution::nodes; @@ -304,22 +304,22 @@ void first_touch_distribution::first_touch(const std::size_t& addr) { } /* store page info in the local window */ - std::size_t page_info[page_info_size] = {homenode, offset, rank}; - argo::backend::atomic::_store_local_owners_dir(page_info, page_info_size, rank, owners_dir_window_index); + std::size_t page_info[kPageInfoSize] = {homenode, offset, rank}; + argo::backend::atomic::_store_local_owners_dir(page_info, kPageInfoSize, rank, owners_dir_window_index); /* store page info in the remote public cas_node window */ if (rank != cas_node) { - argo::backend::atomic::_store_public_owners_dir(page_info, sizeof(std::size_t), page_info_size, cas_node, owners_dir_window_index); + argo::backend::atomic::_store_public_owners_dir(page_info, sizeof(std::size_t), kPageInfoSize, cas_node, owners_dir_window_index); } } else { /* load page info from the remote public cas_node window */ if (rank != cas_node) { - std::size_t page_info[page_info_size]; + std::size_t page_info[kPageInfoSize]; do { - argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), page_info_size, cas_node, owners_dir_window_index); + argo::backend::atomic::_load_public_owners_dir(page_info, sizeof(std::size_t), kPageInfoSize, cas_node, owners_dir_window_index); } while (is_one_equal_to(page_info, global_null)); /* store page info in the local window */ - argo::backend::atomic::_store_local_owners_dir(page_info, page_info_size, rank, owners_dir_window_index); + argo::backend::atomic::_store_local_owners_dir(page_info, kPageInfoSize, rank, owners_dir_window_index); } } } diff --git a/tests/lock.cpp b/tests/lock.cpp index 7e2cd87b..bbdf3ebb 100644 --- a/tests/lock.cpp +++ b/tests/lock.cpp @@ -30,7 +30,7 @@ constexpr std::size_t size = 1<<20; constexpr std::size_t cache_size = size; /** @brief number of threads to spawn for some of the tests */ -constexpr int nThreads = 16; +constexpr int kThreads = 16; /** @brief number of itereations to run for some of the tests */ constexpr int iter = 10000; @@ -190,21 +190,21 @@ void increment_counter2(LockType* l1, LockType* l2, int* counter) { * @brief Checks if locking is working by incrementing a shared counter */ TEST_F(LockTest, StressMCSLock) { - std::thread threads[nThreads]; + std::thread threads[kThreads]; counter = new int(0); mcs_lock = new argo::locallock::mcs_lock(); ASSERT_EQ(*counter, 0); - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i] = std::thread( increment_counter, mcs_lock, counter); } - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i].join(); } - ASSERT_EQ(iter * nThreads, *counter); + ASSERT_EQ(iter * kThreads, *counter); delete counter; delete mcs_lock; @@ -214,7 +214,7 @@ TEST_F(LockTest, StressMCSLock) { * @brief Checks if locking of multiple locks is working by incrementing a shared counter */ TEST_F(LockTest, StressMCSMultipleLocks) { - std::thread threads[nThreads]; + std::thread threads[kThreads]; int locks = 4; counter = new int(0); mcs_lock = new argo::locallock::mcs_lock[locks]; @@ -222,15 +222,15 @@ TEST_F(LockTest, StressMCSMultipleLocks) { ASSERT_EQ(*counter, 0); - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i] = std::thread(increment_counter2, &mcs_lock[i % locks], global_lock, counter); } - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i].join(); } - ASSERT_EQ(iter * nThreads, *counter); + ASSERT_EQ(iter * kThreads, *counter); delete counter; delete[] mcs_lock; @@ -241,22 +241,22 @@ TEST_F(LockTest, StressMCSMultipleLocks) { * @brief Checks locking is working by decrementing a shared counter */ TEST_F(LockTest, StressCohortLock) { - std::thread threads[nThreads]; + std::thread threads[kThreads]; counter = argo::conew_(0); cohort_lock = new argo::globallock::cohort_lock(); ASSERT_EQ(0, *counter); argo::barrier(); - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i] = std::thread( increment_counter, cohort_lock, counter); } - for (int i = 0; i < nThreads; i++) { + for (int i = 0; i < kThreads; i++) { threads[i].join(); } argo::barrier(); - ASSERT_EQ(iter * nThreads * argo::number_of_nodes(), *counter); + ASSERT_EQ(iter * kThreads * argo::number_of_nodes(), *counter); argo::codelete_(counter); delete cohort_lock; }