You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kg...@apache.org on 2021/08/25 19:04:18 UTC
[qpid-dispatch] branch main updated: DISPATCH-2238: optimization of
core action lock use.
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 <kg...@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-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org