From 7dfd54cf4ac182c1166b4579b330d17d0dbfedd6 Mon Sep 17 00:00:00 2001 From: Yonggang Luo Date: Wed, 31 Aug 2022 15:15:00 +0800 Subject: [PATCH] util: Add util_call_once for optimize call to util_call_once_with_context out for hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For hot path, there is only need to a load instruction to load if initialized are true now, So the extra cost is minimal. Signed-off-by: Yonggang Luo Acked-by: Marek Olšák Reviewed-by: Chia-I Wu Part-of: --- src/util/meson.build | 1 + src/util/simple_mtx.c | 7 ++- src/util/simple_mtx.h | 12 ++---- src/util/tests/u_call_once_test.cpp | 67 +++++++++++++++++++++++++++++ src/util/u_call_once.c | 18 ++++---- src/util/u_call_once.h | 48 ++++++++++++++++++++- 6 files changed, 131 insertions(+), 22 deletions(-) create mode 100644 src/util/tests/u_call_once_test.cpp diff --git a/src/util/meson.build b/src/util/meson.build index 37fdc8b299e..1b24de89680 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -352,6 +352,7 @@ if with_tests 'tests/string_buffer_test.cpp', 'tests/timespec_test.cpp', 'tests/u_atomic_test.cpp', + 'tests/u_call_once_test.cpp', 'tests/u_debug_stack_test.cpp', 'tests/u_printf_test.cpp', 'tests/u_qsort_test.cpp', diff --git a/src/util/simple_mtx.c b/src/util/simple_mtx.c index 49f33efcaca..8b53e10f9a5 100644 --- a/src/util/simple_mtx.c +++ b/src/util/simple_mtx.c @@ -16,17 +16,16 @@ void _simple_mtx_plain_init_once(simple_mtx_t *mtx) void simple_mtx_init(simple_mtx_t *mtx, ASSERTED int type) { - const once_flag once = ONCE_FLAG_INIT; + const util_once_flag flag = UTIL_ONCE_FLAG_INIT; assert(type == mtx_plain); - mtx->initialized = false; - mtx->once = once; + mtx->flag = flag; _simple_mtx_init_with_once(mtx); } void simple_mtx_destroy(simple_mtx_t *mtx) { - if (mtx->initialized) { + if (mtx->flag.called) { mtx_destroy(&mtx->mtx); } } diff --git a/src/util/simple_mtx.h b/src/util/simple_mtx.h index 4b41e935717..acfd649b529 100644 --- a/src/util/simple_mtx.h +++ b/src/util/simple_mtx.h @@ -147,23 +147,19 @@ simple_mtx_assert_locked(simple_mtx_t *mtx) #else /* !UTIL_FUTEX_SUPPORTED */ typedef struct simple_mtx_t { - bool initialized; - once_flag once; + util_once_flag flag; mtx_t mtx; } simple_mtx_t; -#define _SIMPLE_MTX_INITIALIZER_NP { false, ONCE_FLAG_INIT } +#define _SIMPLE_MTX_INITIALIZER_NP { UTIL_ONCE_FLAG_INIT } void _simple_mtx_plain_init_once(simple_mtx_t *mtx); static inline void _simple_mtx_init_with_once(simple_mtx_t *mtx) { - if (unlikely(!mtx->initialized)) { - util_call_once_with_context(&mtx->once, mtx, - (util_call_once_callback_t)_simple_mtx_plain_init_once); - mtx->initialized = true; - } + util_call_once_data(&mtx->flag, + (util_call_once_data_func)_simple_mtx_plain_init_once, mtx); } void diff --git a/src/util/tests/u_call_once_test.cpp b/src/util/tests/u_call_once_test.cpp new file mode 100644 index 00000000000..69158624df3 --- /dev/null +++ b/src/util/tests/u_call_once_test.cpp @@ -0,0 +1,67 @@ +/* + * Copyright 2022 Yonggang Luo + * SPDX-License-Identifier: MIT + * + * Testing u_call_once.h + */ + +#include +#include +#include + +#include "c11/threads.h" +#include "util/u_atomic.h" +#include "util/u_call_once.h" + +#define NUM_DEBUG_TEST_THREAD 8 + +static void update_x(int *x) { + p_atomic_inc(x); +} + +static int xC; +static void update_x_global(void) +{ + update_x(&xC); +} + +static int test_call_once(void *_state) +{ + static int xA = 0; + { + static once_flag once = ONCE_FLAG_INIT; + for (int i = 0; i < 100; i += 1) { + util_call_once_data_slow(&once, (util_call_once_data_func)update_x, &xA); + } + } + + static int xB = 0; + { + static util_once_flag once = UTIL_ONCE_FLAG_INIT; + for (int i = 0; i < 100; i += 1) { + util_call_once_data(&once, (util_call_once_data_func)update_x, &xB); + } + } + { + static util_once_flag once = UTIL_ONCE_FLAG_INIT; + for (int i = 0; i < 100; i += 1) { + util_call_once(&once, update_x_global); + } + } + EXPECT_EQ(xA, 1); + EXPECT_EQ(xB, 1); + EXPECT_EQ(xC, 1); + return 0; +} + +TEST(UtilCallOnce, Multithread) +{ + thrd_t threads[NUM_DEBUG_TEST_THREAD]; + for (unsigned i = 0; i < NUM_DEBUG_TEST_THREAD; i++) { + thrd_create(&threads[i], test_call_once, NULL); + } + for (unsigned i = 0; i < NUM_DEBUG_TEST_THREAD; i++) { + int ret; + thrd_join(threads[i], &ret); + } +} diff --git a/src/util/u_call_once.c b/src/util/u_call_once.c index a727a02f8b9..e16a3cfeaf0 100644 --- a/src/util/u_call_once.c +++ b/src/util/u_call_once.c @@ -7,22 +7,24 @@ struct util_call_once_context_t { - void *context; - util_call_once_callback_t callback; + const void *data; + util_call_once_data_func func; }; static thread_local struct util_call_once_context_t call_once_context; -static void util_call_once_with_context_callback(void) +static void +util_call_once_data_slow_once(void) { struct util_call_once_context_t *once_context = &call_once_context; - once_context->callback(once_context->context); + once_context->func(once_context->data); } -void util_call_once_with_context(once_flag *once, void *context, util_call_once_callback_t callback) +void +util_call_once_data_slow(once_flag *once, util_call_once_data_func func, const void *data) { struct util_call_once_context_t *once_context = &call_once_context; - once_context->context = context; - once_context->callback = callback; - call_once(once, util_call_once_with_context_callback); + once_context->data = data; + once_context->func = func; + call_once(once, util_call_once_data_slow_once); } diff --git a/src/util/u_call_once.h b/src/util/u_call_once.h index 277ff721f32..04c910f7dab 100644 --- a/src/util/u_call_once.h +++ b/src/util/u_call_once.h @@ -11,14 +11,58 @@ #include #include "c11/threads.h" +#include "macros.h" +#include "u_atomic.h" #ifdef __cplusplus extern "C" { #endif -typedef void (*util_call_once_callback_t)(void *context); +/* The data can be mutable or immutable. */ +typedef void (*util_call_once_data_func)(const void *data); -void util_call_once_with_context(once_flag *once, void *context, util_call_once_callback_t callback); +struct util_once_flag { + bool called; + once_flag flag; +}; +typedef struct util_once_flag util_once_flag; + +#define UTIL_ONCE_FLAG_INIT { false, ONCE_FLAG_INIT } + +/** + * This is used to optimize the call to call_once out when the func are + * already called and finished, so when util_call_once are called in + * hot path it's only incur an extra load instruction cost. + */ +static ALWAYS_INLINE void +util_call_once(util_once_flag *flag, void (*func)(void)) +{ + if (unlikely(!p_atomic_read_relaxed(&flag->called))) { + call_once(&flag->flag, func); + p_atomic_set(&flag->called, true); + } +} + +/** + * @brief Wrapper around call_once to pass data to func + */ +void +util_call_once_data_slow(once_flag *once, util_call_once_data_func func, const void *data); + +/** + * This is used to optimize the call to util_call_once_data_slow out when + * the func function are already called and finished, + * so when util_call_once_data are called in hot path it's only incur an extra + * load instruction cost. + */ +static ALWAYS_INLINE void +util_call_once_data(util_once_flag *flag, util_call_once_data_func func, const void *data) +{ + if (unlikely(!p_atomic_read_relaxed(&flag->called))) { + util_call_once_data_slow(&(flag->flag), func, data); + p_atomic_set(&flag->called, true); + } +} #ifdef __cplusplus }