From e816186fdeaf53fa5abf6886f157c499e09c4622 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Tue, 15 Jan 2019 23:34:32 +0100 Subject: [PATCH] Implement thread factory with std::thread, and cleanups --- ChangeLog | 6 +++ docs/source/Configuring_SimGrid.rst | 23 +++++++----- include/simgrid/simix.h | 2 - include/xbt/xbt_os_thread.h | 3 -- src/bindings/java/JavaContext.cpp | 1 - src/kernel/context/ContextThread.cpp | 9 +---- src/kernel/context/ContextThread.hpp | 4 +- src/simgrid/sg_config.cpp | 4 +- src/simix/smx_context.cpp | 5 --- src/xbt/xbt_os_thread.c | 56 +--------------------------- 10 files changed, 27 insertions(+), 86 deletions(-) diff --git a/ChangeLog b/ChangeLog index 41b70b5e3d..2e43e785f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,11 +22,17 @@ XBT: OsSemaphore is implemented in a portable way with C++11 threads. This should allow much more threads to be created at the same time, allowing Mac and Java users to simulate many more actors. + - Implement the 'thread' factory with std::thread instead of xbt ones. + It is not possible to set the stack size with threads anymore, but + -fsplit-stack is the way to go nowadays when using threads. - Drop several unused xbt_os_thread_t functions: + - xbt_os_thread_exit() - xbt_os_thread_get_extra_data() - xbt_os_thread_set_extra_data() - xbt_os_thread_self() - xbt_os_thread_self_name() + - xbt_os_thread_setstacksize() + - xbt_os_thread_setguardsize() - Drop xbt_ex_display(), use simgrid::xbt::log_exception() instead. Fixed bugs: diff --git a/docs/source/Configuring_SimGrid.rst b/docs/source/Configuring_SimGrid.rst index 77a615f34c..63383b8347 100644 --- a/docs/source/Configuring_SimGrid.rst +++ b/docs/source/Configuring_SimGrid.rst @@ -798,8 +798,10 @@ stacks), leading to segfaults with corrupted stack traces. If you want to push the scalability limits of your code, you might want to reduce the ``contexts/stack-size`` item. Its default value is 8192 (in KiB), while our Chord simulation works with stacks as small -as 16 KiB, for example. For the thread factory, the default value is -the one of the system but you can still change it with this parameter. +as 16 KiB, for example. This *setting is ignored* when using the +thread factory. Instead, you should compile SimGrid and your +application with ``-fsplit-stack``. Note that this compilation flag is +not compatible with the model-checker right now. The operating system should only allocate memory for the pages of the stack which are actually used and you might not need to use this in @@ -813,14 +815,15 @@ Disabling Stack Guard Pages **Option** ``contexts/guard-size`` **Default** 1 page in most case (0 pages on Windows or with MC) -A stack guard page is usually used which prevents the stack of a given -actor from overflowing on another stack. But the performance impact -may become prohibitive when the amount of actors increases. The -option ``contexts/guard-size`` is the number of stack guard pages -used. By setting it to 0, no guard pages will be used: in this case, -you should avoid using small stacks (with :ref:`contexts/stack-size -`) as the stack will silently overflow on -other parts of the memory. +Unless you use the threads context factory (see +:ref:`cfg=contexts/factory`), a stack guard page is usually used +which prevents the stack of a given actor from overflowing on another +stack. But the performance impact may become prohibitive when the +amount of actors increases. The option ``contexts/guard-size`` is the +number of stack guard pages used. By setting it to 0, no guard pages +will be used: in this case, you should avoid using small stacks (with +:ref:`contexts/stack-size `) as the stack +will silently overflow on other parts of the memory. When no stack guard page is created, stacks may then silently overflow on other parts of the memory if their size is too small for the diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index 017cb2eb2b..b9618c6911 100644 --- a/include/simgrid/simix.h +++ b/include/simgrid/simix.h @@ -61,9 +61,7 @@ typedef enum { /******************************* Networking ***********************************/ extern unsigned smx_context_stack_size; -extern int smx_context_stack_size_was_set; extern unsigned smx_context_guard_size; -extern int smx_context_guard_size_was_set; SG_BEGIN_DECL() diff --git a/include/xbt/xbt_os_thread.h b/include/xbt/xbt_os_thread.h index f09f1a1a8f..44e6557ebc 100644 --- a/include/xbt/xbt_os_thread.h +++ b/include/xbt/xbt_os_thread.h @@ -27,12 +27,9 @@ SG_BEGIN_DECL() /** @brief Thread data type (opaque structure) */ typedef struct xbt_os_thread_ *xbt_os_thread_t; XBT_PUBLIC xbt_os_thread_t xbt_os_thread_create(pvoid_f_pvoid_t start_routine, void* param); -XBT_PUBLIC void xbt_os_thread_exit(int* retcode); /* xbt_os_thread_join frees the joined thread (ie the XBT wrapper around it, the OS frees the rest) */ XBT_PUBLIC void xbt_os_thread_join(xbt_os_thread_t thread, void** thread_return); -XBT_PUBLIC void xbt_os_thread_setstacksize(int stack_size); -XBT_PUBLIC void xbt_os_thread_setguardsize(int guard_size); XBT_PUBLIC int xbt_os_thread_bind(xbt_os_thread_t thread, int core); XBT_PUBLIC int xbt_os_thread_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void)); diff --git a/src/bindings/java/JavaContext.cpp b/src/bindings/java/JavaContext.cpp index 115bb0f49f..7cc00a21ac 100644 --- a/src/bindings/java/JavaContext.cpp +++ b/src/bindings/java/JavaContext.cpp @@ -108,7 +108,6 @@ void JavaContext::stop() env->DeleteGlobalRef(this->jprocess_); XBT_ATTRIB_UNUSED jint error = __java_vm->DetachCurrentThread(); xbt_assert((error == JNI_OK), "The thread couldn't be detached."); - xbt_os_thread_exit(nullptr); } } diff --git a/src/kernel/context/ContextThread.cpp b/src/kernel/context/ContextThread.cpp index 8b4acdf98e..d22c7aa318 100644 --- a/src/kernel/context/ContextThread.cpp +++ b/src/kernel/context/ContextThread.cpp @@ -63,13 +63,8 @@ ThreadContext::ThreadContext(std::function code, void_pfn_smxprocess_t c { /* If the user provided a function for the process then use it */ if (has_code()) { - if (smx_context_stack_size_was_set) - xbt_os_thread_setstacksize(smx_context_stack_size); - if (smx_context_guard_size_was_set) - xbt_os_thread_setguardsize(smx_context_guard_size); - /* create and start the process */ - this->thread_ = xbt_os_thread_create(ThreadContext::wrapper, this); + this->thread_ = new std::thread(ThreadContext::wrapper, this); /* wait the starting of the newly created process */ this->end_.acquire(); } @@ -83,7 +78,7 @@ ThreadContext::ThreadContext(std::function code, void_pfn_smxprocess_t c ThreadContext::~ThreadContext() { if (this->thread_) /* If there is a thread (maestro don't have any), wait for its termination */ - xbt_os_thread_join(this->thread_, nullptr); + thread_->join(); } void *ThreadContext::wrapper(void *param) diff --git a/src/kernel/context/ContextThread.hpp b/src/kernel/context/ContextThread.hpp index 7eea375e8f..0dfc83b44f 100644 --- a/src/kernel/context/ContextThread.hpp +++ b/src/kernel/context/ContextThread.hpp @@ -13,6 +13,8 @@ #include "src/xbt/OsSemaphore.hpp" #include "xbt/xbt_os_thread.h" +#include + namespace simgrid { namespace kernel { namespace context { @@ -32,7 +34,7 @@ public: private: /** A portable thread */ - xbt_os_thread_t thread_ = nullptr; + std::thread* thread_ = nullptr; /** Semaphore used to schedule/yield the process (not needed when the maestro is in main, but harmless then) */ xbt::OsSemaphore begin_{0}; /** Semaphore used to schedule/unschedule (not needed when the maestro is in main, but harmless then) */ diff --git a/src/simgrid/sg_config.cpp b/src/simgrid/sg_config.cpp index d02aa1943a..aad9dd65fa 100644 --- a/src/simgrid/sg_config.cpp +++ b/src/simgrid/sg_config.cpp @@ -319,8 +319,8 @@ void sg_config_init(int *argc, char **argv) extern bool _sg_do_verbose_exit; simgrid::config::bind_flag(_sg_do_verbose_exit, "verbose-exit", "Activate the \"do nothing\" mode in Ctrl-C"); - simgrid::config::declare_flag("contexts/stack-size", "Stack size of contexts in KiB", 8 * 1024, - [](int value) { smx_context_stack_size = value * 1024; }); + simgrid::config::declare_flag("contexts/stack-size", "Stack size of contexts in KiB (not with threads)", + 8 * 1024, [](int value) { smx_context_stack_size = value * 1024; }); simgrid::config::alias("contexts/stack-size", {"contexts/stack_size"}); /* guard size for contexts stacks in memory pages */ diff --git a/src/simix/smx_context.cpp b/src/simix/smx_context.cpp index 23bb02e56d..533669351a 100644 --- a/src/simix/smx_context.cpp +++ b/src/simix/smx_context.cpp @@ -46,9 +46,7 @@ static simgrid::config::Flag context_factory_name( context_factories[0].first); unsigned smx_context_stack_size; -int smx_context_stack_size_was_set = 0; unsigned smx_context_guard_size; -int smx_context_guard_size_was_set = 0; static int smx_parallel_contexts = 1; static int smx_parallel_threshold = 2; static e_xbt_parmap_mode_t smx_parallel_synchronization_mode = XBT_PARMAP_DEFAULT; @@ -60,9 +58,6 @@ void SIMIX_context_mod_init() { xbt_assert(simix_global->context_factory == nullptr); - smx_context_stack_size_was_set = not simgrid::config::is_default("contexts/stack-size"); - smx_context_guard_size_was_set = not simgrid::config::is_default("contexts/guard-size"); - #if HAVE_SMPI && (defined(__APPLE__) || defined(__NetBSD__)) std::string priv = simgrid::config::get_value("smpi/privatization"); if (context_factory_name == "thread" && (priv == "dlopen" || priv == "yes" || priv == "default" || priv == "1")) { diff --git a/src/xbt/xbt_os_thread.c b/src/xbt/xbt_os_thread.c index 4276bd095b..707f5b9cf6 100644 --- a/src/xbt/xbt_os_thread.c +++ b/src/xbt/xbt_os_thread.c @@ -53,9 +53,6 @@ static xbt_os_thread_t main_thread = NULL; /* thread-specific data containing the xbt_os_thread_t structure */ static int thread_mod_inited = 0; -/* defaults attribute for pthreads */ -static pthread_attr_t thread_attr; - /* frees the xbt_os_thread_t corresponding to the current thread */ static void xbt_os_thread_free_thread_data(xbt_os_thread_t thread) { @@ -73,8 +70,6 @@ void xbt_os_thread_mod_preinit(void) main_thread->param = NULL; main_thread->start_routine = NULL; - pthread_attr_init(&thread_attr); - thread_mod_inited = 1; } @@ -111,7 +106,7 @@ xbt_os_thread_t xbt_os_thread_create(pvoid_f_pvoid_t start_routine, void* param) res_thread->start_routine = start_routine; res_thread->param = param; - int errcode = pthread_create(&(res_thread->t), &thread_attr, wrapper_start_routine, res_thread); + int errcode = pthread_create(&(res_thread->t), NULL, wrapper_start_routine, res_thread); xbt_assert(errcode == 0, "pthread_create failed: %s", strerror(errcode)); return res_thread; @@ -134,50 +129,6 @@ int xbt_os_thread_bind(XBT_ATTRIB_UNUSED xbt_os_thread_t thread, XBT_ATTRIB_UNUS return errcode; } -void xbt_os_thread_setstacksize(int stack_size) -{ - size_t alignment[] = { - xbt_pagesize, -#ifdef PTHREAD_STACK_MIN - PTHREAD_STACK_MIN, -#endif - 0 - }; - - xbt_assert(stack_size >= 0, "stack size %d is negative, maybe it exceeds MAX_INT?", stack_size); - - size_t sz = stack_size; - int res = pthread_attr_setstacksize(&thread_attr, sz); - - for (int i = 0; res == EINVAL && alignment[i] > 0; i++) { - /* Invalid size, try again with next multiple of alignment[i]. */ - size_t rem = sz % alignment[i]; - if (rem != 0 || sz == 0) { - size_t sz2 = sz - rem + alignment[i]; - XBT_DEBUG("pthread_attr_setstacksize failed for %zu, try again with %zu", sz, sz2); - sz = sz2; - res = pthread_attr_setstacksize(&thread_attr, sz); - } - } - - if (res == EINVAL) - XBT_WARN("invalid stack size (maybe too big): %zu", sz); - else if (res != 0) - XBT_WARN("unknown error %d in pthread stacksize setting: %zu", res, sz); -} - -void xbt_os_thread_setguardsize(int guard_size) -{ -#ifdef WIN32 - THROW_UNIMPLEMENTED; //pthread_attr_setguardsize is not implemented in pthread.h on windows -#else - size_t sz = guard_size; - int res = pthread_attr_setguardsize(&thread_attr, sz); - if (res) - XBT_WARN("pthread_attr_setguardsize failed (%d) for size: %zu", res, sz); -#endif -} - void xbt_os_thread_join(xbt_os_thread_t thread, void **thread_return) { int errcode = pthread_join(thread->t, thread_return); @@ -186,11 +137,6 @@ void xbt_os_thread_join(xbt_os_thread_t thread, void **thread_return) xbt_os_thread_free_thread_data(thread); } -void xbt_os_thread_exit(int *retval) -{ - pthread_exit(retval); -} - /****** mutex related functions ******/ typedef struct xbt_os_mutex_ { pthread_mutex_t m; -- 2.20.1