From 2ad536e710c5936ff8e525e4bbb5e7046f292aac Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 20 Feb 2023 15:46:02 +0100 Subject: [PATCH] Simplify the library initialization + deprecate 2 XBT functions Work toward making EngineImpl::initialize() the only entry point of the library initialization. It's now impossible to initialize xbt separately of EngineImpl. It should help reducing the mess in that code, which results from the many ways of initializing the library. simgrid-mc, unit-tests, smpi and others each have their own way of initializing everything, resulting in a spagetthi and fragile code. It's a bit of a waste to initialize an EngineImpl even when you don't want to actually run a simulation, but easier code is always better. This commit also includes another one because I git amended locally by error, sorry for the mess. The other commit was about the deprecation of xbt_procname and xbt_getpid that were xbt functions relying on S4U. --- MANIFEST.in | 1 - include/simgrid/Exception.hpp | 3 +- include/simgrid/actor.h | 1 + include/simgrid/engine.h | 6 ++-- include/xbt.h | 2 -- include/xbt/module.h | 13 +++++-- include/xbt/virtu.h | 11 ++++-- src/kernel/EngineImpl.cpp | 28 ++++++++++++--- .../resource/profile/StochasticDatedValue.cpp | 4 ++- src/s4u/s4u_Actor.cpp | 8 +++-- src/s4u/s4u_Engine.cpp | 2 +- src/simgrid/sg_config.cpp | 3 +- src/xbt/backtrace.cpp | 6 ++-- src/xbt/exception.cpp | 6 ++-- src/xbt/xbt_log_layout_format.cpp | 9 +++-- src/xbt/xbt_log_layout_simple.cpp | 9 ++--- src/xbt/xbt_main.cpp | 36 ------------------- src/xbt/xbt_modinter.h | 2 -- src/xbt/xbt_virtu.cpp | 22 ------------ teshsuite/models/lmm_usage/lmm_usage.cpp | 1 - .../models/maxmin_bench/maxmin_bench.cpp | 1 - teshsuite/xbt/mmalloc/mmalloc_test.cpp | 3 +- tools/cmake/DefinePackages.cmake | 1 - 23 files changed, 81 insertions(+), 97 deletions(-) delete mode 100644 src/xbt/xbt_virtu.cpp diff --git a/MANIFEST.in b/MANIFEST.in index 5c50b594f5..c920f5d355 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -2499,7 +2499,6 @@ include src/xbt/xbt_parse_units.cpp include src/xbt/xbt_replay.cpp include src/xbt/xbt_str.cpp include src/xbt/xbt_str_test.cpp -include src/xbt/xbt_virtu.cpp include teshsuite/kernel/CMakeLists.txt include teshsuite/mc/CMakeLists.txt include teshsuite/models/CMakeLists.txt diff --git a/include/simgrid/Exception.hpp b/include/simgrid/Exception.hpp index e580ea9117..f4d151f83e 100644 --- a/include/simgrid/Exception.hpp +++ b/include/simgrid/Exception.hpp @@ -51,7 +51,8 @@ public: /** Create a ThrowPoint with (__FILE__, __LINE__, __func__) */ #define XBT_THROW_POINT \ - ::simgrid::xbt::ThrowPoint(__FILE__, __LINE__, __func__, simgrid::xbt::Backtrace(), xbt_procname(), xbt_getpid()) + ::simgrid::xbt::ThrowPoint(__FILE__, __LINE__, __func__, simgrid::xbt::Backtrace(), sg_actor_self_get_name(), \ + sg_actor_self_get_pid()) class XBT_PUBLIC ImpossibleError : public std::logic_error { public: diff --git a/include/simgrid/actor.h b/include/simgrid/actor.h index dcee50796a..fb34364914 100644 --- a/include/simgrid/actor.h +++ b/include/simgrid/actor.h @@ -72,6 +72,7 @@ XBT_PUBLIC void sg_actor_detach(); XBT_PUBLIC sg_actor_t sg_actor_self(); XBT_PUBLIC aid_t sg_actor_self_get_pid(); XBT_PUBLIC aid_t sg_actor_self_get_ppid(); +/** Returns the name of the current actor (or "maestro" if maestro is running) */ XBT_PUBLIC const char* sg_actor_self_get_name(); XBT_PUBLIC void* sg_actor_self_get_data(); XBT_PUBLIC void sg_actor_self_set_data(void* data); diff --git a/include/simgrid/engine.h b/include/simgrid/engine.h index b4e8c08193..d3ff02b859 100644 --- a/include/simgrid/engine.h +++ b/include/simgrid/engine.h @@ -11,9 +11,9 @@ SG_BEGIN_DECL /* C interface */ - /** Initialize the SimGrid engine, taking the command line parameters of your main function. */ - XBT_PUBLIC void - simgrid_init(int* argc, char** argv); +/** Initialize the SimGrid engine, taking the command line parameters of your main function. */ +XBT_PUBLIC void +simgrid_init(int* argc, char** argv); /** Creates a new platform, including hosts, links, and the routing table. * diff --git a/include/xbt.h b/include/xbt.h index deafed807e..0f6e5b8c04 100644 --- a/include/xbt.h +++ b/include/xbt.h @@ -17,8 +17,6 @@ #include #include -#include - #include #include diff --git a/include/xbt/module.h b/include/xbt/module.h index 43fe15d755..ee9d548160 100644 --- a/include/xbt/module.h +++ b/include/xbt/module.h @@ -8,11 +8,20 @@ #ifndef XBT_MODULE_H #define XBT_MODULE_H -#include /* XBT_PUBLIC */ +// avoid deprecation warning on include (remove entire file with XBT_ATTRIB_DEPRECATED_v337) +#ifndef XBT_MODULE_H_NO_DEPRECATED_WARNING +#warning xbt/module.h is deprecated and will be removed in v3.37. +#endif + +#include +#include SG_BEGIN_DECL -XBT_PUBLIC void xbt_init(int* argc, char** argv); +XBT_ATTRIB_DEPRECATED_v337("Please use simgrid_init(&argc, argv) instead") static void xbt_init(int* argc, char** argv) +{ + simgrid_init(argc, argv); +} SG_END_DECL diff --git a/include/xbt/virtu.h b/include/xbt/virtu.h index b1a6e04c5e..136b305dd9 100644 --- a/include/xbt/virtu.h +++ b/include/xbt/virtu.h @@ -8,6 +8,7 @@ #ifndef XBT_VIRTU_H #define XBT_VIRTU_H +#include #include #ifdef __cplusplus @@ -28,9 +29,15 @@ XBT_PUBLIC_DATA std::vector cmdline; SG_BEGIN_DECL -XBT_PUBLIC const char* xbt_procname(void); +XBT_ATTRIB_DEPRECATED_v337("Please use sg_actor_self_get_name()") static const char* xbt_procname(void) +{ + return sg_actor_self_get_name() ?: "maestro"; +} -XBT_PUBLIC int xbt_getpid(void); +XBT_ATTRIB_DEPRECATED_v337("Please use sg_actor_self_get_pid()") static int xbt_getpid(void) +{ + return sg_actor_self_get_pid(); +}; SG_END_DECL diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index 1547836e44..025228ae6e 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -20,7 +20,8 @@ #include "src/simgrid/sg_config.hpp" #include "src/smpi/include/smpi_actor.hpp" #include "src/xbt/xbt_modinter.h" /* whether initialization was already done */ -#include "xbt/module.h" + +#include "xbt/log.hpp" #include #include @@ -139,6 +140,15 @@ static void install_signal_handlers() } } +static simgrid::config::Flag cfg_dbg_clean_atexit{ + "debug/clean-atexit", "Whether to cleanup SimGrid at exit. Disable it if your code segfaults after its end.", true}; +static void xbt_postexit() +{ + if (not cfg_dbg_clean_atexit) + return; + xbt_log_postexit(); +} + namespace simgrid::kernel { EngineImpl::~EngineImpl() @@ -175,8 +185,18 @@ void EngineImpl::initialize(int* argc, char** argv) simgrid::mc::AppSide::initialize(); #endif - if (xbt_initialized == 0) { - xbt_init(argc, argv); + static bool inited = false; + if (not inited) { + inited = true; + atexit(xbt_postexit); + xbt_log_init(argc, argv); + + simgrid::xbt::install_exception_handler(); + + if (*argc > 0) + simgrid::xbt::binary_name = argv[0]; + for (int i = 0; i < *argc; i++) + simgrid::xbt::cmdline.emplace_back(argv[i]); sg_config_init(argc, argv); } @@ -188,7 +208,7 @@ void EngineImpl::initialize(int* argc, char** argv) /* register a function to be called after the environment creation */ s4u::Engine::on_platform_created_cb([this]() { this->presolve(); }); - if (config::get_value("debug/clean-atexit")) + if (cfg_dbg_clean_atexit) atexit(shutdown); } diff --git a/src/kernel/resource/profile/StochasticDatedValue.cpp b/src/kernel/resource/profile/StochasticDatedValue.cpp index 4f7208ad7f..c1d383ebe7 100644 --- a/src/kernel/resource/profile/StochasticDatedValue.cpp +++ b/src/kernel/resource/profile/StochasticDatedValue.cpp @@ -4,8 +4,10 @@ * under the terms of the license (GNU LGPL) which comes with this package. */ #include "src/kernel/resource/profile/StochasticDatedValue.hpp" -#include "xbt.h" + +#include "xbt/asserts.h" #include "xbt/random.hpp" + #include namespace simgrid::kernel::profile { diff --git a/src/s4u/s4u_Actor.cpp b/src/s4u/s4u_Actor.cpp index ea0ea365f0..1e022aa5ea 100644 --- a/src/s4u/s4u_Actor.cpp +++ b/src/s4u/s4u_Actor.cpp @@ -413,7 +413,8 @@ ExecPtr exec_async(double flops) aid_t get_pid() { - return simgrid::kernel::actor::ActorImpl::self()->get_pid(); + auto* self = simgrid::kernel::actor::ActorImpl::self(); + return self ? self->get_pid() : 0; } aid_t get_ppid() @@ -428,7 +429,8 @@ std::string get_name() const char* get_cname() { - return simgrid::kernel::actor::ActorImpl::self()->get_cname(); + auto* self = simgrid::kernel::actor::ActorImpl::self(); + return self ? self->get_cname() : nullptr; } Host* get_host() @@ -753,6 +755,8 @@ aid_t sg_actor_self_get_ppid() const char* sg_actor_self_get_name() { + if (simgrid::s4u::Actor::is_maestro()) + return "maestro"; return simgrid::s4u::this_actor::get_cname(); } diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp index c20cd8e857..3ed3452015 100644 --- a/src/s4u/s4u_Engine.cpp +++ b/src/s4u/s4u_Engine.cpp @@ -587,7 +587,7 @@ Engine* Engine::set_default_comm_data_copy_callback( /* **************************** Public C interface *************************** */ void simgrid_init(int* argc, char** argv) { - static simgrid::s4u::Engine e(argc, argv); + simgrid::s4u::Engine::get_instance(argc, argv); } void simgrid_load_platform(const char* file) { diff --git a/src/simgrid/sg_config.cpp b/src/simgrid/sg_config.cpp index ddaec3f509..fb1df9d4fc 100644 --- a/src/simgrid/sg_config.cpp +++ b/src/simgrid/sg_config.cpp @@ -137,6 +137,7 @@ void sg_config_init(int *argc, char **argv) XBT_WARN("Call to sg_config_init() after initialization ignored"); return; } + _sg_cfg_init_status = 1; /* Plugins and models configuration */ simgrid_plugins().create_flag("plugin", "The plugins", "", true); @@ -234,8 +235,6 @@ void sg_config_init(int *argc, char **argv) static simgrid::config::Flag cfg_execution_cutpath{ "exception/cutpath", "Whether to cut all path information from call traces, used e.g. in exceptions.", false}; - _sg_cfg_init_status = 1; - sg_config_cmd_line(argc, argv); xbt_mallocator_initialization_is_done(simgrid::kernel::context::Context::is_parallel()); diff --git a/src/xbt/backtrace.cpp b/src/xbt/backtrace.cpp index 24ebc39921..4914d2b0b1 100644 --- a/src/xbt/backtrace.cpp +++ b/src/xbt/backtrace.cpp @@ -5,10 +5,11 @@ #include "src/internal_config.h" +#include +#include #include #include #include -#include #include #include @@ -90,7 +91,8 @@ std::string Backtrace::resolve() const void Backtrace::display() const { std::string backtrace = resolve(); - std::fprintf(stderr, "Backtrace (displayed in actor %s%s):\n%s\n", xbt_procname(), + std::fprintf(stderr, "Backtrace (displayed in actor %s%s):\n%s\n", + simgrid::s4u::Actor::is_maestro() ? "maestro" : sg_actor_self_get_name(), (xbt_log_no_loc ? " -- short trace because of --log=no_loc" : ""), backtrace.empty() ? "(backtrace not set -- did you install Boost.Stacktrace?)" : backtrace.c_str()); } diff --git a/src/xbt/exception.cpp b/src/xbt/exception.cpp index 5aa9eae501..51e9649d72 100644 --- a/src/xbt/exception.cpp +++ b/src/xbt/exception.cpp @@ -17,9 +17,9 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(xbt_exception, xbt, "Exceptions"); void _xbt_throw(char* message, const char* file, int line, const char* func) { - simgrid::Exception e( - simgrid::xbt::ThrowPoint(file, line, func, simgrid::xbt::Backtrace(), xbt_procname(), xbt_getpid()), - message ? message : ""); + simgrid::Exception e(simgrid::xbt::ThrowPoint(file, line, func, simgrid::xbt::Backtrace(), sg_actor_self_get_name(), + sg_actor_self_get_pid()), + message ? message : ""); xbt_free(message); throw e; } diff --git a/src/xbt/xbt_log_layout_format.cpp b/src/xbt/xbt_log_layout_format.cpp index 61bb830b77..e09d701257 100644 --- a/src/xbt/xbt_log_layout_format.cpp +++ b/src/xbt/xbt_log_layout_format.cpp @@ -9,7 +9,9 @@ #include "simgrid/host.h" #include "src/xbt/log_private.hpp" #include "xbt/sysdep.h" -#include "xbt/virtu.h" +#include +#include + #include #include @@ -67,6 +69,7 @@ static constexpr const char* ERRMSG = } else \ (void)0 #define show_int(data) show_it((data), "d") +#define show_long(data) show_it((data), "ld") #define show_double(data) show_it((data), "f") static bool xbt_log_layout_format_doit(const s_xbt_log_layout_t* l, xbt_log_event_t ev, const char* msg_fmt) @@ -134,10 +137,10 @@ static bool xbt_log_layout_format_doit(const s_xbt_log_layout_t* l, xbt_log_even case 't': /* thread/process name; LOG4J compliant */ case 'P': /* Used before SimGrid 3.26 and kept for compatiblity. Should not hurt. */ case 'a': /* actor name; SimGrid extension */ - show_string(xbt_procname()); + show_string(sg_actor_self_get_name()); break; case 'i': /* actor ID; SimGrid extension */ - show_int(xbt_getpid()); + show_long(sg_actor_self_get_pid()); break; case 'F': /* file name; LOG4J compliant */ show_string(ev->fileName); diff --git a/src/xbt/xbt_log_layout_simple.cpp b/src/xbt/xbt_log_layout_simple.cpp index a689123749..da43e17984 100644 --- a/src/xbt/xbt_log_layout_simple.cpp +++ b/src/xbt/xbt_log_layout_simple.cpp @@ -7,7 +7,8 @@ #include "src/xbt/log_private.hpp" #include "xbt/sysdep.h" -#include "xbt/virtu.h" +#include +#include #include "simgrid/engine.h" /* simgrid_get_clock */ #include "simgrid/host.h" /* sg_host_self_get_name */ @@ -34,12 +35,12 @@ static bool xbt_log_layout_simple_doit(const s_xbt_log_layout_t*, xbt_log_event_ check_overflow(1); /* Display the proc info if available */ - procname = xbt_procname(); + procname = sg_actor_self_get_name(); if (procname && strcmp(procname,"maestro")) { - len = snprintf(p, rem_size, "%s:%s:(%d) ", sg_host_self_get_name(), procname, xbt_getpid()); + len = snprintf(p, rem_size, "%s:%s:(%ld) ", sg_host_self_get_name(), procname, sg_actor_self_get_pid()); check_overflow(len); } else if (not procname) { - len = snprintf(p, rem_size, "%s::(%d) ", sg_host_self_get_name(), xbt_getpid()); + len = snprintf(p, rem_size, "%s::(%ld) ", sg_host_self_get_name(), sg_actor_self_get_pid()); check_overflow(len); } diff --git a/src/xbt/xbt_main.cpp b/src/xbt/xbt_main.cpp index d8a906ebb6..48e5edfef4 100644 --- a/src/xbt/xbt_main.cpp +++ b/src/xbt/xbt_main.cpp @@ -18,7 +18,6 @@ #include "xbt/log.h" #include "xbt/log.hpp" #include "xbt/misc.h" -#include "xbt/module.h" /* this module */ #include "xbt/sysdep.h" #include @@ -38,13 +37,6 @@ std::string binary_name; /* Name of the system process containing us (m std::vector cmdline; /* all we got in argv */ } // namespace simgrid::xbt - -int xbt_initialized = 0; -simgrid::config::Flag cfg_dbg_clean_atexit{ - "debug/clean-atexit", - "Whether to cleanup SimGrid at exit. Disable it if your code segfaults after its end.", - true}; - const int xbt_pagesize = static_cast(sysconf(_SC_PAGESIZE)); const int xbt_pagebits = static_cast(log2(xbt_pagesize)); @@ -57,34 +49,6 @@ XBT_ATTRIB_NOINLINE void sthread_disable() asm(""); } -static void xbt_postexit() -{ - if (not cfg_dbg_clean_atexit) - return; - xbt_initialized--; - xbt_log_postexit(); -} - -/** @brief Initialize the xbt mechanisms. */ -void xbt_init(int *argc, char **argv) -{ - xbt_initialized++; - if (xbt_initialized > 1) { - XBT_DEBUG("XBT has been initialized %d times.", xbt_initialized); - return; - } - atexit(xbt_postexit); - - simgrid::xbt::install_exception_handler(); - - if (*argc > 0) - simgrid::xbt::binary_name = argv[0]; - for (int i = 0; i < *argc; i++) - simgrid::xbt::cmdline.emplace_back(argv[i]); - - xbt_log_init(argc, argv); -} - /* these two functions belong to xbt/sysdep.h, which have no corresponding .c file */ /** @brief like xbt_free, but you can be sure that it is a function */ void xbt_free_f(void* p) noexcept(noexcept(::free)) diff --git a/src/xbt/xbt_modinter.h b/src/xbt/xbt_modinter.h index bd3625eb21..b715ae3976 100644 --- a/src/xbt/xbt_modinter.h +++ b/src/xbt/xbt_modinter.h @@ -15,8 +15,6 @@ SG_BEGIN_DECL void xbt_log_postexit(void); -extern int xbt_initialized; - SG_END_DECL #endif diff --git a/src/xbt/xbt_virtu.cpp b/src/xbt/xbt_virtu.cpp deleted file mode 100644 index 56b26168a9..0000000000 --- a/src/xbt/xbt_virtu.cpp +++ /dev/null @@ -1,22 +0,0 @@ -/* virtualization layer for XBT */ - -/* Copyright (c) 2007-2023. The SimGrid Team. All rights reserved. */ - -/* This program is free software; you can redistribute it and/or modify it - * under the terms of the license (GNU LGPL) which comes with this package. */ - -#include -#include - -#include "src/kernel/actor/ActorImpl.hpp" - -int xbt_getpid() -{ - const auto* self = simgrid::kernel::actor::ActorImpl::self(); - return self == nullptr ? 0 : static_cast(self->get_pid()); -} - -const char* xbt_procname(void) -{ - return simgrid::s4u::Actor::is_maestro() ? "maestro" : simgrid::kernel::actor::ActorImpl::self()->get_cname(); -} diff --git a/teshsuite/models/lmm_usage/lmm_usage.cpp b/teshsuite/models/lmm_usage/lmm_usage.cpp index c37cfdc0c1..190d995bdd 100644 --- a/teshsuite/models/lmm_usage/lmm_usage.cpp +++ b/teshsuite/models/lmm_usage/lmm_usage.cpp @@ -8,7 +8,6 @@ #include "simgrid/s4u/Engine.hpp" #include "src/kernel/lmm/maxmin.hpp" #include "xbt/log.h" -#include "xbt/module.h" #include "xbt/sysdep.h" #include #include diff --git a/teshsuite/models/maxmin_bench/maxmin_bench.cpp b/teshsuite/models/maxmin_bench/maxmin_bench.cpp index d905956d18..b43d234cf6 100644 --- a/teshsuite/models/maxmin_bench/maxmin_bench.cpp +++ b/teshsuite/models/maxmin_bench/maxmin_bench.cpp @@ -7,7 +7,6 @@ #include "simgrid/s4u/Engine.hpp" #include "src/kernel/lmm/maxmin.hpp" -#include "xbt/module.h" #include "xbt/random.hpp" #include "xbt/sysdep.h" /* time manipulation for benchmarking */ #include "xbt/xbt_os_time.h" diff --git a/teshsuite/xbt/mmalloc/mmalloc_test.cpp b/teshsuite/xbt/mmalloc/mmalloc_test.cpp index 3b03ea9ae5..c76ea464f9 100644 --- a/teshsuite/xbt/mmalloc/mmalloc_test.cpp +++ b/teshsuite/xbt/mmalloc/mmalloc_test.cpp @@ -4,6 +4,7 @@ * under the terms of the license (GNU LGPL) which comes with this package. */ #include "simgrid/Exception.hpp" +#include "simgrid/engine.h" #include "src/xbt/mmalloc/mmalloc.h" #include "xbt.h" @@ -34,7 +35,7 @@ int main(int argc, char**argv) { xbt_mheap_t heapA = nullptr; std::array pointers; - xbt_init(&argc,argv); + simgrid_init(&argc, argv); XBT_INFO("Allocating a new heap"); unsigned long mask = ~((unsigned long)xbt_pagesize - 1); diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake index c30d4cd519..874ea7dac6 100644 --- a/tools/cmake/DefinePackages.cmake +++ b/tools/cmake/DefinePackages.cmake @@ -284,7 +284,6 @@ set(XBT_SRC src/xbt/xbt_parse_units.cpp src/xbt/xbt_replay.cpp src/xbt/xbt_str.cpp - src/xbt/xbt_virtu.cpp ) if(HAVE_MMALLOC) -- 2.20.1