From 90ccfa42de25c383ad30281b14806eb63cf5cc0c Mon Sep 17 00:00:00 2001 From: David Boehme Date: Mon, 22 Sep 2025 11:27:44 -0700 Subject: [PATCH 1/5] Encapsulate the global static variables --- src/adiak.c | 54 +++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/adiak.c b/src/adiak.c index 62c0424..12d7641 100644 --- a/src/adiak.c +++ b/src/adiak.c @@ -48,12 +48,7 @@ typedef struct { record_list_t *shared_record_list; } adiak_t; -static adiak_t *adiak_config; -static volatile adiak_tool_t **tool_list; - -static adiak_tool_t *local_tool_list; -adiak_t adiak_public = { ADIAK_VERSION, ADIAK_VERSION, 0, 1, &local_tool_list, 0, NULL }; - +adiak_t adiak_public = { ADIAK_VERSION, ADIAK_VERSION, 0, 1, NULL, 0, NULL }; static int measure_adiak_walltime; static int measure_adiak_systime; @@ -79,7 +74,7 @@ static adiak_datatype_t base_catstring_ref = { adiak_catstring, adiak_categorica static adiak_datatype_t base_jsonstring_ref = { adiak_jsonstring, adiak_categorical, 0, 0, NULL, 1, 0 }; static adiak_datatype_t base_path_ref = { adiak_path, adiak_categorical, 0, 0, NULL, 1, 0 }; -static void adiak_common_init(); +static adiak_t* adiak_get_config(); static void adiak_register(int adiak_version, int category, adiak_nameval_cb_t nv, adiak_nameval_info_cb_t nvi, @@ -129,12 +124,11 @@ int adiak_raw_namevalue(const char *name, int category, const char *subcategory, record_list_t* rec = record_nameval(name, category, subcategory, value, type); info_ptr = rec->info; } - if (!tool_list) - return 0; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); + adiak_tool_t** tool_list = adiak_config->tool_list; - for (tool = (adiak_tool_t *) *tool_list; tool != NULL; tool = tool->next) { + for (tool = *tool_list; tool != NULL; tool = tool->next) { if (!tool->report_on_all_ranks && !adiak_config->reportable_rank) continue; if (tool->category != adiak_category_all && tool->category != category) @@ -277,7 +271,7 @@ void adiak_register_cb_with_info(int adiak_version, int category, void adiak_list_namevals(int adiak_version, int category, adiak_nameval_cb_t nv, void *opaque_val) { record_list_t *i; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { if (category != adiak_category_all && i->category != category) continue; @@ -289,7 +283,7 @@ void adiak_list_namevals(int adiak_version, int category, adiak_nameval_cb_t nv, void adiak_list_namevals_with_info(int adiak_version, int category, adiak_nameval_info_cb_t nv, void *opaque_val) { record_list_t *i; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { if (category != adiak_category_all && i->category != category) continue; @@ -301,7 +295,7 @@ void adiak_list_namevals_with_info(int adiak_version, int category, adiak_nameva int adiak_get_nameval(const char *name, adiak_datatype_t **t, adiak_value_t **value, int *cat, const char **subcat) { record_list_t *i; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { if (strcmp(i->name, name) == 0) { if (t) @@ -321,7 +315,7 @@ int adiak_get_nameval(const char *name, adiak_datatype_t **t, adiak_value_t **va int adiak_get_nameval_with_info(const char *name, adiak_datatype_t **t, adiak_value_t **value, adiak_record_info_t **info) { record_list_t *i; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { if (strcmp(i->name, name) == 0) { if (t) @@ -428,19 +422,23 @@ int adiak_get_subval(adiak_datatype_t* t, adiak_value_t* val, int elem, adiak_da return -1; } -static void adiak_common_init() +static adiak_t* adiak_get_config() { - static int initialized = 0; - if (initialized) - return; - initialized = 1; + static adiak_t* adiak_config = NULL; + static adiak_tool_t* local_tool_list = NULL; + + if (adiak_config) + return adiak_config; adiak_config = (adiak_t *) adksys_get_public_adiak_symbol(); if (!adiak_config) adiak_config = &adiak_public; if (ADIAK_VERSION < adiak_config->minimum_version) adiak_config->minimum_version = ADIAK_VERSION; - tool_list = (volatile adiak_tool_t **) adiak_config->tool_list; + if (!adiak_config->tool_list) + adiak_config->tool_list = &local_tool_list; + + return adiak_config; } void adiak_init(void *mpi_communicator_p) @@ -450,7 +448,7 @@ void adiak_init(void *mpi_communicator_p) return; initialized = 1; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); #if (USE_MPI) if (mpi_communicator_p && adksys_mpi_initialized()) { @@ -489,7 +487,9 @@ static void adiak_register(int adiak_version, int category, int report_on_all_ranks, void *opaque_val) { adiak_tool_t *newtool; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); + adiak_tool_t** tool_list = adiak_config->tool_list; + newtool = (adiak_tool_t *) malloc(sizeof(adiak_tool_t)); memset(newtool, 0, sizeof(*newtool)); newtool->version = adiak_version; @@ -498,11 +498,13 @@ static void adiak_register(int adiak_version, int category, newtool->name_val_cb = nv; newtool->nameval_info_cb = nvi; newtool->category = category; - newtool->next = (adiak_tool_t *) *tool_list; + newtool->next = *tool_list; newtool->prev = NULL; + if (*tool_list) (*tool_list)->prev = newtool; *tool_list = newtool; + if (report_on_all_ranks && !adiak_config->report_on_all_ranks) adiak_config->report_on_all_ranks = 1; } @@ -989,7 +991,7 @@ static record_list_t* record_nameval(const char *name, int category, const char addrecord->name = (const char *) strdup(name); - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); addrecord->list_next = adiak_config->shared_record_list; adiak_config->shared_record_list = addrecord; @@ -1013,7 +1015,7 @@ int adiak_clean() record_list_t *i, *next; int result; - adiak_common_init(); + adiak_t* adiak_config = adiak_get_config(); val.v_int = 0; result = adiak_raw_namevalue("clean", adiak_control, NULL, &val, &base_int); From 69ec34e32614b1bd8f7a5a80e7054ca0a74c34cc Mon Sep 17 00:00:00 2001 From: David Boehme Date: Mon, 22 Sep 2025 17:26:15 -0700 Subject: [PATCH 2/5] Store hash in global object --- src/adiak.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/adiak.c b/src/adiak.c index 12d7641..a8df8f0 100644 --- a/src/adiak.c +++ b/src/adiak.c @@ -14,6 +14,8 @@ #include "adiak_tool.h" #include "adksys.h" +#define RECORD_HASH_SIZE 128 + typedef struct adiak_tool_t { //Below fields are present in v1 int version; @@ -46,9 +48,17 @@ typedef struct { adiak_tool_t **tool_list; int use_mpi; record_list_t *shared_record_list; + record_list_t *record_hash[RECORD_HASH_SIZE]; } adiak_t; -adiak_t adiak_public = { ADIAK_VERSION, ADIAK_VERSION, 0, 1, NULL, 0, NULL }; +#define ADIAK_T_VERSION 1 + +adiak_t adiak_public = { ADIAK_T_VERSION, ADIAK_T_VERSION, 0, 1, NULL, 0, NULL, { NULL } }; + +/* With ADIAK_T_VERSION 1 or higher the hash list is stored in the adiak_t struct. + We fall back to a local hash list if a lower version is found as adiak_public. + */ +static record_list_t* local_record_hash[RECORD_HASH_SIZE]; static int measure_adiak_walltime; static int measure_adiak_systime; @@ -99,9 +109,6 @@ static int measure_walltime(); static int measure_systime(); static int measure_cputime(); -#define RECORD_HASH_SIZE 128 -static record_list_t *record_hash[RECORD_HASH_SIZE]; - #define MAX_PATH_LEN 4096 adiak_datatype_t *adiak_new_datatype(const char *typestr, ...) @@ -433,8 +440,8 @@ static adiak_t* adiak_get_config() adiak_config = (adiak_t *) adksys_get_public_adiak_symbol(); if (!adiak_config) adiak_config = &adiak_public; - if (ADIAK_VERSION < adiak_config->minimum_version) - adiak_config->minimum_version = ADIAK_VERSION; + if (ADIAK_T_VERSION < adiak_config->minimum_version) + adiak_config->minimum_version = ADIAK_T_VERSION; if (!adiak_config->tool_list) adiak_config->tool_list = &local_tool_list; @@ -953,6 +960,11 @@ static record_list_t* record_nameval(const char *name, int category, const char unsigned long hashval; int newrecord = 0; + adiak_t* adiak_config = adiak_get_config(); + record_list_t** record_hash = local_record_hash; + if (adiak_config->minimum_version >= 1) + record_hash = adiak_config->record_hash; + hashval = strhash(name) % RECORD_HASH_SIZE; for (i = record_hash[hashval]; i != NULL; i = i->hash_next) { if (strcmp(i->name, name) == 0) { @@ -991,8 +1003,6 @@ static record_list_t* record_nameval(const char *name, int category, const char addrecord->name = (const char *) strdup(name); - adiak_t* adiak_config = adiak_get_config(); - addrecord->list_next = adiak_config->shared_record_list; adiak_config->shared_record_list = addrecord; @@ -1029,8 +1039,15 @@ int adiak_clean() next = i->list_next; free(i); } - memset(record_hash, 0, sizeof(record_hash)); + + record_list_t** record_hash = local_record_hash; + if (adiak_config->minimum_version >= 1) + record_hash = adiak_config->record_hash; + + memset(record_hash, 0, sizeof(local_record_hash)); + adiak_config->shared_record_list = NULL; + return result; } From 80b2c1d5679f5cd97089ff7017b997d46a94364c Mon Sep 17 00:00:00 2001 From: David Boehme Date: Tue, 23 Sep 2025 09:11:17 -0700 Subject: [PATCH 3/5] Fix printing of containers in testlib --- tests/testlib.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/testlib.c b/tests/testlib.c index d09c03c..678dea7 100644 --- a/tests/testlib.c +++ b/tests/testlib.c @@ -98,12 +98,13 @@ static void print_value(adiak_value_t *val, adiak_datatype_t *t) } case adiak_set: { printf("["); - for (int i = 0; i < adiak_num_subvals(t); i++) { + int num_elements = adiak_num_subvals(t); + for (int i = 0; i < num_elements; i++) { adiak_value_t subval; adiak_datatype_t* subtype; adiak_get_subval(t, val, i, &subtype, &subval); print_value(&subval, subtype); - if (i+1 != t->num_elements) + if (i+1 != num_elements) printf(", "); } printf("]"); @@ -111,12 +112,13 @@ static void print_value(adiak_value_t *val, adiak_datatype_t *t) } case adiak_list: { printf("{"); - for (int i = 0; i < adiak_num_subvals(t); i++) { + int num_elements = adiak_num_subvals(t); + for (int i = 0; i < num_elements; i++) { adiak_value_t subval; adiak_datatype_t* subtype; adiak_get_subval(t, val, i, &subtype, &subval); print_value(&subval, subtype); - if (i+1 != t->num_elements) + if (i+1 != num_elements) printf(", "); } printf("}"); @@ -124,12 +126,13 @@ static void print_value(adiak_value_t *val, adiak_datatype_t *t) } case adiak_tuple: { printf("("); - for (int i = 0; i < adiak_num_subvals(t); i++) { + int num_elements = adiak_num_subvals(t); + for (int i = 0; i < num_elements; i++) { adiak_value_t subval; adiak_datatype_t* subtype; adiak_get_subval(t, val, i, &subtype, &subval); print_value(&subval, subtype); - if (i+1 != t->num_elements) + if (i+1 != num_elements) printf(", "); } printf(")"); From 6bd6046364459247472501ca49234a21b6867b49 Mon Sep 17 00:00:00 2001 From: David Boehme Date: Tue, 23 Sep 2025 09:35:56 -0700 Subject: [PATCH 4/5] Use hash to find records by name --- src/adiak.c | 75 ++++++++++++++++++++-------------- tests/test_application-api.cpp | 2 + 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/adiak.c b/src/adiak.c index a8df8f0..f64bf3d 100644 --- a/src/adiak.c +++ b/src/adiak.c @@ -109,6 +109,9 @@ static int measure_walltime(); static int measure_systime(); static int measure_cputime(); +static unsigned long strhash(const char*); +static record_list_t* find_record_by_name(const char* str); + #define MAX_PATH_LEN 4096 adiak_datatype_t *adiak_new_datatype(const char *typestr, ...) @@ -301,38 +304,32 @@ void adiak_list_namevals_with_info(int adiak_version, int category, adiak_nameva int adiak_get_nameval(const char *name, adiak_datatype_t **t, adiak_value_t **value, int *cat, const char **subcat) { - record_list_t *i; - adiak_t* adiak_config = adiak_get_config(); - for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { - if (strcmp(i->name, name) == 0) { - if (t) - *t = i->dtype; - if (value) - *value = i->value; - if (cat) - *cat = i->category; - if (subcat) - *subcat = i->subcategory; - return 0; - } + record_list_t *i = find_record_by_name(name); + if (i != NULL) { + if (t) + *t = i->dtype; + if (value) + *value = i->value; + if (cat) + *cat = i->category; + if (subcat) + *subcat = i->subcategory; + return 0; } return -1; } int adiak_get_nameval_with_info(const char *name, adiak_datatype_t **t, adiak_value_t **value, adiak_record_info_t **info) { - record_list_t *i; - adiak_t* adiak_config = adiak_get_config(); - for (i = adiak_config->shared_record_list; i != NULL; i = i->list_next) { - if (strcmp(i->name, name) == 0) { - if (t) - *t = i->dtype; - if (value) - *value = i->value; - if (info) - *info = i->info; - return 0; - } + record_list_t* i = find_record_by_name(name); + if (i != NULL) { + if (t) + *t = i->dtype; + if (value) + *value = i->value; + if (info) + *info = i->info; + return 0; } return -1; } @@ -953,17 +950,35 @@ static unsigned long strhash(const char *str) { return hash; } +static record_list_t** get_record_hash_list(adiak_t* adiak_config) +{ + return adiak_config->minimum_version >= 1 ? adiak_config->record_hash : local_record_hash; +} + +static record_list_t* find_record_by_name(const char* name) +{ + record_list_t* rec = NULL; + adiak_t* adiak_config = adiak_get_config(); + record_list_t** record_hash = get_record_hash_list(adiak_config); + + unsigned long hashval = strhash(name) % RECORD_HASH_SIZE; + for (rec = record_hash[hashval]; rec != NULL; rec = rec->hash_next) { + if (strcmp(rec->name, name) == 0) + break; + } + + return rec; +} + static record_list_t* record_nameval(const char *name, int category, const char *subcategory, adiak_value_t *value, adiak_datatype_t *dtype) { record_list_t *addrecord = NULL, *i; - unsigned long hashval; int newrecord = 0; + unsigned long hashval = 0; adiak_t* adiak_config = adiak_get_config(); - record_list_t** record_hash = local_record_hash; - if (adiak_config->minimum_version >= 1) - record_hash = adiak_config->record_hash; + record_list_t** record_hash = get_record_hash_list(adiak_config); hashval = strhash(name) % RECORD_HASH_SIZE; for (i = record_hash[hashval]; i != NULL; i = i->hash_next) { diff --git a/tests/test_application-api.cpp b/tests/test_application-api.cpp index 561570d..f5022c3 100644 --- a/tests/test_application-api.cpp +++ b/tests/test_application-api.cpp @@ -217,6 +217,8 @@ TEST(AdiakApplicationAPI, C_BasicTypes) EXPECT_EQ(val->v_int, 42); EXPECT_EQ(cat, test_cat); EXPECT_STREQ(subcat, "subcat:c"); + + EXPECT_EQ(adiak_get_nameval_with_info("blagarbl", &dtype, &val, &info), -1); } TEST(AdiakApplicationAPI, C_CompoundTypes) From cb729c022ebbf274bec40e562e8cb810a5d61a5a Mon Sep 17 00:00:00 2001 From: David Boehme Date: Tue, 23 Sep 2025 11:37:41 -0700 Subject: [PATCH 5/5] Try fixing outdated CMake minimum version in github workflow --- .github/workflows/cmake.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index ff83e34..f6ff544 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -56,7 +56,7 @@ jobs: # Note the current convention is to use the -S and -B options here to specify source # and build directories, but this is only available with CMake 3.13 and higher. # The CMake binaries on the Github Actions machines are (as of this writing) 3.12 - run: cmake $GITHUB_WORKSPACE -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_C_COMPILER=${{matrix.config.cc}} -DCMAKE_CXX_COMPILER=${{matrix.config.cxx}} + run: cmake $GITHUB_WORKSPACE -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DCMAKE_C_COMPILER=${{matrix.config.cc}} -DCMAKE_CXX_COMPILER=${{matrix.config.cxx}} - name: Build working-directory: ${{github.workspace}}/build