Skip to content

Name constants with a leading "k" and mixed case #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kostis wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Name constants with a leading "k" and mixed case #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11e45ba
Name constants with a leading "k" and mixed case
kostis Nov 29, 2022
8ea0a16
Merge branch 'master' into konstant-names
kostis Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/cpplint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,-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,-runtime/string,-whitespace/braces,-whitespace/indent,-whitespace/line_length,-whitespace/tab --recursive src tests
42 changes: 21 additions & 21 deletions src/data_distribution/first_touch_distribution.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -105,20 +105,20 @@ class first_touch_distribution : public base_distribution<instance> {

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<instance>::total_size + 1;
const std::size_t addr = (ptr - base_distribution<instance>::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<std::mutex> def_lock(owners_mutex, std::defer_lock);
def_lock.lock();
/* handle all the necessary directory actions for the global address */
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();

Expand All @@ -139,7 +139,7 @@ class first_touch_distribution : public base_distribution<instance> {
static const std::size_t rank = argo::backend::node_id();
static const std::size_t global_null = base_distribution<instance>::total_size + 1;
const std::size_t addr = (ptr - base_distribution<instance>::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<std::mutex> def_lock(owners_mutex, std::defer_lock);
def_lock.lock();
Expand All @@ -160,21 +160,21 @@ class first_touch_distribution : public base_distribution<instance> {
}

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<instance>::total_size + 1;
const std::size_t drift = (ptr - base_distribution<instance>::start_address) % granularity;
const std::size_t addr = (ptr - base_distribution<instance>::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<std::mutex> def_lock(owners_mutex, std::defer_lock);
def_lock.lock();
/* handle all the necessary directory actions for the global address */
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();

Expand All @@ -196,7 +196,7 @@ class first_touch_distribution : public base_distribution<instance> {
static const std::size_t global_null = base_distribution<instance>::total_size + 1;
const std::size_t drift = (ptr - base_distribution<instance>::start_address) % granularity;
const std::size_t addr = (ptr - base_distribution<instance>::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<std::mutex> def_lock(owners_mutex, std::defer_lock);
def_lock.lock();
Expand Down Expand Up @@ -224,7 +224,7 @@ void first_touch_distribution<instance>::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<instance>::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<instance>::nodes;

/* fetch the ownership value for the page from the local window */
Expand All @@ -233,17 +233,17 @@ void first_touch_distribution<instance>::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) {
Expand All @@ -260,7 +260,7 @@ void first_touch_distribution<instance>::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<instance>::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<instance>::nodes;

Expand Down Expand Up @@ -303,22 +303,22 @@ void first_touch_distribution<instance>::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);
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions tests/lock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 iterations to run for some of the tests */
constexpr int iter = 10000;
Copy link
Copy Markdown
Member

@lundgren87 lundgren87 Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to understand what the difference between L33 and L35 is?
It seems to me that if one adopts the kMixedCase convention, should this not apply to both?

Copy link
Copy Markdown
Contributor Author

@kostis kostis Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a point there.

My guess is that the cpplint check(s) concern only variables that are used as array indices and are either in camel case (e.g. nThreads) or in snake case (e.g. page_info_size). The checks leave variables such as iter which are not used as array indices unchanged.


Expand Down Expand Up @@ -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<argo::locallock::mcs_lock>, 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;
Expand All @@ -214,23 +214,23 @@ 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];
argo::locallock::mcs_lock *global_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_counter2<argo::locallock::mcs_lock>,
&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;
Expand All @@ -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_<int>(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<argo::globallock::cohort_lock>, 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;
}
Expand Down
Loading