Skip to content

Commit 568cd7c

Browse files
author
Raghuveer Devulapalli
committed
Minor comments based on review
(1) Add meson flag to build with std threads to ASAN tests (2) Remove unnecessary comments
1 parent 6c46d71 commit 568cd7c

File tree

3 files changed

+8
-13
lines changed

3 files changed

+8
-13
lines changed

.github/workflows/c-cpp.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ jobs:
165165
CXX: clang++-18
166166
run: |
167167
make clean
168-
meson setup -Dbuild_tests=true -Duse_openmp=true -Db_sanitize=address,undefined -Dfatal_sanitizers=true -Dasan_ci_dont_validate=true -Db_lundef=false --warnlevel 0 --buildtype release builddir
168+
meson setup -Dbuild_tests=true -Duse_openmp=true -Duse_stdthreads=true -Db_sanitize=address,undefined -Dfatal_sanitizers=true -Dasan_ci_dont_validate=true -Db_lundef=false --warnlevel 0 --buildtype release builddir
169169
cd builddir
170170
ninja
171171
@@ -202,7 +202,7 @@ jobs:
202202
CXX: clang++-18
203203
run: |
204204
make clean
205-
meson setup -Dbuild_tests=true -Duse_openmp=true -Db_sanitize=address,undefined -Dfatal_sanitizers=true -Dasan_ci_dont_validate=true -Db_lundef=false --warnlevel 0 --buildtype release builddir
205+
meson setup -Dbuild_tests=true -Duse_openmp=true -Duse_stdthreads=true -Db_sanitize=address,undefined -Dfatal_sanitizers=true -Dasan_ci_dont_validate=true -Db_lundef=false --warnlevel 0 --buildtype release builddir
206206
cd builddir
207207
ninja
208208

src/xss-common-qsort.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -752,12 +752,7 @@ X86_SIMD_SORT_INLINE void xss_qsort(T *arr, arrsize_t arrsize, bool hasnan)
752752

753753
#ifdef XSS_BUILD_WITH_STD_THREADS
754754
bool use_parallel = arrsize > 100000;
755-
#else
756-
bool use_parallel = false;
757-
#endif
758-
759755
if (use_parallel) {
760-
#ifdef XSS_BUILD_WITH_STD_THREADS
761756
// This thread limit was determined experimentally
762757
constexpr int thread_limit = 8;
763758
int thread_count = std::min(
@@ -777,13 +772,16 @@ X86_SIMD_SORT_INLINE void xss_qsort(T *arr, arrsize_t arrsize, bool hasnan)
777772
pool);
778773
// Wait for all tasks to complete
779774
pool.wait_all();
780-
#endif
781775
}
782776
else {
783777
// For small arrays, just use the sequential version
784778
qsort_<vtype, comparator, T>(
785779
arr, 0, arrsize - 1, 2 * (arrsize_t)log2(arrsize), 0);
786780
}
781+
#else
782+
qsort_<vtype, comparator, T>(
783+
arr, 0, arrsize - 1, 2 * (arrsize_t)log2(arrsize), 0);
784+
#endif // XSS_BUILD_WITH_STD_THREADS
787785

788786
replace_inf_with_nan(arr, arrsize, nan_count, descending);
789787
}

src/xss-thread-pool.hpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ namespace tp {
3737
std::condition_variable
3838
done_condition; // Condition variable for waiting
3939
int active_tasks {0};
40-
bool stop;
40+
bool stop {false};
4141

4242
public:
43-
ThreadPool(size_t num_threads) : stop(false)
43+
ThreadPool(size_t num_threads)
4444
{
4545
for (size_t i = 0; i < num_threads; ++i) {
4646
// Create a worker thread and add it to the pool
@@ -97,15 +97,13 @@ namespace tp {
9797
done_condition.wait(lock, [this] {
9898
return tasks.empty() && (active_tasks == 0);
9999
});
100-
// lock is automatically released here
101100
}
102101

103102
// Track the number of active tasks
104103
void task_start()
105104
{
106105
std::unique_lock<std::mutex> lock(queue_mutex);
107106
active_tasks++;
108-
// lock is automatically released here
109107
}
110108

111109
// Decrement the active task count and notify if all tasks are done
@@ -116,7 +114,6 @@ namespace tp {
116114
if (tasks.empty() && active_tasks == 0) {
117115
done_condition.notify_all();
118116
}
119-
// lock is automatically released here
120117
}
121118
};
122119

0 commit comments

Comments
 (0)