Merge pull request open-mpi#12498 from devreal/fix-info-subscribe

Fix the info subscriber mechanism and hidden info keys

Author: bosilca

ompi/communicator/comm.c

@@ -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 );
@@ -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);
@@ -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;
 }
@@ -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;

ompi/file/file.c

@@ -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;

ompi/info/info.c

@@ -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);

ompi/mpi/c/comm_get_info.c

@@ -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;
 }

ompi/mpi/c/file_get_info.c

@@ -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;
 }

ompi/mpi/c/win_get_info.c

@@ -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);
 }

ompi/win/win.c

@@ -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;
@@ -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;
 
@@ -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;
 
@@ -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;

opal/util/info.c

@@ -65,14 +65,18 @@ OBJ_CLASS_INSTANCE(opal_info_entry_t, opal_list_item_t, info_entry_constructor,
                    info_entry_destructor);
 
 /*
- * Duplicate an info
+ * Duplicate an info into newinfo. If public_info is true we only duplicate
+ * key-value pairs that are not internal and that had been referenced,
+ * either through opal_info_get or opal_info_set.
  */
-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;
@@ -85,6 +89,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)
 {
@@ -136,7 +150,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;
 
@@ -147,6 +161,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;
@@ -171,6 +186,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;
@@ -184,7 +200,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;
 }
@@ -372,6 +401,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)
@@ -410,52 +440,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;
-}

opal/util/info.h

@@ -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 */
 };
 
 /**
@@ -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)
@@ -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.
  *
@@ -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 */

opal/util/info_subscriber.c

@@ -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);