Cleanup some Pthread code. (#294)

Signed-off-by: Samuel K. Gutierrez <[email protected]>

Author: samuelkgutierrez

src/quo-vadis-pthread.cc

@@ -1,6 +1,6 @@
 /* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*qv_policy_t
- * Copyright (c) 2022-2024 Triad National Security, LLC
+ * Copyright (c) 2022-2025 Triad National Security, LLC
  *                         All rights reserved.
  *
  * Copyright (c) 2022-2024 Inria
@@ -24,7 +24,6 @@
 #include "qvi-scope.h"
 #include "qvi-utils.h"
 
-
 struct qvi_pthread_args_s {
     qv_scope_t *scope = nullptr;
     qvi_pthread_routine_fun_ptr_t th_routine = nullptr;
@@ -143,13 +142,12 @@ qv_pthread_colors_fill(
    qv_policy_t policy,
    int stride,
    int npieces
-){
+) {
     int rc = QV_SUCCESS;
 
-    if(stride < 1)
-        return QV_ERR_INVLD_ARG;
-    
-    switch(policy){
+    if (stride < 1) return QV_ERR_INVLD_ARG;
+
+    switch(policy) {
     case QV_POLICY_SPREAD:
         {
             break;
@@ -164,8 +162,6 @@ qv_pthread_colors_fill(
 
     case QV_POLICY_SCATTER:
         {
-            
-            
             break;
         }
 
@@ -186,7 +182,6 @@ qv_pthread_colors_fill(
             break;
         }
     }
-
     return rc;
 }
 

src/qvi-group-pthread.cc

@@ -1,6 +1,6 @@
 /* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2020-2024 Triad National Security, LLC
+ * Copyright (c) 2020-2025 Triad National Security, LLC
  *                         All rights reserved.
  *
  * This file is part of the quo-vadis project. See the LICENSE file at the

src/qvi-group-pthread.h

@@ -21,7 +21,7 @@
 
 struct qvi_group_pthread : public qvi_group {
     /** Underlying group instance. */
-    qvi_pthread_group_t *thgroup = nullptr;
+    qvi_pthread_group *thgroup = nullptr;
     /** Constructor. */
     qvi_group_pthread(void) = default;
     /** Constructor. */

src/qvi-pthread.cc

@@ -33,7 +33,7 @@ qvi_pthread_group::qvi_pthread_group(
     */
 
     m_data_g = new qvi_bbuff *[m_size]();
-    for(int i = 0 ; i < group_size ; i++){
+    for (int i = 0 ; i < group_size ; i++) {
         const int rc = qvi_bbuff_new(&m_data_g[i]);
         if (qvi_unlikely(rc != 0)) throw qvi_runtime_error();
     }
@@ -47,7 +47,7 @@ qvi_pthread_group::call_first_from_pthread_create(
 ) {
     const pid_t mytid = qvi_gettid();
     auto args = (qvi_pthread_group_pthread_create_args_s *)arg;
-    qvi_pthread_group_t *const group = args->group;
+    qvi_pthread_group *const group = args->group;
     const qvi_pthread_routine_fun_ptr_t thread_routine = args->throutine;
     void *const th_routine_argp = args->throutine_argp;
     // Let the threads add their TIDs to the vector.
@@ -204,9 +204,9 @@ int
 qvi_pthread_group::split(
     int color,
     int key,
-    qvi_pthread_group_t **child
+    qvi_pthread_group **child
 ) {
-    qvi_pthread_group_t *ichild = nullptr;
+    qvi_pthread_group *ichild = nullptr;
 
     qvi_subgroup_info sginfo;
     int rc = m_subgroup_info(color, key, &sginfo);

src/qvi-pthread.h

@@ -19,22 +19,21 @@
 #include "qvi-bbuff.h"
 
 typedef void *(*qvi_pthread_routine_fun_ptr_t)(void *);
-
+// Foward declaration.
 struct qvi_pthread_group;
-typedef struct qvi_pthread_group qvi_pthread_group_t;
 
 struct qvi_pthread_group_pthread_create_args_s {
     /** Thread group. */
-    qvi_pthread_group_t *group = nullptr;
+    qvi_pthread_group *group = nullptr;
     /** The routine to call after group construction. */
     qvi_pthread_routine_fun_ptr_t throutine = nullptr;
     /** Thread routine arguments. */
     void *throutine_argp = nullptr;
-    /** Constructor. */
+    /** Default constructor. */
     qvi_pthread_group_pthread_create_args_s(void) = delete;
     /** Constructor. */
     qvi_pthread_group_pthread_create_args_s(
-        qvi_pthread_group_t *group_a,
+        qvi_pthread_group *group_a,
         qvi_pthread_routine_fun_ptr_t throutine_a,
         void *throutine_argp_a
     ) : group(group_a)
@@ -73,7 +72,7 @@ struct qvi_pthread_group {
         qvi_subgroup_info *sginfo
     );
 public:
-    /** Constructor. */
+    /** Default constructor. */
     qvi_pthread_group(void) = delete;
     /**
      * Constructor. This is called by the parent process to construct the
@@ -113,7 +112,7 @@ struct qvi_pthread_group {
     split(
         int color,
         int key,
-        qvi_pthread_group_t **child
+        qvi_pthread_group **child
     );
 
     int

tests/common-test-utils.h

@@ -90,8 +90,8 @@ ctu_scope_report(
     }
 
     printf(
-        "[%d] %s sgrank is %d\n"
-        "[%d] %s sgsize is %d\n",
+        "[%d] %s scope group rank is %d\n"
+        "[%d] %s scope group size is %d\n",
         pid, scope_name, sgrank,
         pid, scope_name, sgsize
     );

tests/test-pthread-split.c

@@ -29,11 +29,11 @@ thread_work(
         ers = "qv_scope_group_rank failed";
         ctu_panic("%s (rc=%s)", ers, qv_strerr(rc));
     }
-    
+
     ctu_scope_report(scope, "thread_scope_in_thread_routine");
     ctu_emit_task_bind(scope);
-    
-    fprintf(stdout,"[%d] ============ Thread %d splitting in two pieces\n", tid, rank);
+
+    printf("[%d] ============ Thread %d splitting in two pieces\n", tid, rank);
     qv_scope_t *pthread_subscope = NULL;
     rc = qv_scope_split(scope, 2, rank, &pthread_subscope);
     if (rc != QV_SUCCESS) {
@@ -44,7 +44,7 @@ thread_work(
     ctu_scope_report(pthread_subscope, "thread_subscope");
     ctu_emit_task_bind(pthread_subscope);
 
-    
+
     rc = qv_scope_free(pthread_subscope);
     if (rc != QV_SUCCESS) {
         ers = "qv_scope_free failed";
@@ -73,6 +73,11 @@ main(void)
         ers = "MPI_Comm_size() failed";
         ctu_panic("%s (rc=%d)", ers, rc);
     }
+    // TODO: As Edgar pointed out, this will work only in the single process
+    // case. The mpi_scope needs to be split to get a new single MPI scope.
+    if (wsize != 1) {
+        ctu_panic("!!! This test works only with one MPI process!");
+    }
 
     rc = MPI_Comm_rank(comm, &wrank);
     if (rc != MPI_SUCCESS) {
@@ -100,46 +105,41 @@ main(void)
 
     ctu_scope_report(mpi_scope, "mpi_scope");
     ctu_emit_task_bind(mpi_scope);
-
-    //As Edgar pointed out, this will work only in the
-    //single process case.
-    //The mpi_scope need to be plsit in order to get
-    //a new mpi_single_process_scope 
-    
     //
     // Test qv_pthread_scope_split
     //
     int npieces = 2;
     int nthreads = ncores;
     int stride = 1;
     int colors[nthreads];
-    
-    printf("[%d] Testing thread_scope_split (nthreads=%i, npieces=%i)\n", tid, nthreads, npieces);
-    
+
+    printf(
+        "[%d] Testing thread_scope_split (nthreads=%d, npieces=%d)\n",
+        tid, nthreads, npieces
+    );
+
     for (int i = 0 ; i < nthreads ; i++) {
         colors[i] = i % npieces;
     }
 
-    fprintf(stdout,"Array values :");
+    printf("Manual values: ");
     for (int i = 0 ; i < nthreads ; i++) {
-        fprintf(stdout,"val[%i]: %i |",i,colors[i]);
+        printf("val[%i]:%i | ",i,colors[i]);
     }
-    fprintf(stdout,"\n");
-    
+    printf("\n");
 
     rc = qv_pthread_colors_fill(colors, nthreads, QV_POLICY_PACKED, stride, npieces);
     if (rc != QV_SUCCESS) {
         ers = "qv_pthread_colors_fill() failed";
         ctu_panic("%s (rc=%s)", ers, qv_strerr(rc));
     }
 
-    fprintf(stdout,"Array values :");
+    fprintf(stdout,"Filled values: ");
     for (int i = 0 ; i < nthreads ; i++) {
-        fprintf(stdout,"val[%i]: %i |",i,colors[i]);
+        fprintf(stdout,"val[%i]:%i | ",i,colors[i]);
     }
     fprintf(stdout,"\n");
-    
-    
+
     qv_scope_t **th_scopes = NULL;
     rc = qv_pthread_scope_split(
         mpi_scope, npieces, colors, nthreads, &th_scopes
@@ -189,7 +189,7 @@ main(void)
 
     printf("[%d] Testing thread_scope_split_at (nthreads=%i)\n", tid, nthreads);
 
-    int colors2[nthreads];    
+    int colors2[nthreads];
     for (int i = 0 ; i < nthreads ; i++) {
         colors2[i] = i % ncores;
     }
@@ -200,7 +200,7 @@ main(void)
     }
     fprintf(stdout,"\n");
 
-    
+
     rc = qv_pthread_colors_fill(colors2, nthreads, QV_POLICY_PACKED, stride, ncores);
     if (rc != QV_SUCCESS) {
         ers = "qv_pthread_colors_fill() failed";