This is an automated email from the ASF dual-hosted git repository. kgiusti pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
The following commit(s) were added to refs/heads/main by this push: new 10a0a56 DISPATCH-2238: optimization of core action lock use. 10a0a56 is described below commit 10a0a56db2b83fef6271d9413d36b8efd240f071 Author: Kenneth Giusti <kgiu...@apache.org> AuthorDate: Mon Aug 23 20:56:47 2021 -0400 DISPATCH-2238: optimization of core action lock use. Analysis of mutex use shows that the core action list mutex is heavily contended. This patch implements a few tricks to reduce the amount of time the lock is held. This closes #1349 --- src/router_core/router_core.c | 8 +++- src/router_core/router_core_private.h | 6 ++- src/router_core/router_core_thread.c | 77 +++++++++++++++++------------------ 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c index 812c50f..fb506c6 100644 --- a/src/router_core/router_core.c +++ b/src/router_core/router_core.c @@ -447,8 +447,10 @@ void qdr_action_enqueue(qdr_core_t *core, qdr_action_t *action) { sys_mutex_lock(core->action_lock); DEQ_INSERT_TAIL(core->action_list, action); - sys_cond_signal(core->action_cond); + const bool need_wake = core->sleeping; sys_mutex_unlock(core->action_lock); + if (need_wake) + sys_cond_signal(core->action_cond); } @@ -456,8 +458,10 @@ void qdr_action_background_enqueue(qdr_core_t *core, qdr_action_t *action) { sys_mutex_lock(core->action_lock); DEQ_INSERT_TAIL(core->action_list_background, action); - sys_cond_signal(core->action_cond); + const bool need_wake = core->sleeping; sys_mutex_unlock(core->action_lock); + if (need_wake) + sys_cond_signal(core->action_cond); } diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h index c08753a..9754465 100644 --- a/src/router_core/router_core_private.h +++ b/src/router_core/router_core_private.h @@ -829,11 +829,13 @@ struct qdr_core_t { qd_log_source_t *log; qd_log_source_t *agent_log; sys_thread_t *thread; - bool running; - qdr_action_list_t action_list; + qdr_action_list_t action_list_background; /// Actions processed only when the action_list is empty + qdr_action_list_t action_list; sys_cond_t *action_cond; sys_mutex_t *action_lock; + bool running; + bool sleeping; sys_mutex_t *work_lock; qdr_core_timer_list_t scheduled_timers; diff --git a/src/router_core/router_core_thread.c b/src/router_core/router_core_thread.c index a3688bd..c4448c7 100644 --- a/src/router_core/router_core_thread.c +++ b/src/router_core/router_core_thread.c @@ -174,37 +174,11 @@ void qdr_adaptors_finalize(qdr_core_t *core) } -/* - * router_core_process_background_action_LH - * - * Process up to one available background action. - * Return true iff an action was processed. - */ -static bool router_core_process_background_action_LH(qdr_core_t *core) -{ - qdr_action_t *action = DEQ_HEAD(core->action_list_background); - - if (!!action) { - DEQ_REMOVE_HEAD(core->action_list_background); - sys_mutex_unlock(core->action_lock); - if (action->label) - qd_log(core->log, QD_LOG_TRACE, "Core background action '%s'%s", action->label, core->running ? "" : " (discard)"); - action->action_handler(core, action, !core->running); - sys_mutex_lock(core->action_lock); - - free_qdr_action_t(action); - return true; - } - - return false; -} - - void *router_core_thread(void *arg) { qdr_core_t *core = (qdr_core_t*) arg; - qdr_action_list_t action_list; - qdr_action_t *action; + qdr_action_list_t action_list = DEQ_EMPTY; + qdr_action_t *bg_action = 0; qd_log(core->log, QD_LOG_INFO, "Router Core thread running. %s/%s", core->router_area, core->router_id); while (core->running) { @@ -213,25 +187,48 @@ void *router_core_thread(void *arg) // sys_mutex_lock(core->action_lock); - // - // Block on the condition variable when there is no action to do - // - while (core->running && DEQ_IS_EMPTY(core->action_list)) { - if (!router_core_process_background_action_LH(core)) - sys_cond_wait(core->action_cond, core->action_lock); + for (;;) { + if (!DEQ_IS_EMPTY(core->action_list)) { + DEQ_MOVE(core->action_list, action_list); + break; + } + + // no pending actions so process one background action if present + // + bg_action = DEQ_HEAD(core->action_list_background); + if (bg_action) { + DEQ_REMOVE_HEAD(core->action_list_background); + break; + } + + if (!core->running) + break; + + // + // Block on the condition variable when there is no action to do + // + core->sleeping = true; + sys_cond_wait(core->action_cond, core->action_lock); + core->sleeping = false; } - // - // Move the entire action list to a private list so we can process it without - // holding the lock - // - DEQ_MOVE(core->action_list, action_list); sys_mutex_unlock(core->action_lock); + // bg_action is set only when there are no other actions pending + // + if (bg_action) { + if (bg_action->label) + qd_log(core->log, QD_LOG_TRACE, "Core background action '%s'%s", bg_action->label, core->running ? "" : " (discard)"); + bg_action->action_handler(core, bg_action, !core->running); + free_qdr_action_t(bg_action); + bg_action = 0; + continue; + } + // // Process and free all of the action items in the list // - action = DEQ_HEAD(action_list); + qdr_action_t *action = DEQ_HEAD(action_list); while (action) { DEQ_REMOVE_HEAD(action_list); if (action->label) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org