Skip to content
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.

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

mca/base: add a new MCA variable type for include lists #12826

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,7 @@ AC_CONFIG_FILES([
test/asm/Makefile
test/datatype/Makefile
test/class/Makefile
test/mca/Makefile
test/mpool/Makefile
test/support/Makefile
test/threads/Makefile
Expand Down
12 changes: 12 additions & 0 deletions ompi/mpi/tool/cvar_read.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* reserved.
* Copyright (c) 2021 Amazon.com, Inc. or its affiliates. All Rights
* reserved.
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
Expand Down Expand Up @@ -88,6 +89,17 @@ int MPI_T_cvar_read (MPI_T_cvar_handle handle, void *buf)
}

break;
case MCA_BASE_VAR_TYPE_SERIALIZABLE: {
char *tmp = value->serializable.serialize(&value->serializable);
if (strlen(tmp) == 0) {
((char *)buf)[0] = '\0';
} else {
strcpy ((char *) buf, tmp);
}
free (tmp);

break;
}
default:
rc = MPI_T_ERR_INVALID;
}
Expand Down
2 changes: 2 additions & 0 deletions ompi/mpi/tool/mpit_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Copyright (c) 2020 The University of Tennessee and The University
* of Tennessee Research Foundation. All rights
* reserved.
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
Expand Down Expand Up @@ -56,6 +57,7 @@ static MPI_Datatype mca_to_mpi_datatypes[MCA_BASE_VAR_TYPE_MAX] = {
[MCA_BASE_VAR_TYPE_UINT32_T] = MPI_UINT32_T,
[MCA_BASE_VAR_TYPE_INT64_T] = MPI_INT64_T,
[MCA_BASE_VAR_TYPE_UINT64_T] = MPI_UINT64_T,
[MCA_BASE_VAR_TYPE_SERIALIZABLE] = MPI_CHAR,
};

int ompit_var_type_to_datatype (mca_base_var_type_t type, MPI_Datatype *datatype)
Expand Down
9 changes: 7 additions & 2 deletions opal/class/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# reserved.
# Copyright (c) 2021 Nanook Consulting. All rights reserved.
# Copyright (c) 2022 Amazon.com, Inc. or its affiliates. All Rights reserved.
# Copyright (c) 2024 Google, LLC. All rights reserved.
# $COPYRIGHT$
#
# Additional copyrights may follow
Expand All @@ -41,7 +42,9 @@ headers += \
class/opal_value_array.h \
class/opal_ring_buffer.h \
class/opal_rb_tree.h \
class/opal_interval_tree.h
class/opal_interval_tree.h \
class/opal_include_list.h \
class/opal_serializable.h

libopen_pal_core_la_SOURCES += \
class/opal_bitmap.c \
Expand All @@ -58,4 +61,6 @@ libopen_pal_core_la_SOURCES += \
class/opal_value_array.c \
class/opal_ring_buffer.c \
class/opal_rb_tree.c \
class/opal_interval_tree.c
class/opal_interval_tree.c \
class/opal_include_list.c \
class/opal_serializable.c
146 changes: 146 additions & 0 deletions opal/class/opal_include_list.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
*
* $HEADER$
*/

#include <regex.h>

#include "opal_config.h"

#include "opal/class/opal_include_list.h"
#include "opal/class/opal_object.h"
#include "opal/include/opal/constants.h"
#include "opal/mca/base/base.h"
#include "opal/util/argv.h"
#include "opal/util/output.h"
#include "opal/util/printf.h"

static void include_list_destructor(opal_include_list_t *p);

static int opal_include_list_deserialize(opal_include_list_t *object, const char *value)
{
/* release any current value and set to defaults */
include_list_destructor(object);

if (NULL == value || 0 == strlen(value)) {
return OPAL_SUCCESS;
}

if ('^' == value[0]) {
object->is_exclude = true;
++value;
}

object->items = opal_argv_split(value, ',');
return OPAL_SUCCESS;
}

static char *opal_include_list_serialize(const opal_include_list_t *object)
{
if (NULL == object->items) {
return strdup("");
}

char *tmp = opal_argv_join(object->items, ',');
if (object->is_exclude) {
char *value_str = NULL;
(void) opal_asprintf(&value_str, "^%s", tmp);
free(tmp);
return value_str;
}

return tmp;
}

static bool opal_include_list_is_set(const opal_include_list_t *object)
{
return (object->items != NULL);
}

static void include_list_contructor(opal_include_list_t *p)
{
p->super.deserialize = (opal_serializable_deserialize_fn_t)opal_include_list_deserialize;
p->super.serialize = (opal_serializable_serialize_fn_t)opal_include_list_serialize;
p->super.is_set = (opal_serializable_is_set_fn_t)opal_include_list_is_set;
p->items = NULL;
p->is_exclude = false;
}

static void include_list_destructor(opal_include_list_t *p)
{
opal_argv_free(p->items);
include_list_contructor(p);
}

OBJ_CLASS_INSTANCE(opal_include_list_t, opal_object_t, include_list_contructor,
include_list_destructor);

static int include_list_match_regex(opal_include_list_t *include_list, const char *value,
Copy link
Member

Choose a reason for hiding this comment

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

This function is doing the opposite of one might expect by reading the function name: it is not finding an item matching a regular expression from a list, but instead it is finding if an item matches one of the regular expressions in the list. At some point these two outcomes can be considered similar but their complexity is drastically different.

Assuming there is a real need for such the capability to match regular expressions stored in a list, this function need to be documented. And maybe renamed include_regex_list_match?

Copy link
Member Author

@hjelmn hjelmn Feb 26, 2025

Choose a reason for hiding this comment

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

I have a use case for this. With btl/uct we agreed on an include list of the supported memory domains. Problem is the naming is not necessarily simple. Right now with have mlx5_0 but it will fail to match other HCAs in the system. Same is true for our hardware (somegooglenic[0-9]). I could just enumerate all the possibilities but having it be a regex makes the include list cleaner and easier to maintain (I can just put somegooglenic.* in the list).

bool case_sensitive)
{
int regex_flags = REG_EXTENDED | REG_NOSUB;
regex_t regex;

if (!case_sensitive) {
regex_flags |= REG_ICASE;
}

for (int i = 0 ; include_list->items[i] ; ++i) {
int rc = regcomp(&regex, include_list->items[i], regex_flags);
if (rc != 0) {
/* incorrectly formatted regular expression */
opal_output_verbose(MCA_BASE_VERBOSE_WARN, 0, "error compiling regular expression: %s, "
"ignoring", include_list->items[i]);
continue;
}

rc = regexec(&regex, value, /*nmatch=*/0, /*pmatch=*/NULL, /*eflags=*/0);
regfree(&regex);
if (0 == rc) {
return (include_list->is_exclude ? -1 : 1) * (i + 1);
}
}

return include_list->is_exclude ? 1 : -1;
}

static int include_list_match(opal_include_list_t *include_list, const char *value,
bool case_sensitive)
{
for (int i = 0 ; include_list->items[i] ; ++i) {
bool match = false;
if (case_sensitive) {
if (0 == strcmp(include_list->items[i], value)) {
match = true;
}
} else if (0 == strcasecmp(include_list->items[i], value)) {
match = true;
}

if (match) {
return (include_list->is_exclude ? -1 : 1) * (i + 1);
}
}

return include_list->is_exclude ? 1 : -1;
}

int opal_include_list_match(opal_include_list_t *include_list, const char *value,
bool regex_match, bool case_sensitive)
{
if (include_list == NULL || value == NULL || include_list->items == NULL) {
opal_output_verbose(MCA_BASE_VERBOSE_ERROR, 0, "error matching in include list");
return -1;
}

if (regex_match) {
return include_list_match_regex(include_list, value, case_sensitive);
}

return include_list_match(include_list, value, case_sensitive);
}
50 changes: 50 additions & 0 deletions opal/class/opal_include_list.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
*
* $HEADER$
*/

#if !defined(OPAL_INCLUDE_LIST_H)
#define OPAL_INCLUDE_LIST_H

#include "opal_config.h"
#include "opal/class/opal_object.h"
#include "opal/class/opal_serializable.h"

/**
* An include list. These are strings of the form:
* foo,bar,baz (include)
* ^foo,bar,baz (exclude)
*/
struct opal_include_list {
opal_serializable_t super;
Copy link
Member

Choose a reason for hiding this comment

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

I question the need for the intermediary opal_serializable_t class.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not strictly necessary but allows the building of additional variable types as needed. I can't think of anything off the top of my head that would benefit but figured it is a good to have.

Copy link
Member Author

@hjelmn hjelmn Feb 26, 2025

Choose a reason for hiding this comment

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

The general idea is that these variables are strings to the user (command line, MPI_T, env vars, etc) but may be something else inside Open MPI: argv list, struct, etc. The serializable takes the string and fills in the internal structure and vise versa. All of this is handled in mca/var so it works with MPI_T as well.

/** argv array of items */
char **items;
/** is this an exclude list */
bool is_exclude;
};
typedef struct opal_include_list opal_include_list_t;

OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_include_list_t);

/**
* Match a string against the include list and return the list rank.
*
* @param[in] include_list Include list
* @param[in] value Value to match
* @param[in] regex_match Treat the entries in the include list as regular expressions
* @param[in] case_sensitive Make matching case sensitive
*
* This method searches the include list for value. If an entry matches then the rank of the
* include list match is returned. A negative number is returned if the list is an exclude
* list.
*/
OPAL_DECLSPEC int opal_include_list_match(opal_include_list_t *include_list, const char *value,
bool regex_match, bool case_sensitive);


#endif /* OPAL_INCLUDE_LIST_H */
53 changes: 53 additions & 0 deletions opal/class/opal_serializable.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
*
* $HEADER$
*/

#include <regex.h>

#include "opal_config.h"

#include "opal/class/opal_include_list.h"
#include "opal/class/opal_object.h"
#include "opal/include/opal/constants.h"
#include "opal/mca/base/base.h"
#include "opal/util/argv.h"
#include "opal/util/output.h"

static int opal_serializable_deserialize(opal_serializable_t *object, const char *value)
{
(void)object;
(void)value;
return OPAL_ERR_NOT_IMPLEMENTED;
}

static char *opal_serializable_serialize(const opal_serializable_t *object)
{
(void)object;
return NULL;
}

static bool opal_serializable_is_set(const opal_serializable_t *object)
{
(void)object;
return false;
}

static void opal_serializable_constructor(opal_serializable_t *p)
{
p->deserialize = opal_serializable_deserialize;
p->serialize = opal_serializable_serialize;
p->is_set = opal_serializable_is_set;
}

static void opal_serializable_destructor(opal_serializable_t *p)
{
}

OBJ_CLASS_INSTANCE(opal_serializable_t, opal_object_t, opal_serializable_constructor,
opal_serializable_destructor);
36 changes: 36 additions & 0 deletions opal/class/opal_serializable.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2024 Google, LLC. All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
*
* $HEADER$
*/

#if !defined(OPAL_SERIALIZABLE_H)
#define OPAL_SERIALIZABLE_H

#include "opal_config.h"
#include "opal/class/opal_object.h"

struct opal_serializable;
typedef struct opal_serializable opal_serializable_t;

typedef int (*opal_serializable_deserialize_fn_t)(opal_serializable_t *object, const char *value);
typedef char *(*opal_serializable_serialize_fn_t)(const opal_serializable_t *object);
typedef bool (*opal_serializable_is_set_fn_t)(const opal_serializable_t *object);

struct opal_serializable {
opal_object_t super;
/** de-serialize the object from a string */
opal_serializable_deserialize_fn_t deserialize;
/** serialize the object into a string */
opal_serializable_serialize_fn_t serialize;
/** object has a value set */
opal_serializable_is_set_fn_t is_set;
};

OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_serializable_t);

#endif /* OPAL_SERIALIZABLE_H */
Loading
Loading