Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Ledest:erlang:25
erlang
3631-Avoid-truncating-thread-names-for-better-r...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 3631-Avoid-truncating-thread-names-for-better-runtime-obs.patch of Package erlang
From bc959f99652dbba79c60f7d062a127b48a276d0e Mon Sep 17 00:00:00 2001 From: lexprfuncall <carl.shapiro@gmail.com> Date: Wed, 1 Mar 2023 18:45:00 -0800 Subject: [PATCH] Avoid truncating thread names for better runtime observability The Erlang runtime gives many of its threads descriptive names. When those threads are part of a logical group, a unique ID is added to the name for disambiguation. After construction, many thread names in Erlang have a string length greater than 16 characters. To fit within operating system limits, Erlang then truncates them from right to left. To minimize confusion after truncation, the unique ID is always placed at the left of a thread name so its information is not likely to be lost. The convention used by Erlang presents challenges to the use of thread names as keys when reporting on thread activity at the operating system level. The more common convention, used by other runtimes, is to have the description followed by a unique ID. When followed, sorting threads by name places like workers next to each other and the unique ID can be dropped to create a grouping key. Placing the unique ID first, as Erlang does, means that a different strategy needs to be used for sorting the threads of an Erlang process. Furthermore, the truncation necessitates a complicated strategy for analyzing the description to identify a possible common substring to be used as a grouping key. This change switches the Erlang runtime to use the more common convention in order to make reporting on the thread usage in an Erlang process easier for tooling. To do so, it shortens the content of the initial printf(3) format strings to ensure their output is always 16 or fewer characters so the name is never truncated. It also moves the unique ID in the format string to the right of the description, so the names of worker threads appear next to each other after sorting alphabetically from left to right. To prevent the accidental creation of long thread names in the future, the silent truncation has been eliminated from the lowest-layer of thread functionality. It now returns an EINVAL when given a long name which will be caught when the runtime is started. This would break the NIF and driver libraries so the silent truncation has moved up to a higher layer in order to preserve compatibility. New unit tests have been added to test setting and getting thread names. --- erts/emulator/beam/erl_async.c | 2 +- erts/emulator/beam/erl_drv_thread.c | 4 +- erts/emulator/beam/erl_process.c | 14 +-- erts/emulator/beam/erl_threads.h | 8 ++ erts/emulator/beam/erl_trace.c | 2 +- erts/emulator/sys/unix/sys.c | 5 +- erts/include/internal/ethread.h | 3 +- erts/lib_src/pthread/ethread.c | 22 +--- erts/test/ethread_SUITE.erl | 10 +- erts/test/ethread_SUITE_data/ethread_tests.c | 119 ++++++++++++++++++- 10 files changed, 157 insertions(+), 32 deletions(-) diff --git a/erts/emulator/beam/erl_async.c b/erts/emulator/beam/erl_async.c index 3e3dc3a29d..85bc8ee853 100644 --- a/erts/emulator/beam/erl_async.c +++ b/erts/emulator/beam/erl_async.c @@ -199,7 +199,7 @@ erts_init_async(void) for (i = 0; i < erts_async_max_threads; i++) { ErtsAsyncQ *aq = async_q(i); - erts_snprintf(thr_opts.name, sizeof(thr_name), "async_%d", i+1); + erts_snprintf(thr_opts.name, sizeof(thr_name), "erts_async_%d", i+1); erts_thr_create(&aq->thr_id, async_main, (void*) aq, &thr_opts); } diff --git a/erts/emulator/beam/erl_drv_thread.c b/erts/emulator/beam/erl_drv_thread.c index 949d89232a..a17b98d1e6 100644 --- a/erts/emulator/beam/erl_drv_thread.c +++ b/erts/emulator/beam/erl_drv_thread.c @@ -609,6 +609,7 @@ erl_drv_thread_create(char *name, struct ErlDrvTid_ *dtid; ethr_thr_opts ethr_opts = ETHR_THR_OPTS_DEFAULT_INITER; ethr_thr_opts *use_opts; + char name_buff[ETHR_THR_NAME_MAX + 1]; if (!opts && !name) use_opts = NULL; @@ -616,7 +617,8 @@ erl_drv_thread_create(char *name, if(opts) ethr_opts.suggested_stack_size = opts->suggested_stack_size; - ethr_opts.name = name; + erts_snprintf(name_buff, sizeof(name_buff), "%s", name); + ethr_opts.name = name_buff; use_opts = ðr_opts; } diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c index c2ca5a03f3..e8ca466571 100644 --- a/erts/emulator/beam/erl_process.c +++ b/erts/emulator/beam/erl_process.c @@ -8776,7 +8776,7 @@ erts_start_schedulers(void) { ethr_tid tid; int res = 0; - char name[32]; + char name[ETHR_THR_NAME_MAX + 1]; ethr_thr_opts opts = ETHR_THR_OPTS_DEFAULT_INITER; int ix; @@ -8786,7 +8786,7 @@ erts_start_schedulers(void) if (erts_runq_supervision_interval) { opts.suggested_stack_size = 16; - erts_snprintf(opts.name, sizeof(name), "runq_supervisor"); + erts_snprintf(opts.name, sizeof(name), "erts_runq_sup"); erts_atomic_init_nob(&runq_supervisor_sleeping, 0); if (0 != ethr_event_init(&runq_supervision_event)) erts_exit(ERTS_ABORT_EXIT, "Failed to create run-queue supervision event\n"); @@ -8807,7 +8807,7 @@ erts_start_schedulers(void) for (ix = 0; ix < erts_no_schedulers; ix++) { ErtsSchedulerData *esdp = ERTS_SCHEDULER_IX(ix); ASSERT(ix == esdp->no - 1); - erts_snprintf(opts.name, sizeof(name), "%lu_scheduler", ix + 1); + erts_snprintf(opts.name, sizeof(name), "erts_sched_%d", ix + 1); res = ethr_thr_create(&esdp->tid, sched_thread_func, (void*)esdp, &opts); if (res != 0) { erts_exit(ERTS_ABORT_EXIT, "Failed to create scheduler thread %d, error = %d\n", ix, res); @@ -8821,7 +8821,7 @@ erts_start_schedulers(void) { for (ix = 0; ix < erts_no_dirty_cpu_schedulers; ix++) { ErtsSchedulerData *esdp = ERTS_DIRTY_CPU_SCHEDULER_IX(ix); - erts_snprintf(opts.name, sizeof(name), "%d_dirty_cpu_scheduler", ix + 1); + erts_snprintf(opts.name, sizeof(name), "erts_dcpus_%d", ix + 1); opts.suggested_stack_size = erts_dcpu_sched_thread_suggested_stack_size; res = ethr_thr_create(&esdp->tid,sched_dirty_cpu_thread_func,(void*)esdp,&opts); if (res != 0) @@ -8829,7 +8829,7 @@ erts_start_schedulers(void) } for (ix = 0; ix < erts_no_dirty_io_schedulers; ix++) { ErtsSchedulerData *esdp = ERTS_DIRTY_IO_SCHEDULER_IX(ix); - erts_snprintf(opts.name, sizeof(name), "%d_dirty_io_scheduler", ix + 1); + erts_snprintf(opts.name, sizeof(name), "erts_dios_%d", ix + 1); opts.suggested_stack_size = erts_dio_sched_thread_suggested_stack_size; res = ethr_thr_create(&esdp->tid,sched_dirty_io_thread_func,(void*)esdp,&opts); if (res != 0) @@ -8840,7 +8840,7 @@ erts_start_schedulers(void) ix = 0; while (ix < erts_no_aux_work_threads) { int id = ix == 0 ? 1 : ix + 1 - (int) erts_no_schedulers; - erts_snprintf(opts.name, sizeof(name), "%d_aux", id); + erts_snprintf(opts.name, sizeof(name), "erts_aux_%d", id); res = ethr_thr_create(&tid, aux_thread, (void *) (Sint) ix, &opts); if (res != 0) @@ -8867,7 +8867,7 @@ erts_start_schedulers(void) bpt->blocked = 0; bpt->id = ix; - erts_snprintf(opts.name, sizeof(name), "%d_poller", ix); + erts_snprintf(opts.name, sizeof(name), "erts_poll_%d", ix); res = ethr_thr_create(&tid, poll_thread, (void*) bpt, &opts); if (res != 0) diff --git a/erts/emulator/beam/erl_threads.h b/erts/emulator/beam/erl_threads.h index ac3d0d73aa..41f64b93a4 100644 --- a/erts/emulator/beam/erl_threads.h +++ b/erts/emulator/beam/erl_threads.h @@ -430,6 +430,7 @@ ERTS_GLB_INLINE void erts_thr_exit(void *res); ERTS_GLB_INLINE void erts_thr_install_exit_handler(void (*exit_handler)(void)); ERTS_GLB_INLINE erts_tid_t erts_thr_self(void); ERTS_GLB_INLINE int erts_thr_getname(erts_tid_t tid, char *buf, size_t len); +ERTS_GLB_INLINE void erts_thr_setname(char *buf); ERTS_GLB_INLINE int erts_equal_tids(erts_tid_t x, erts_tid_t y); ERTS_GLB_INLINE void erts_mtx_init(erts_mtx_t *mtx, const char *name, @@ -1623,6 +1624,13 @@ erts_thr_getname(erts_tid_t tid, char *buf, size_t len) return ethr_getname(tid, buf, len); } +ERTS_GLB_INLINE void +erts_thr_setname(char *buf) +{ + if (strlen(buf) > ETHR_THR_NAME_MAX) + erts_thr_fatal_error(EINVAL, "too long thread name"); + ethr_setname(buf); +} ERTS_GLB_INLINE int erts_equal_tids(erts_tid_t x, erts_tid_t y) diff --git a/erts/emulator/beam/erl_trace.c b/erts/emulator/beam/erl_trace.c index 82d4cf728c..6d572ab9f3 100644 --- a/erts/emulator/beam/erl_trace.c +++ b/erts/emulator/beam/erl_trace.c @@ -2473,7 +2473,7 @@ init_sys_msg_dispatcher(void) { erts_thr_opts_t thr_opts = ERTS_THR_OPTS_DEFAULT_INITER; thr_opts.detached = 1; - thr_opts.name = "sys_msg_dispatcher"; + thr_opts.name = "erts_smsg_disp"; init_smq_element_alloc(); sys_message_queue = NULL; sys_message_queue_end = NULL; diff --git a/erts/emulator/sys/unix/sys.c b/erts/emulator/sys/unix/sys.c index 7ff8425d52..4af776904f 100644 --- a/erts/emulator/sys/unix/sys.c +++ b/erts/emulator/sys/unix/sys.c @@ -1057,7 +1057,7 @@ init_smp_sig_notify(void) { erts_thr_opts_t thr_opts = ERTS_THR_OPTS_DEFAULT_INITER; thr_opts.detached = 1; - thr_opts.name = "sys_sig_dispatcher"; + thr_opts.name = "erts_ssig_disp"; if (pipe(sig_notify_fds) < 0) { erts_exit(ERTS_ABORT_EXIT, @@ -1107,9 +1107,10 @@ erts_sys_main_thread(void) #else /* Become signal receiver thread... */ #ifdef ERTS_ENABLE_LOCK_CHECK - erts_lc_set_thread_name("signal_receiver"); + erts_lc_set_thread_name("main"); #endif #endif + erts_thr_setname("erts_main"); smp_sig_notify(0); /* Notify initialized */ /* Wait for a signal to arrive... */ diff --git a/erts/include/internal/ethread.h b/erts/include/internal/ethread.h index bba35ef02a..b589a54d39 100644 --- a/erts/include/internal/ethread.h +++ b/erts/include/internal/ethread.h @@ -497,10 +497,11 @@ typedef struct { typedef struct { int detached; /* boolean (default false) */ int suggested_stack_size; /* kilo words (default sys dependent) */ - char *name; /* max 14 char long (default no-name) */ + char *name; /* max 15 char long (default no-name) */ } ethr_thr_opts; #define ETHR_THR_OPTS_DEFAULT_INITER {0, -1, NULL} +#define ETHR_THR_NAME_MAX 15 #if !defined(ETHR_TRY_INLINE_FUNCS) || defined(ETHR_AUX_IMPL__) # define ETHR_NEED_SPINLOCK_PROTOTYPES__ diff --git a/erts/lib_src/pthread/ethread.c b/erts/lib_src/pthread/ethread.c index b17aa3a17f..bfacaa27d0 100644 --- a/erts/lib_src/pthread/ethread.c +++ b/erts/lib_src/pthread/ethread.c @@ -81,7 +81,7 @@ typedef struct { void *prep_func_res; size_t stacksize; char *name; - char name_buff[32]; + char name_buff[ETHR_THR_NAME_MAX + 1]; } ethr_thr_wrap_data__; static void *thr_wrapper(void *vtwd) @@ -334,21 +334,9 @@ ethr_thr_create(ethr_tid *tid, void * (*func)(void *), void *arg, twd.stacksize = 0; if (opts && opts->name) { - size_t nlen = sizeof(twd.name_buff); -#ifdef __HAIKU__ - if (nlen > B_OS_NAME_LENGTH) - nlen = B_OS_NAME_LENGTH; -#else - /* - * Length of 16 is known to work. At least pthread_setname_np() - * is documented to fail on too long name string, but documentation - * does not say what the limit is. Do not have the time to dig - * further into that now... - */ - if (nlen > 16) - nlen = 16; -#endif - snprintf(twd.name_buff, nlen, "%s", opts->name); + if (strlen(opts->name) >= sizeof(twd.name_buff)) + return EINVAL; + strcpy(twd.name_buff, opts->name); twd.name = twd.name_buff; } else twd.name = NULL; @@ -506,6 +494,8 @@ ethr_getname(ethr_tid tid, char *buf, size_t len) void ethr_setname(char *name) { + if (strlen(name) > ETHR_THR_NAME_MAX) + return; #if defined(ETHR_HAVE_PTHREAD_SETNAME_NP_2) pthread_setname_np(ethr_self(), name); #elif defined(ETHR_HAVE_PTHREAD_SET_NAME_NP_2) diff --git a/erts/test/ethread_SUITE.erl b/erts/test/ethread_SUITE.erl index 19f738c572..e5a5e3dba2 100644 --- a/erts/test/ethread_SUITE.erl +++ b/erts/test/ethread_SUITE.erl @@ -43,7 +43,8 @@ rwspinlock/1, rwmutex/1, atomic/1, - dw_atomic_massage/1]). + dw_atomic_massage/1, + thread_name/1]). -include_lib("common_test/include/ct.hrl"). @@ -65,7 +66,8 @@ all() -> rwspinlock, rwmutex, atomic, - dw_atomic_massage]. + dw_atomic_massage, + thread_name]. init_per_testcase(Case, Config) -> case inet:gethostname() of @@ -158,6 +160,10 @@ atomic(Config) -> dw_atomic_massage(Config) -> run_case(Config, "dw_atomic_massage", ""). +%% Tests thread names. +thread_name(Config) -> + run_case(Config, "thread_name", ""). + %% %% %% Auxiliary functions diff --git a/erts/test/ethread_SUITE_data/ethread_tests.c b/erts/test/ethread_SUITE_data/ethread_tests.c index 048acd6a95..fcff68a294 100644 --- a/erts/test/ethread_SUITE_data/ethread_tests.c +++ b/erts/test/ethread_SUITE_data/ethread_tests.c @@ -41,7 +41,7 @@ #define PRINT_VA_LIST(FRMT) \ do { \ - if (FRMT && FRMT != '\0') { \ + if (FRMT && *(FRMT) != '\0') { \ va_list args; \ va_start(args, FRMT); \ vfprintf(stderr, FRMT, args); \ @@ -1757,6 +1757,7 @@ at_dw_thr(void *vval) break; } } + return NULL; } static void @@ -1783,6 +1784,120 @@ dw_atomic_massage_test(void) } } +static ethr_mutex thread_name_mutex; +static ethr_cond thread_name_cond; +static int thread_name_state; + +static void * +thread_name_thread(void *my_tid) +{ + int res; + + ethr_mutex_lock(&thread_name_mutex); + thread_name_state = 1; + while (thread_name_state == 1) { + res = ethr_cond_wait(&thread_name_cond, &thread_name_mutex); + ASSERT(res == 0); + } + ethr_mutex_unlock(&thread_name_mutex); + return NULL; +} + +static void +thread_name(void) +{ + static const ethr_thr_opts default_thr_opts = ETHR_THR_OPTS_DEFAULT_INITER; + ethr_tid tid; + ethr_thr_opts thr_opts; + int res; + char buf[ETHR_THR_NAME_MAX + 1]; + + res = ethr_mutex_init(&thread_name_mutex); + ASSERT(res == 0); + res = ethr_cond_init(&thread_name_cond); + ASSERT(res == 0); + + if (ethr_getname(ethr_self(), buf, sizeof(buf)) == ENOSYS) { + skip("thread names are not supported"); + return; + } + + /* create a thread with the minimum name length */ + thread_name_state = 0; + + memcpy(&thr_opts, &default_thr_opts, sizeof(thr_opts)); + thr_opts.name = ""; + res = ethr_thr_create(&tid, thread_name_thread, NULL, &thr_opts); + ASSERT(res == 0); + + memset(buf, 0xFF, sizeof(buf)); + res = ethr_getname(tid, buf, sizeof(buf)); + ASSERT(res == 0); + + res = strcmp(buf, ""); + ASSERT(res == 0); + + ethr_mutex_lock(&thread_name_mutex); + thread_name_state = 0; + ethr_cond_signal(&thread_name_cond); + ethr_mutex_unlock(&thread_name_mutex); + + res = ethr_thr_join(tid, NULL); + ASSERT(res == 0); + + /* create a thread with a middling name length */ + thread_name_state = 0; + + memcpy(&thr_opts, &default_thr_opts, sizeof(thr_opts)); + thr_opts.name = "123456789"; + res = ethr_thr_create(&tid, thread_name_thread, NULL, &thr_opts); + ASSERT(res == 0); + + memset(buf, 0xFF, sizeof(buf)); + res = ethr_getname(tid, buf, sizeof(buf)); + ASSERT(res == 0); + + res = strcmp(buf, "123456789"); + ASSERT(res == 0); + + ethr_mutex_lock(&thread_name_mutex); + thread_name_state = 0; + ethr_cond_signal(&thread_name_cond); + ethr_mutex_unlock(&thread_name_mutex); + + res = ethr_thr_join(tid, NULL); + ASSERT(res == 0); + + /* create a thread with the maximum name length */ + thread_name_state = 0; + + memcpy(&thr_opts, &default_thr_opts, sizeof(thr_opts)); + thr_opts.name = "123456789012345"; + res = ethr_thr_create(&tid, thread_name_thread, NULL, &thr_opts); + ASSERT(res == 0); + + memset(buf, 0xFF, sizeof(buf)); + res = ethr_getname(tid, buf, sizeof(buf)); + ASSERT(res == 0); + + res = strcmp(buf, "123456789012345"); + ASSERT(res == 0); + + ethr_mutex_lock(&thread_name_mutex); + thread_name_state = 0; + ethr_cond_signal(&thread_name_cond); + ethr_mutex_unlock(&thread_name_mutex); + + res = ethr_thr_join(tid, NULL); + ASSERT(res == 0); + + /* create a thread with an over-sized name length */ + memcpy(&thr_opts, &default_thr_opts, sizeof(thr_opts)); + thr_opts.name = "1234567890123456"; + res = ethr_thr_create(&tid, thread_name_thread, NULL, &thr_opts); + ASSERT(res == EINVAL); +} + void * at_thread(void *unused) { @@ -1958,6 +2073,8 @@ main(int argc, char *argv[]) atomic_test(); else if (strcmp(testcase, "dw_atomic_massage") == 0) dw_atomic_massage_test(); + else if (strcmp(testcase, "thread_name") == 0) + thread_name(); else skip("Test case \"%s\" not implemented yet", testcase); -- 2.35.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
