Rework mpi module initialization (LLNL#281)

* Rework mpi submodule initialization

* Remove obsolete init hook list

* Fix mpi log prefix setup

include/caliper/Caliper.h

@@ -357,9 +357,6 @@ class Caliper : public CaliperMetadataAccessInterface
     /// \brief Add a list of available caliper services.
     static void    add_services(const CaliperService*);
 
-    /// \brief Add a function that is called during %Caliper initialization.
-    static void    add_init_hook(void(*fn)());
-
     friend struct GlobalData;
 };
 

include/caliper/common/Log.h

@@ -29,6 +29,7 @@ class Log
 
     static void init();
     static void fini();
+    static bool is_initialized();
 
     inline std::ostream& stream() {
         if (verbosity() < m_level)

src/caliper/CMakeLists.txt

@@ -26,14 +26,17 @@ if (${BUILD_SHARED_LIBS})
 endif()
 
 list(APPEND CALIPER_SERIAL_OBJS
-  caliper/machine_serial.cpp)
+  caliper/machine_serial.cpp
+  caliper/setup_serial.cpp)
 
 if (CALIPER_HAVE_MPI)
   list(APPEND CALIPER_ALL_OBJS
-    caliper/machine_mpi.cpp)
+    caliper/machine_mpi.cpp
+    caliper/setup_mpi.cpp)
 else()
   list(APPEND CALIPER_ALL_OBJS
-    caliper/machine_serial.cpp)
+    caliper/machine_serial.cpp
+    caliper/setup_serial.cpp)
 endif()
 
 set(CALIPER_SERIAL_OBJS ${CALIPER_SERIAL_OBJS} PARENT_SCOPE)

src/caliper/Caliper.cpp

@@ -45,6 +45,8 @@ namespace cali
 extern void init_attribute_classes(Caliper* c);
 extern void init_api_attributes(Caliper* c);
 
+extern void init_submodules();
+
 extern void config_sanity_check(RuntimeConfig);
 
 }
@@ -367,33 +369,6 @@ struct Caliper::GlobalData
 
     static const ConfigSet::Entry      s_configdata[];
 
-    struct InitHookList {
-        void          (*hook)();
-        InitHookList* next;
-    };
-
-    static InitHookList*               s_init_hooks;
-
-    // --- static functions
-
-    static void add_init_hook(void (*hook)()) {
-        InitHookList* elem = new InitHookList { hook, s_init_hooks };
-        s_init_hooks = elem;
-    }
-
-    static void run_init_hooks() {
-        InitHookList* ptr = s_init_hooks;
-
-        while (ptr) {
-            (*(ptr->hook))();
-            InitHookList* tmp = ptr->next;
-            delete ptr;
-            ptr = tmp;
-        }
-
-        s_init_hooks = nullptr;
-    }
-
     // --- data
 
     bool                               automerge;
@@ -490,12 +465,10 @@ struct Caliper::GlobalData
     }
 
     void init() {
-        run_init_hooks();
+        init_submodules();
 
         parse_attribute_config(RuntimeConfig::get_default_config().init("caliper", s_configdata));
 
-        services::add_default_service_specs();
-
         if (Log::verbosity() >= 2)
             print_available_services( Log(2).stream() << "Available services: " ) << std::endl;
 
@@ -614,8 +587,6 @@ mutex                  Caliper::GlobalData::s_init_mutex;
 Caliper::GlobalData::S_GObject Caliper::GlobalData::gObj;
 thread_local Caliper::GlobalData::S_TLSObject Caliper::GlobalData::tObj;
 
-Caliper::GlobalData::InitHookList* Caliper::GlobalData::s_init_hooks = nullptr;
-
 const ConfigSet::Entry Caliper::GlobalData::s_configdata[] = {
     // key, type, value, short description, long description
     { "attribute_properties", CALI_TYPE_STRING, "",
@@ -1956,12 +1927,3 @@ Caliper::add_services(const CaliperService* s)
 {
     services::add_service_specs(s);
 }
-
-void
-Caliper::add_init_hook(void (*hook)())
-{
-    if (is_initialized())
-        Log(0).stream() << "add_init_hook(): Caliper is already initialized - cannot add init hook" << std::endl;
-    else
-        GlobalData::add_init_hook(hook);
-}

src/caliper/ConfigManager.cpp

@@ -24,6 +24,8 @@ namespace cali
 extern const ConfigManager::ConfigInfo* builtin_controllers_table[];
 extern const char* builtin_option_specs;
 
+extern void add_submodule_controllers_and_services();
+
 }
 
 namespace
@@ -369,7 +371,7 @@ struct ConfigManager::Options::OptionsImpl
         // are there
         //
 
-        services::add_default_service_specs();
+        add_submodule_controllers_and_services();
         auto slist = services::get_available_services();
 
         for (const std::string &opt : enabled_options) {
@@ -762,6 +764,8 @@ struct ConfigManager::ConfigManagerImpl
 
     void
     import_builtin_config_specs() {
+        add_submodule_controllers_and_services();
+
         for (const ConfigInfo& s : ::ConfigSpecManager::instance()->get_config_specs())
             add_config_spec(s.spec, s.create, s.check_args, true /* ignore existing */ );
     }

src/caliper/setup_mpi.cpp

@@ -0,0 +1,37 @@
+// Copyright (c) 2020, Lawrence Livermore National Security, LLC.
+// See top-level LICENSE file for details.
+
+#include "caliper/caliper-config.h"
+
+#include "../services/Services.h"
+
+namespace cali
+{
+
+#ifdef CALIPER_HAVE_MPI
+namespace mpi
+{
+
+extern void setup_mpi();
+extern void add_mpi_controllers_and_services();
+
+}
+#endif
+
+void add_submodule_controllers_and_services()
+{
+    services::add_default_service_specs();
+#ifdef CALIPER_HAVE_MPI
+    mpi::add_mpi_controllers_and_services();
+#endif
+}
+
+void init_submodules()
+{
+#ifdef CALIPER_HAVE_MPI
+    mpi::setup_mpi();
+#endif
+    add_submodule_controllers_and_services();
+}
+
+} // namespace cali

src/caliper/setup_serial.cpp

@@ -0,0 +1,21 @@
+// Copyright (c) 2020, Lawrence Livermore National Security, LLC.
+// See top-level LICENSE file for details.
+
+#include "caliper/caliper-config.h"
+
+#include "../services/Services.h"
+
+namespace cali
+{
+
+void add_submodule_controllers_and_services()
+{
+    services::add_default_service_specs();
+}
+
+void init_submodules()
+{
+    add_submodule_controllers_and_services();
+}
+
+} // namespace cali

src/common/Log.cpp

@@ -159,4 +159,10 @@ Log::fini()
 {
     delete LogImpl::s_instance;
     LogImpl::s_instance = nullptr;
-}
+}
+
+bool
+Log::is_initialized()
+{
+    return LogImpl::s_instance != nullptr;
+}

src/mpi/CMakeLists.txt

@@ -5,7 +5,7 @@ set(CALIPER_MPI_ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(CALIPER_MPI_SOURCES
   aggregate_over_mpi.cpp
   mpi_machine.cpp
-  setup_mpi.cpp)
+  mpi_setup.cpp)
 
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}) # Find MpiEvents.h
 

src/mpi/setup_mpi.cpp → src/mpi/mpi_setup.cpp

@@ -31,20 +31,23 @@ extern CaliperService tau_service;
 
 extern ConfigManager::ConfigInfo spot_controller_info;
 extern ConfigManager::ConfigInfo spot_v1_controller_info;
+}
 
-//   Pre-init setup routine that allows us to do some MPI-specific
-// initialization, e.g. disabling most logging on non-rank 0 ranks.
-//
-//   This is called by Caliper::init() _before_ Caliper is initialized:
-// can't use the Caliper API here!
+namespace
+{
 
 void
-setup_mpi()
+setup_log_prefix()
 {
-    int is_initialized = 0;
-    MPI_Initialized(&is_initialized);
+    static bool done = false;
+
+    if (done)
+        return;
 
-    if (is_initialized) {
+    int mpi_is_initialized = 0;
+    MPI_Initialized(&mpi_is_initialized);
+
+    if (mpi_is_initialized && Log::is_initialized()) {
         int rank = 0;
 
         MPI_Comm_rank(MPI_COMM_WORLD, &rank);
@@ -57,14 +60,20 @@ setup_mpi()
 
         if (rank > 0)
             Log::set_verbosity(0);
+
+        done = true;
     }
 }
 
+} // namespace [anonymous]
 
-// void mpirt_constructor() __attribute__((constructor));
+namespace cali
+{
+namespace mpi
+{
 
 void
-mpirt_constructor()
+add_mpi_controllers_and_services()
 {
     CaliperService services[] = {
         mpiwrap_service,
@@ -84,25 +93,27 @@ mpirt_constructor()
         nullptr
     };
 
-    if (Caliper::is_initialized())
-        setup_mpi();
-    else
-        Caliper::add_init_hook(setup_mpi);
-
     Caliper::add_services(services);
-
     add_global_config_specs(controllers);
 }
 
+void
+setup_mpi()
+{
+    ::setup_log_prefix();
 }
 
+} // namespace mpi
+} // namespace cali
+
+
 extern "C"
 {
 
 void
 cali_mpi_init()
 {
-    ::mpirt_constructor();
+    ::setup_log_prefix();
 }
 
 }

test/cali-annotation-perftest.cpp

@@ -30,13 +30,6 @@
 #include <caliper/cali.h>
 #include <caliper/tools-util/Args.h>
 
-#ifdef CALIPER_HAVE_MPI
-extern "C"
-{
-void cali_mpi_init(); // init spot controller
-}
-#endif
-
 #include <chrono>
 #include <iostream>
 
@@ -139,9 +132,6 @@ void record_globals(const Config& cfg, int threads, const cali::ConfigManager::a
 
 int main(int argc, char* argv[])
 {
-#ifdef CALIPER_HAVE_MPI
-    cali_mpi_init(); // init spot controller
-#endif
     cali_config_preset("CALI_ATTRIBUTE_DEFAULT_SCOPE", "process");
 
     const util::Args::Table option_table[] = {