Skip to content

Commit

Permalink
Fix the info subscriber mechanism and hidden info keys
Browse files Browse the repository at this point in the history 
Signed-off-by: Joseph Schuchart <schuchart@icl.utk.edu>
  • Loading branch information
@devreal
devreal committed Apr 26, 2024
1 parent bebfc41 commit cdda311
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 125 deletions.
13 changes: 0 additions & 13 deletions ompi/communicator/comm.c
View file
Expand Up @@ -707,11 +707,6 @@ int ompi_comm_split_with_info( ompi_communicator_t* comm, int color, int key,
/* Activate the communicator and init coll-component */
rc = ompi_comm_activate (&newcomp, comm, NULL, NULL, NULL, false, mode);

/* MPI-4 §7.4.4 requires us to remove all unknown keys from the info object */
if (NULL != newcomp->super.s_info) {
opal_info_remove_unreferenced(newcomp->super.s_info);
}

exit:
free ( results );
free ( sorted );
Expand Down Expand Up @@ -1028,9 +1023,6 @@ static int ompi_comm_split_type_core(ompi_communicator_t *comm,
goto exit;
}

/* MPI-4 §7.4.4 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(newcomp->super.s_info);

/* TODO: there probably is better way to handle this case without throwing away the
* intermediate communicator. */
rc = ompi_comm_split (newcomp, local_split_type, key, newcomm, false);
Expand Down Expand Up @@ -1363,9 +1355,6 @@ int ompi_comm_dup_with_info ( ompi_communicator_t * comm, opal_info_t *info, omp
return rc;
}

/* MPI-4 §7.4.4 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(newcomp->super.s_info);

*newcomm = newcomp;
return MPI_SUCCESS;
}
Expand Down Expand Up @@ -1522,8 +1511,6 @@ static int ompi_comm_idup_with_info_finish (ompi_comm_request_t *request)
{
ompi_comm_idup_with_info_context_t *context =
(ompi_comm_idup_with_info_context_t *) request->context;
/* MPI-4 §7.4.4 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(context->newcomp->super.s_info);

/* done */
return MPI_SUCCESS;
Expand Down
3 changes: 0 additions & 3 deletions ompi/file/file.c
View file
Expand Up @@ -138,9 +138,6 @@ int ompi_file_open(struct ompi_communicator_t *comm, const char *filename,
return ret;
}

/* MPI-4 §14.2.8 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(file->super.s_info);

/* All done */

*fh = file;
Expand Down
2 changes: 1 addition & 1 deletion ompi/info/info.c
View file
Expand Up @@ -243,7 +243,7 @@ int ompi_mpiinfo_init_env(int argc, char *argv[], ompi_info_t *info)
// related calls:

int ompi_info_dup (ompi_info_t *info, ompi_info_t **newinfo) {
return opal_info_dup (&(info->super), (opal_info_t **)newinfo);
return opal_info_dup_public (&(info->super), (opal_info_t **)newinfo);
}
int ompi_info_set (ompi_info_t *info, const char *key, const char *value) {
return opal_info_set (&(info->super), key, value);
Expand Down
2 changes: 1 addition & 1 deletion ompi/mpi/c/comm_get_info.c
View file
Expand Up @@ -61,7 +61,7 @@ int MPI_Comm_get_info(MPI_Comm comm, MPI_Info *info_used)

opal_info_t *opal_info_used = &(*info_used)->super;

opal_info_dup(comm->super.s_info, &opal_info_used);
opal_info_dup_public(comm->super.s_info, &opal_info_used);

return MPI_SUCCESS;
}
2 changes: 1 addition & 1 deletion ompi/mpi/c/file_get_info.c
View file
Expand Up @@ -86,7 +86,7 @@ int MPI_File_get_info(MPI_File fh, MPI_Info *info_used)
}
opal_info_t *opal_info_used = &(*info_used)->super;

opal_info_dup(fh->super.s_info, &opal_info_used);
opal_info_dup_public(fh->super.s_info, &opal_info_used);

return OMPI_SUCCESS;
}
2 changes: 1 addition & 1 deletion ompi/mpi/c/win_get_info.c
View file
Expand Up @@ -60,7 +60,7 @@ int MPI_Win_get_info(MPI_Win win, MPI_Info *info_used)
}
opal_info_t *opal_info_used = &(*info_used)->super;

ret = opal_info_dup(win->super.s_info, &opal_info_used);
ret = opal_info_dup_public(win->super.s_info, &opal_info_used);

OMPI_ERRHANDLER_RETURN(ret, win, ret, FUNC_NAME);
}
12 changes: 0 additions & 12 deletions ompi/win/win.c
View file
Expand Up @@ -266,9 +266,6 @@ ompi_win_create(void *base, size_t size,
return ret;
}

/* MPI-4 §12.2.7 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(win->super.s_info);

*newwin = win;

return OMPI_SUCCESS;
Expand Down Expand Up @@ -300,9 +297,6 @@ ompi_win_allocate(size_t size, int disp_unit, opal_info_t *info,
return ret;
}

/* MPI-4 §12.2.7 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(win->super.s_info);

*((void**) baseptr) = base;
*newwin = win;

Expand Down Expand Up @@ -335,9 +329,6 @@ ompi_win_allocate_shared(size_t size, int disp_unit, opal_info_t *info,
return ret;
}

/* MPI-4 §12.2.7 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(win->super.s_info);

*((void**) baseptr) = base;
*newwin = win;

Expand Down Expand Up @@ -368,9 +359,6 @@ ompi_win_create_dynamic(opal_info_t *info, ompi_communicator_t *comm, ompi_win_t
return ret;
}

/* MPI-4 §12.2.7 requires us to remove all unknown keys from the info object */
opal_info_remove_unreferenced(win->super.s_info);

*newwin = win;

return OMPI_SUCCESS;
Expand Down
83 changes: 31 additions & 52 deletions opal/util/info.c
View file
Expand Up @@ -67,12 +67,14 @@ OBJ_CLASS_INSTANCE(opal_info_entry_t, opal_list_item_t, info_entry_constructor,
/*
* Duplicate an info
*/
int opal_info_dup(opal_info_t *info, opal_info_t **newinfo)
static int opal_info_dup_impl(opal_info_t *info, opal_info_t **newinfo, bool public_only)
{
opal_info_entry_t *iterator;

OPAL_THREAD_LOCK(info->i_lock);
OPAL_LIST_FOREACH (iterator, &info->super, opal_info_entry_t) {
/* skip keys that are internal if we didn't ask for them */
if (public_only && (iterator->ie_internal || iterator->ie_referenced == 0)) continue;
/* create a new info entry and retain the string objects */
opal_info_entry_t *newentry = OBJ_NEW(opal_info_entry_t);
newentry->ie_key = iterator->ie_key;
Expand All @@ -85,6 +87,16 @@ int opal_info_dup(opal_info_t *info, opal_info_t **newinfo)
return OPAL_SUCCESS;
}

int opal_info_dup_public(opal_info_t *info, opal_info_t **newinfo)
{
return opal_info_dup_impl(info, newinfo, true);
}

int opal_info_dup(opal_info_t *info, opal_info_t **newinfo)
{
return opal_info_dup_impl(info, newinfo, false);
}

static void opal_info_get_nolock(opal_info_t *info, const char *key, opal_cstring_t **value,
int *flag)
{
Expand Down Expand Up @@ -136,7 +148,7 @@ static int opal_info_set_cstring_nolock(opal_info_t *info, const char *key, opal
return OPAL_SUCCESS;
}

static int opal_info_set_nolock(opal_info_t *info, const char *key, const char *value)
static int opal_info_set_nolock(opal_info_t *info, const char *key, const char *value, bool internal)
{
opal_info_entry_t *old_info;

Expand All @@ -147,6 +159,7 @@ static int opal_info_set_nolock(opal_info_t *info, const char *key, const char *
*/
size_t value_len = strlen(value);
old_info->ie_referenced++;
old_info->ie_internal = internal;
if (old_info->ie_value->length == value_len
&& 0 == strcmp(old_info->ie_value->string, value)) {
return OPAL_SUCCESS;
Expand All @@ -171,6 +184,7 @@ static int opal_info_set_nolock(opal_info_t *info, const char *key, const char *
return OPAL_ERR_OUT_OF_RESOURCE;
}
new_info->ie_referenced++;
new_info->ie_internal = internal;
opal_list_append(&(info->super), (opal_list_item_t *) new_info);
}
return OPAL_SUCCESS;
Expand All @@ -184,7 +198,20 @@ int opal_info_set(opal_info_t *info, const char *key, const char *value)
int ret;

OPAL_THREAD_LOCK(info->i_lock);
ret = opal_info_set_nolock(info, key, value);
ret = opal_info_set_nolock(info, key, value, false);
OPAL_THREAD_UNLOCK(info->i_lock);
return ret;
}

/*
* Set a value on the info
*/
int opal_info_set_internal(opal_info_t *info, const char *key, const char *value)
{
int ret;

OPAL_THREAD_LOCK(info->i_lock);
ret = opal_info_set_nolock(info, key, value, true);
OPAL_THREAD_UNLOCK(info->i_lock);
return ret;
}
Expand Down Expand Up @@ -372,6 +399,7 @@ static void info_entry_constructor(opal_info_entry_t *entry)
entry->ie_key = NULL;
entry->ie_value = NULL;
entry->ie_referenced = 0;
entry->ie_internal = false;
}

static void info_entry_destructor(opal_info_entry_t *entry)
Expand Down Expand Up @@ -410,52 +438,3 @@ static opal_info_entry_t *info_find_key(opal_info_t *info, const char *key)
}
return NULL;
}

/**
* Mark the entry \c key as referenced.
*/
int opal_info_mark_referenced(opal_info_t *info, const char *key)
{
opal_info_entry_t *entry;

OPAL_THREAD_LOCK(info->i_lock);
entry = info_find_key(info, key);
entry->ie_referenced++;
OPAL_THREAD_UNLOCK(info->i_lock);

return OPAL_SUCCESS;
}

/**
* Remove a reference from the entry \c key.
*/
int opal_info_unmark_referenced(opal_info_t *info, const char *key)
{
opal_info_entry_t *entry;

OPAL_THREAD_LOCK(info->i_lock);
entry = info_find_key(info, key);
entry->ie_referenced--;
OPAL_THREAD_UNLOCK(info->i_lock);

return OPAL_SUCCESS;
}

/**
* Remove any entries that are not marked as referenced
*/
int opal_info_remove_unreferenced(opal_info_t *info)
{
opal_info_entry_t *iterator, *next;
/* iterate over all entries and remove the ones that are not referenced */
OPAL_THREAD_LOCK(info->i_lock);
OPAL_LIST_FOREACH_SAFE (iterator, next, &info->super, opal_info_entry_t) {
if (!iterator->ie_referenced) {
opal_list_remove_item(&info->super, &iterator->super);
}
}
OPAL_THREAD_UNLOCK(info->i_lock);


return OPAL_SUCCESS;
}
68 changes: 30 additions & 38 deletions opal/util/info.h
View file
Expand Up @@ -67,6 +67,7 @@ struct opal_info_entry_t {
opal_cstring_t *ie_key; /**< "key" part of the (key, value) pair */
uint32_t ie_referenced; /**< number of times this entry was internally
referenced */
bool ie_internal; /**< internal keys are not handed back to the user */
};

/**
Expand All @@ -90,7 +91,23 @@ OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_info_t);
OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_info_entry_t);

/**
* opal_info_dup - Duplicate an 'MPI_Info' object
* opal_info_dup - Duplicate public keys of an 'MPI_Info' object
*
* @param info source info object (handle)
* @param newinfo pointer to the new info object (handle)
*
* @retval OPAL_SUCCESS upon success
* @retval OPAL_ERR_OUT_OF_RESOURCE if out of memory
*
* Not only will the (key, value) pairs be duplicated, the order
* of keys will be the same in 'newinfo' as it is in 'info'. When
* an info object is no longer being used, it should be freed with
* \c opal_info_free.
*/
int opal_info_dup_public(opal_info_t *info, opal_info_t **newinfo);

/**
* opal_info_dup - Duplicate all entries of an 'MPI_Info' object
*
* @param info source info object (handle)
* @param newinfo pointer to the new info object (handle)
Expand All @@ -117,6 +134,18 @@ int opal_info_dup(opal_info_t *info, opal_info_t **newinfo);
*/
OPAL_DECLSPEC int opal_info_set(opal_info_t *info, const char *key, const char *value);

/**
* Set a new key,value pair on info and mark it as internal.
*
* @param info pointer to opal_info_t object
* @param key pointer to the new key object
* @param value pointer to the new value object
*
* @retval OPAL_SUCCESS upon success
* @retval OPAL_ERR_OUT_OF_RESOURCE if out of memory
*/
OPAL_DECLSPEC int opal_info_set_internal(opal_info_t *info, const char *key, const char *value);

/**
* Set a new key,value pair on info.
*
Expand Down Expand Up @@ -287,43 +316,6 @@ static inline int opal_info_get_nkeys(opal_info_t *info, int *nkeys)
return OPAL_SUCCESS;
}


/**
* Mark the entry \c key as referenced.
*
* This function is useful for lazily initialized components
* that do not read the key immediately but want to make sure
* the key is kept by the object owning the info key.
*
* @param info Pointer to opal_info_t object.
* @param key The key which to mark as referenced.
*
* @retval OPAL_SUCCESS
*/
int opal_info_mark_referenced(opal_info_t *info, const char *key);

/**
* Remove a reference from the entry \c key.
*
* This function should be used by components reading the key
* without wanting to retain it in the object owning the info.
*
* @param info Pointer to opal_info_t object.
* @param key The key which to unmark as referenced.
*
* @retval OPAL_SUCCESS
*/
int opal_info_unmark_referenced(opal_info_t *info, const char *key);

/**
* Remove any entries that are not marked as referenced
*
* @param info Pointer to opal_info_t object.
*
* @retval OPAL_SUCCESS
*/
int opal_info_remove_unreferenced(opal_info_t *info);

END_C_DECLS

#endif /* OPAL_INFO_H */
7 changes: 4 additions & 3 deletions opal/util/info_subscriber.c
View file
Expand Up @@ -269,9 +269,10 @@ int opal_infosubscribe_change_info(opal_infosubscriber_t *object, opal_info_t *n
updated_value = opal_infosubscribe_inform_subscribers(object, iterator->ie_key->string,
iterator->ie_value->string,
&found_callback);
if (NULL != updated_value
&& 0 != strncmp(updated_value, value_str->string, value_str->length)) {
err = opal_info_set(object->s_info, iterator->ie_key->string, updated_value);
if (NULL != updated_value) {
err = opal_info_set(object->s_info, key_str->string, updated_value);
} else {
err = opal_info_set_internal(object->s_info, key_str->string, value_str->string);
}
OBJ_RELEASE(value_str);
OBJ_RELEASE(key_str);
Expand Down

0 comments on commit cdda311

Please sign in to comment.