From 743a1b1037462b37da1a59921e706aee33b6453d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 16 Jan 2016 06:06:35 +0200 Subject: [PATCH] Removed unnecessary calls to atomic_add The polling statistics collection used atomic_add to increment values. This is not an optimal way to update statistical values. Moved to per thread values which are summed up when they are read. Moved the functions used to gather polling statistics to their own file and created a specific data type for statistics. --- server/core/CMakeLists.txt | 2 +- server/core/gateway.c | 4 ++ server/core/poll.c | 102 +++++++++++++++++------------- server/core/statistics.c | 121 ++++++++++++++++++++++++++++++++++++ server/core/test/testpoll.c | 2 + server/include/statistics.h | 49 +++++++++++++++ server/include/test_utils.h | 2 + 7 files changed, 239 insertions(+), 43 deletions(-) create mode 100644 server/core/statistics.c create mode 100644 server/include/statistics.h diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index e32bc05e9..4918f7fa2 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -1,4 +1,4 @@ -add_library(maxscale-common SHARED adminusers.c atomic.c buffer.c config.c dbusers.c dcb.c filter.c externcmd.c gwbitmask.c gwdirs.c gw_utils.c hashtable.c hint.c housekeeper.c load_utils.c maxscale_pcre2.c memlog.c modutil.c monitor.c poll.c random_jkiss.c resultset.c secrets.c server.c service.c session.c spinlock.c thread.c users.c utils.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc ${CMAKE_SOURCE_DIR}/utils/skygw_utils.cc) +add_library(maxscale-common SHARED adminusers.c atomic.c buffer.c config.c dbusers.c dcb.c filter.c externcmd.c gwbitmask.c gwdirs.c gw_utils.c hashtable.c hint.c housekeeper.c load_utils.c maxscale_pcre2.c memlog.c modutil.c monitor.c poll.c random_jkiss.c resultset.c secrets.c server.c service.c session.c spinlock.c thread.c users.c utils.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc ${CMAKE_SOURCE_DIR}/utils/skygw_utils.cc statistics.c) target_link_libraries(maxscale-common ${MYSQLCLIENT_LIBRARIES} ${LZMA_LINK_FLAGS} ${PCRE2_LIBRARIES} ${CURL_LIBRARIES} ssl aio pthread crypt dl crypto inih z rt m stdc++) diff --git a/server/core/gateway.c b/server/core/gateway.c index 4381b8858..35e2ac00b 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -89,6 +89,7 @@ #include #include #include +#include #define STRING_BUFFER_SIZE 1024 #define PIDFD_CLOSED -1 @@ -1917,6 +1918,9 @@ int main(int argc, char **argv) goto return_main; } + /** Initialize statistics */ + ts_stats_init(); + /* Init MaxScale poll system */ poll_init(); diff --git a/server/core/poll.c b/server/core/poll.c index a037e7d82..dde93e750 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #define PROFILE_POLL 0 @@ -156,21 +157,21 @@ static THREAD_DATA *thread_data = NULL; /*< Status of each thread */ */ static struct { - int n_read; /*< Number of read events */ - int n_write; /*< Number of write events */ - int n_error; /*< Number of error events */ - int n_hup; /*< Number of hangup events */ - int n_accept; /*< Number of accept events */ - int n_polls; /*< Number of poll cycles */ - int n_pollev; /*< Number of polls returning events */ - int n_nbpollev; /*< Number of polls returning events */ - int n_nothreads; /*< Number of times no threads are polling */ - int n_fds[MAXNFDS]; /*< Number of wakeups with particular n_fds value */ - int evq_length; /*< Event queue length */ - int evq_pending; /*< Number of pending descriptors in event queue */ - int evq_max; /*< Maximum event queue length */ - int wake_evqpending;/*< Woken from epoll_wait with pending events in queue */ - int blockingpolls; /*< Number of epoll_waits with a timeout specified */ + ts_stats_t *n_read; /*< Number of read events */ + ts_stats_t *n_write; /*< Number of write events */ + ts_stats_t *n_error; /*< Number of error events */ + ts_stats_t *n_hup; /*< Number of hangup events */ + ts_stats_t *n_accept; /*< Number of accept events */ + ts_stats_t *n_polls; /*< Number of poll cycles */ + ts_stats_t *n_pollev; /*< Number of polls returning events */ + ts_stats_t *n_nbpollev; /*< Number of polls returning events */ + ts_stats_t *n_nothreads; /*< Number of times no threads are polling */ + int n_fds[MAXNFDS]; /*< Number of wakeups with particular n_fds value */ + int evq_length; /*< Event queue length */ + int evq_pending; /*< Number of pending descriptors in event queue */ + int evq_max; /*< Maximum event queue length */ + int wake_evqpending; /*< Woken from epoll_wait with pending events in queue */ + ts_stats_t *blockingpolls; /*< Number of epoll_waits with a timeout specified */ } pollStats; #define N_QUEUE_TIMES 30 @@ -230,6 +231,22 @@ poll_init() thread_data[i].state = THREAD_STOPPED; } } + + if ((pollStats.n_read = ts_stats_alloc()) == NULL || + (pollStats.n_write = ts_stats_alloc()) == NULL || + (pollStats.n_error = ts_stats_alloc()) == NULL || + (pollStats.n_hup = ts_stats_alloc()) == NULL || + (pollStats.n_accept = ts_stats_alloc()) == NULL || + (pollStats.n_polls = ts_stats_alloc()) == NULL || + (pollStats.n_pollev = ts_stats_alloc()) == NULL || + (pollStats.n_nbpollev = ts_stats_alloc()) == NULL || + (pollStats.n_nothreads = ts_stats_alloc()) == NULL || + (pollStats.blockingpolls = ts_stats_alloc()) == NULL) + { + perror("Fatal error: Memory allocation failed."); + exit(-1); + } + #if MUTEX_EPOLL simple_mutex_init(&epoll_wait_mutex, "epoll_wait_mutex"); #endif @@ -540,6 +557,8 @@ poll_waitevents(void *arg) intptr_t thread_id = (intptr_t)arg; int poll_spins = 0; + ts_stats_set_thread_id(thread_id); + /** Add this thread to the bitmask of running polling threads */ bitmask_set(&poll_mask, thread_id); if (thread_data) @@ -567,7 +586,7 @@ poll_waitevents(void *arg) thread_data[thread_id].state = THREAD_POLLING; } - atomic_add(&pollStats.n_polls, 1); + ts_stats_add(pollStats.n_polls, 1); if ((nfds = epoll_wait(epoll_fd, events, MAX_EVENTS, 0)) == -1) { atomic_add(&n_waiting, -1); @@ -590,7 +609,7 @@ poll_waitevents(void *arg) */ else if (nfds == 0 && pollStats.evq_pending == 0 && poll_spins++ > number_poll_spins) { - atomic_add(&pollStats.blockingpolls, 1); + ts_stats_add(pollStats.blockingpolls, 1); nfds = epoll_wait(epoll_fd, events, MAX_EVENTS, @@ -608,7 +627,7 @@ poll_waitevents(void *arg) if (n_waiting == 0) { - atomic_add(&pollStats.n_nothreads, 1); + ts_stats_add(pollStats.n_nothreads, 1); } #if MUTEX_EPOLL simple_mutex_unlock(&epoll_wait_mutex); @@ -619,13 +638,13 @@ poll_waitevents(void *arg) timeout_bias = 1; if (poll_spins <= number_poll_spins + 1) { - atomic_add(&pollStats.n_nbpollev, 1); + ts_stats_add(pollStats.n_nbpollev, 1); } poll_spins = 0; MXS_DEBUG("%lu [poll_waitevents] epoll_wait found %d fds", pthread_self(), nfds); - atomic_add(&pollStats.n_pollev, 1); + ts_stats_add(pollStats.n_pollev, 1); if (thread_data) { thread_data[thread_id].n_fds = nfds; @@ -910,7 +929,7 @@ process_pollq(int thread_id) if (eno == 0) { - atomic_add(&pollStats.n_write, 1); + ts_stats_add(pollStats.n_write, 1); /** Read session id to thread's local storage */ dcb_get_ses_log_info(dcb, &mxs_log_tls.li_sesid, @@ -942,7 +961,7 @@ process_pollq(int thread_id) "Accept in fd %d", pthread_self(), dcb->fd); - atomic_add(&pollStats.n_accept, 1); + ts_stats_add(pollStats.n_accept, 1); dcb_get_ses_log_info(dcb, &mxs_log_tls.li_sesid, &mxs_log_tls.li_enabled_priorities); @@ -959,7 +978,7 @@ process_pollq(int thread_id) pthread_self(), dcb, dcb->fd); - atomic_add(&pollStats.n_read, 1); + ts_stats_add(pollStats.n_read, 1); /** Read session id to thread's local storage */ dcb_get_ses_log_info(dcb, &mxs_log_tls.li_sesid, @@ -997,7 +1016,7 @@ process_pollq(int thread_id) eno, strerror_r(eno, errbuf, sizeof(errbuf))); } - atomic_add(&pollStats.n_error, 1); + ts_stats_add(pollStats.n_error, 1); /** Read session id to thread's local storage */ dcb_get_ses_log_info(dcb, &mxs_log_tls.li_sesid, @@ -1022,7 +1041,7 @@ process_pollq(int thread_id) dcb->fd, eno, strerror_r(eno, errbuf, sizeof(errbuf))); - atomic_add(&pollStats.n_hup, 1); + ts_stats_add(pollStats.n_hup, 1); spinlock_acquire(&dcb->dcb_initlock); if ((dcb->flags & DCBF_HUNG) == 0) { @@ -1058,7 +1077,7 @@ process_pollq(int thread_id) dcb->fd, eno, strerror_r(eno, errbuf, sizeof(errbuf))); - atomic_add(&pollStats.n_hup, 1); + ts_stats_add(pollStats.n_hup, 1); spinlock_acquire(&dcb->dcb_initlock); if ((dcb->flags & DCBF_HUNG) == 0) { @@ -1209,7 +1228,6 @@ spin_reporter(void *dcb, char *desc, int value) dcb_printf((DCB *)dcb, "\t%-40s %d\n", desc, value); } - /** * Debug routine to print the polling statistics * @@ -1222,25 +1240,25 @@ dprintPollStats(DCB *dcb) dcb_printf(dcb, "\nPoll Statistics.\n\n"); dcb_printf(dcb, "No. of epoll cycles: %d\n", - pollStats.n_polls); + ts_stats_sum(pollStats.n_polls)); dcb_printf(dcb, "No. of epoll cycles with wait: %d\n", - pollStats.blockingpolls); + ts_stats_sum(pollStats.blockingpolls)); dcb_printf(dcb, "No. of epoll calls returning events: %d\n", - pollStats.n_pollev); + ts_stats_sum(pollStats.n_pollev)); dcb_printf(dcb, "No. of non-blocking calls returning events: %d\n", - pollStats.n_nbpollev); + ts_stats_sum(pollStats.n_nbpollev)); dcb_printf(dcb, "No. of read events: %d\n", - pollStats.n_read); + ts_stats_sum(pollStats.n_read)); dcb_printf(dcb, "No. of write events: %d\n", - pollStats.n_write); + ts_stats_sum(pollStats.n_write)); dcb_printf(dcb, "No. of error events: %d\n", - pollStats.n_error); + ts_stats_sum(pollStats.n_error)); dcb_printf(dcb, "No. of hangup events: %d\n", - pollStats.n_hup); + ts_stats_sum(pollStats.n_hup)); dcb_printf(dcb, "No. of accept events: %d\n", - pollStats.n_accept); + ts_stats_sum(pollStats.n_accept)); dcb_printf(dcb, "No. of times no threads polling: %d\n", - pollStats.n_nothreads); + ts_stats_sum(pollStats.n_nothreads)); dcb_printf(dcb, "Current event queue length: %d\n", pollStats.evq_length); dcb_printf(dcb, "Maximum event queue length: %d\n", @@ -1796,15 +1814,15 @@ poll_get_stat(POLL_STAT stat) switch (stat) { case POLL_STAT_READ: - return pollStats.n_read; + return ts_stats_sum(pollStats.n_read); case POLL_STAT_WRITE: - return pollStats.n_write; + return ts_stats_sum(pollStats.n_write); case POLL_STAT_ERROR: - return pollStats.n_error; + return ts_stats_sum(pollStats.n_error); case POLL_STAT_HANGUP: - return pollStats.n_hup; + return ts_stats_sum(pollStats.n_hup); case POLL_STAT_ACCEPT: - return pollStats.n_accept; + return ts_stats_sum(pollStats.n_accept); case POLL_STAT_EVQ_LEN: return pollStats.evq_length; case POLL_STAT_EVQ_PENDING: diff --git a/server/core/statistics.c b/server/core/statistics.c new file mode 100644 index 000000000..78bf04784 --- /dev/null +++ b/server/core/statistics.c @@ -0,0 +1,121 @@ +/* + * This file is distributed as part of the MariaDB Corporation MaxScale. It is free + * software: you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation, + * version 2. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 51 + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright MariaDB Corporation Ab 2016 + */ + +#include +#include +#include +#include + +thread_local int current_thread_id = 0; + +static int thread_count = 0; +static bool initialized = false; + +/** + * Initialize the statistics gathering + */ +void ts_stats_init() +{ + ss_dassert(!initialized); + thread_count = config_threadcount(); + initialized = true; +} + +/** + * End the statistics gathering + */ +void ts_stats_end() +{ + ss_dassert(initialized); +} + +/** + * Create a new statistics object + * + * @return New stats_t object or NULL if memory allocation failed + */ +ts_stats_t ts_stats_alloc() +{ + ss_dassert(initialized); + return calloc(thread_count, sizeof(int)); +} + +/** + * Free a statistics object + * + * @param stats Stats to free + */ +void ts_stats_free(ts_stats_t stats) +{ + ss_dassert(initialized); + free(stats); +} + +/** + * Set the current thread id + * + * This should only be called only once by each thread. + * @param id Thread id + */ +void ts_stats_set_thread_id(int id) +{ + ss_dassert(initialized); + current_thread_id = id; +} + +/** + * Add @c value to @c stats + * + * @param stats Statistics to add to + * @param value Value to add + */ +void ts_stats_add(ts_stats_t stats, int value) +{ + ss_dassert(initialized); + ((int*)stats)[current_thread_id] += value; +} + +/** + * Assign a value to the statistics + * + * This sets the value for the current thread only. + * @param stats Statistics to set + * @param value Value to set to + */ +void ts_stats_set(ts_stats_t stats, int value) +{ + ss_dassert(initialized); + ((int*)stats)[current_thread_id] = value; +} + +/** + * Read the total value of the statistics object + * + * @param stats Statistics to read + * @return Value of statistics + */ +int ts_stats_sum(ts_stats_t stats) +{ + ss_dassert(initialized); + int sum = 0; + for (int i = 0; i < thread_count; i++) + { + sum += ((int*)stats)[i]; + } + return sum; +} diff --git a/server/core/test/testpoll.c b/server/core/test/testpoll.c index 40ed00f0c..4c366d3ce 100644 --- a/server/core/test/testpoll.c +++ b/server/core/test/testpoll.c @@ -40,6 +40,7 @@ #include #include #include +#include /** * test1 Allocate a service and do lots of other things @@ -56,6 +57,7 @@ int result; /* Poll tests */ ss_dfprintf(stderr, "testpoll : Initialise the polling system."); + init_test_env(NULL); poll_init(); ss_dfprintf(stderr, "\t..done\nAdd a DCB"); dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); diff --git a/server/include/statistics.h b/server/include/statistics.h new file mode 100644 index 000000000..cad929ea0 --- /dev/null +++ b/server/include/statistics.h @@ -0,0 +1,49 @@ +#ifndef _STATISTICS_HG +#define _STATISTICS_HG +/* + * This file is distributed as part of the MariaDB Corporation MaxScale. It is free + * software: you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation, + * version 2. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 51 + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright MariaDB Corporation Ab 2016 + */ + +/** + * @file statistics.h - Lock-free statistics gathering + * + * @verbatim + * Revision History + * + * Date Who Description + * 21/01/16 Markus Makela Initial implementation + * @endverbatim + */ + +typedef void* ts_stats_t; + +/** stats_init should be called only once */ +void ts_stats_init(); + +/** No-op for now */ +void ts_stats_end(); + +/** Every thread should call set_current_thread_id only once */ +void ts_stats_set_thread_id(int id); + +ts_stats_t ts_stats_alloc(); +void ts_stats_free(ts_stats_t stats); +void ts_stats_add(ts_stats_t stats, int value); +void ts_stats_set(ts_stats_t stats, int value); +int ts_stats_sum(ts_stats_t stats); + +#endif diff --git a/server/include/test_utils.h b/server/include/test_utils.h index f1dea6f6b..f94f60e00 100644 --- a/server/include/test_utils.h +++ b/server/include/test_utils.h @@ -5,6 +5,7 @@ #include #include #include +#include void init_test_env(char *path) { @@ -12,6 +13,7 @@ void init_test_env(char *path) const char* logdir = path ? path : TEST_LOG_DIR; + ts_stats_init(); mxs_log_init(NULL, logdir, MXS_LOG_TARGET_DEFAULT); poll_init(); hkinit();