From ce4e3608d0a77b298c7332d2e6310e8d55c7c612 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 2 Nov 2009 13:51:18 -0500 Subject: [PATCH 01/68] make sure luidgen picks up the nobug cflags --- src/tool/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tool/Makefile.am b/src/tool/Makefile.am index 2bf1acf99..bacd925d8 100644 --- a/src/tool/Makefile.am +++ b/src/tool/Makefile.am @@ -18,7 +18,7 @@ lumitool_srcdir = $(top_srcdir)/src/tool noinst_PROGRAMS += luidgen -luidgen_CFLAGS = $(CFLAGS) -std=gnu99 -Wall -Werror +luidgen_CFLAGS = $(CFLAGS) $(NOBUGMT_LUMIERA_CFLAGS) -std=gnu99 -Wall -Werror luidgen_CPPFLAGS = -I$(top_srcdir)/src/ luidgen_LDADD = liblumiera.la $(NOBUGMT_LUMIERA_LIBS) -lboost_regex-mt -lboost_program_options-mt luidgen_SOURCES = $(lumitool_srcdir)/luidgen.c From 046dab0ca3729dd0a4aafae7378c1e20ad38a152 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 14 Nov 2009 22:19:04 -0500 Subject: [PATCH 02/68] Add missing nobugmt ld flags to test_threads --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index c33287932..765339aea 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -118,7 +118,7 @@ test_interfaces_LDADD = \ check_PROGRAMS += test-threads test_threads_SOURCES = $(tests_srcdir)/backend/test-threads.c test_threads_CPPFLAGS = $(AM_CPPFLAGS) -std=gnu99 -Wall -Werror -I$(top_srcdir)/src/ -test_threads_CFLAGS = $(AM_CFLAGS) $(PTHREAD_CFLAGS) -test_threads_LDADD = liblumierabackend.la liblumiera.la -lnobugmt -lpthread -ldl -lm liblumieracommon.la liblumieraproc.la -ldl -lboost_program_options-mt -lboost_regex-mt +test_threads_CFLAGS = $(AM_CFLAGS) $(NOBUGMT_LUMIERA_CFLAGS) +test_threads_LDADD = $(NOBUGMT_LUMIERA_LIBS) liblumierabackend.la liblumiera.la -ldl -lm liblumieracommon.la liblumieraproc.la -ldl -lboost_program_options-mt -lboost_regex-mt TESTS = $(tests_srcdir)/test.sh From 4bac65df72218e0abe2b0b92480e8c7961c63501 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 26 Nov 2009 10:13:32 -0500 Subject: [PATCH 03/68] copied test.sh from nobug 1147947 --- tests/test.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test.sh b/tests/test.sh index 89de9ae54..951d2fcea 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -30,7 +30,6 @@ NOBUG_LOGREGEX='^\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: \(TRACE\|INFO\|NOTICE\|WARNI arg0="$0" srcdir="$(dirname "$arg0")" -vgsuppression_mangle='/^\(\(==\)\|\(\*\*\)[0-9]*\(==\)\|\(\*\*\)\)\|\(\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: ECHO:\)/d;' ulimit -S -t 5 -v 524288 valgrind="" @@ -45,10 +44,10 @@ else if [[ -x ".libs/vgsuppression" ]]; then ./libtool --mode=execute valgrind --leak-check=yes --show-reachable=yes -q --gen-suppressions=all vgsuppression 2>&1 \ - | sed -e "$vgsuppression_mangle" >vgsuppression.supp + | awk '/^{/ {i = 1;} /^}/ {i = 0; print $0;} {if (i == 1) print $0;}' >vgsuppression.supp else valgrind --leak-check=yes --show-reachable=yes -q --gen-suppressions=all ./vgsuppression 2>&1 \ - | sed -e "$vgsuppression_mangle" >vgsuppression.supp + | awk '/^{/ {i = 1;} /^}/ {i = 0; print $0;} {if (i == 1) print $0;}' >vgsuppression.supp fi fi valgrind="$(which valgrind) --leak-check=yes --show-reachable=no --suppressions=vgsuppression.supp -q $VALGRINDFLAGS" @@ -120,6 +119,11 @@ function TEST() rm -f ,expect_stderr expect_return=0 + local valgrind="$valgrind" + if [ "$VALGRINDFLAGS" = 'DISABLE' ]; then + valgrind= + fi + while read -r line; do cmd="${line%%:*}" arg="${line#*:}" @@ -185,7 +189,7 @@ function TEST() if declare -F | grep $TESTBIN >&/dev/null; then CALL= elif test -x $TESTBIN; then - CALL="env $TESTBIN_PREFIX" + CALL="env $TESTBIN_PREFIX $valgrind" else CALL='-' echo -n >,stdout @@ -302,9 +306,9 @@ function TESTING() echo -e "\n#### $1" >>,testlog if [[ -x ".libs/$2" ]]; then - TESTBIN_PREFIX="./libtool --mode=execute $valgrind" + TESTBIN_PREFIX="./libtool --mode=execute" else - TESTBIN_PREFIX="$valgrind" + TESTBIN_PREFIX= fi TESTBIN="$2" } From 00eb4eda46a6bfde81e69c985d2269b4e513f9e5 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sun, 22 Nov 2009 19:55:08 -0500 Subject: [PATCH 04/68] partial code skeleton --- src/backend/Makefile.am | 2 + src/backend/threadpool.c | 243 ++++++++++++++++++++++++++++++++ src/backend/threadpool.h | 109 ++++++++++++++ src/backend/threads.c | 38 ++++- src/backend/threads.h | 37 ++++- tests/50threadpool.tests | 5 + tests/backend/test-threadpool.c | 53 +++++++ tests/library/test-threadpool.c | 53 +++++++ 8 files changed, 535 insertions(+), 5 deletions(-) create mode 100644 src/backend/threadpool.c create mode 100644 src/backend/threadpool.h create mode 100644 tests/50threadpool.tests create mode 100644 tests/backend/test-threadpool.c create mode 100644 tests/library/test-threadpool.c diff --git a/src/backend/Makefile.am b/src/backend/Makefile.am index 5729d910b..a1f85788c 100644 --- a/src/backend/Makefile.am +++ b/src/backend/Makefile.am @@ -26,6 +26,7 @@ liblumierabackend_la_SOURCES = \ $(liblumierabackend_la_srcdir)/mediaaccessfacade.cpp \ $(liblumierabackend_la_srcdir)/backend.c \ $(liblumierabackend_la_srcdir)/threads.c \ + $(liblumierabackend_la_srcdir)/threadpool.c \ $(liblumierabackend_la_srcdir)/file.c \ $(liblumierabackend_la_srcdir)/filehandle.c \ $(liblumierabackend_la_srcdir)/filedescriptor.c \ @@ -41,6 +42,7 @@ liblumierabackend_la_SOURCES = \ noinst_HEADERS += \ $(liblumierabackend_la_srcdir)/backend.h \ $(liblumierabackend_la_srcdir)/threads.h \ + $(liblumierabackend_la_srcdir)/threadpool.h \ $(liblumierabackend_la_srcdir)/file.h \ $(liblumierabackend_la_srcdir)/filehandle.h \ $(liblumierabackend_la_srcdir)/filedescriptor.h \ diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c new file mode 100644 index 000000000..27729895b --- /dev/null +++ b/src/backend/threadpool.c @@ -0,0 +1,243 @@ +/* + threadpool.c - Manage pools of threads + + Copyright (C) Lumiera.org + 2009, Michael Ploujnikov + + This program 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; either version 2 of the + License, or (at your option) any later version. + + 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., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +//TODO: Support library includes// + +#include "include/logging.h" +#include "lib/safeclib.h" + +//TODO: Lumiera header includes// +#include "backend/threadpool.h" + +//TODO: internal/static forward declarations// +/** + * Find a suitable thread pool for the given thread type. + */ +static +LumieraThreadpool +lumiera_threadpool_findpool(enum lumiera_thread_class kind); + +typedef struct lumiera_threadpoolmanager_struct lumiera_threadpoolmanager; +typedef lumiera_threadpoolmanager* LumieraThreadpoolmanager; + +struct lumiera_threadpoolmanager_struct +{ + llist pools; +}; + +/** + * Create the singleton threadpool manager + */ +static +void +lumiera_threadpoolmanager_new(void); + +/** + * Delete/remove the singleton threadpool manager + */ +static +void +lumiera_threadpoolmanager_delete(void); + +// singleton threadpool manager instance +static LumieraThreadpoolmanager lumiera_tpmanager; + +typedef struct lumiera_threadpool_struct lumiera_threadpool; +typedef lumiera_threadpool* LumieraThreadpool; + +enum lumiera_threadpool_type + { + // TODO: the following types need to be more thought out: + /** the default kind of threadpool **/ + LUMIERA_DEFAULT_THREADPOOL, + /** a threadpool for special threads **/ + LUMIERA_SPECIAL_THREADPOOL + }; + +struct lumiera_threadpool_struct +{ + /* a list of threadpools for a threadpool manager */ + llist node; + + enum lumiera_threadpool_type type; + + llist threads; +}; + +/** + * Create a thread pool. + */ +static +LumieraThreadpool +lumiera_threadpool_new(enum lumiera_threadpool_type type); + +/** + * Delete/remove a threadpool. + */ +static +void +lumiera_threadpool_delete(LumieraThreadpool threadpool); + +//TODO: System includes// +#include + +/** + * @file + * + */ + +//NOBUG_DEFINE_FLAG_PARENT (threadpool, lumiera); /*TODO insert a suitable/better parent flag here */ + + +//code goes here// + +void* pool_thread_loop(void * arg) +{ + (void) arg; + while (1) + { + ; + } + return arg; +} + +static pthread_once_t attr_once = PTHREAD_ONCE_INIT; +static pthread_attr_t attrs; + +static void thread_attr_init (void) +{ + pthread_attr_init (&attrs); + pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED); + //cancel ... +} + +LumieraThread +lumiera_thread_new (enum lumiera_thread_class kind, + LumieraReccondition finished, + const char* purpose, + struct nobug_flag* flag) +{ + (void) kind; + (void) purpose; + (void) flag; + + if (attr_once == PTHREAD_ONCE_INIT) + pthread_once (&attr_once, thread_attr_init); + + LumieraThread thread = lumiera_malloc (sizeof(*thread)); + + thread->finished = finished; + + pthread_t dummy; + printf("creating a thread\n"); + int error = pthread_create (&dummy, &attrs, &pool_thread_loop, thread); + + if (error) return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? + return thread; +} + +LumieraThreadpool +lumiera_threadpool_new(enum lumiera_threadpool_type type) +{ + LumieraThreadpool self = lumiera_malloc (sizeof (*self)); + self->type = type; + + /* we start with no threads in the list */ + llist_init (&self->threads); + + return self; +} + +LumieraThreadpoolmanager lumiera_tpmanager = NULL; + +void +lumiera_threadpoolmanager_new() +{ + REQUIRE (!lumiera_tpmanager, "Threadpool manager already initialized"); + lumiera_tpmanager = lumiera_malloc (sizeof (lumiera_threadpoolmanager)); + llist_init (&lumiera_tpmanager->pools); + + // create a default threadpool + LumieraThreadpool default_tp = lumiera_threadpool_new(LUMIERA_DEFAULT_THREADPOOL); + llist_insert_head (&lumiera_tpmanager->pools, &default_tp->node); +}; + +LumieraThread +lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, + const char* purpose, + struct nobug_flag* flag) +{ + LumieraThreadpool tp = + lumiera_threadpool_findpool(enum lumiera_thread_class kind); + + if (!tp) + { + // ERROR: could not find a suitable threadpool or there are no threadpools + return (LumieraThreadpool)0; + } + + // TODO: either get a thread from the pool or create a new one +} + +LumieraThreadpool +lumiera_threadpool_findpool(enum lumiera_thread_class thread_kind) +{ + if (!lumiera_tpmanager) + { + // create a threadpool manager if it doesn't exist yet + lumiera_threadpoolmanager_new(); + } + if (!lumiera_tpmanager) + { + // ERROR: if the threadpool manager still doesn't exist, something went wrong + return (LumieraThreadpool)0; + } + else + { + // for now, ignore the thread all together and just return the first threadpool + (void) thread_kind; + // BUG: what if there are no pools in the list??? + // where/when should I check for that case? + return (LumieraThreadpool)(llist_head (&lumiera_tpmanager->pools)); + } +} + +void +lumiera_threadpoolmanager_new(void) +{ + REQUIRE (!lumiera_tpmanager, "Threadpool manager already initialized"); + + lumiera_tpmanager = lumiera_malloc (sizeof (lumiera_tpmanager)); + llist_init (&lumiera_tpmanager->pools); + + // start with just one default threadpool in the list + LumieraThreadpool tp = + lumiera_threadpool_new(LUMIERA_DEFAULT_THREADPOOL); + llist_insert_head (&lumiera_tp->pools, &tp->node); +} + +/* +// Local Variables: +// mode: C +// c-file-style: "gnu" +// indent-tabs-mode: nil +// End: +*/ diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h new file mode 100644 index 000000000..6d11cb093 --- /dev/null +++ b/src/backend/threadpool.h @@ -0,0 +1,109 @@ +/* + threadpool.h - Manage pools of threads + + Copyright (C) Lumiera.org + 2009, Michael Ploujnikov + + This program 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; either version 2 of the + License, or (at your option) any later version. + + 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., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#ifndef LUMIERA_THREADPOOL_H +#define LUMIERA_THREADPOOL_H + +//TODO: Support library includes// +#include "lib/reccondition.h" +#include "lib/llist.h" + +//TODO: Forward declarations// + + +//TODO: Lumiera header includes// +#include "threads.h" + +//TODO: System includes// +#include + + +/** + * @file + * + */ + +//TODO: declarations go here// + +typedef struct lumiera_threadpool_struct lumiera_threadpool; +typedef lumiera_threadpool* LumieraThreadpool; + +enum lumiera_threadpool_type + { + // TODO: the following types need to be more thought out: + /** the default kind of threadpool **/ + LUMIERA_DEFAULT_THREADPOOL, + /** a threadpool for special threads **/ + LUMIERA_SPECIAL_THREADPOOL + }; + +struct lumiera_threadpool_struct +{ + /* a list of threadpools for a threadpool manager */ + llist node; + + enum lumiera_threadpool_type type; + + llist threads; +}; + + + +/** + * Create a thread pool. + */ +LumieraThreadpool +lumiera_threadpool_new(enum lumiera_threadpool_type type); + +/** + * Delete/remove a threadpool. + */ +void +lumiera_threadpool_delete(LumieraThreadpool threadpool); + + +/** + * Acquire a thread from a threadpool. + * This may either pick a thread from the list or create a new one when the list is empty. + * This function doesn't need to be accessible outside of the threadpool implementation. + */ +LumieraThread +lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, + const char* purpose, + struct nobug_flag* flag); + +/** + * Release a thread + * This ends up putting a (parked/idle) thread back on the list of a threadpool. + * This function doesn't need to be accessible outside of the threadpool implementation. + */ +void +lumiera_threadpool_release_thread(LumieraThread thread); + + +#endif +/* +// Local Variables: +// mode: C +// c-file-style: "gnu" +// indent-tabs-mode: nil +// End: +*/ diff --git a/src/backend/threads.c b/src/backend/threads.c index 67f2d4aa4..e181b6902 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -50,7 +50,7 @@ struct lumiera_thread_mockup LumieraReccondition finished; }; - +#if 0 static void* pthread_runner (void* thread) { pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); @@ -79,8 +79,9 @@ static void thread_attr_init (void) pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED); //cancel ... } +#endif - +#if 0 LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*start_routine)(void *), @@ -108,6 +109,39 @@ lumiera_thread_run (enum lumiera_thread_class kind, if (error) return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? return (LumieraThread) 1; } +#endif + +// TODO: new implementation, remove the above one +// maybe this shouldn't return LumieraThread at all +// when this is called it should have already been decided that the function +// shall run in parallel, as a thread +LumieraThread +lumiera_thread_run (enum lumiera_thread_class kind, + void (*function)(void *), + void * arg, + LumieraReccondition finished, + const char* purpose, + struct nobug_flag* flag) +{ + (void)finished; + (void)function; + (void)arg; + // ask the threadpool for a thread (it might create a new one) + LumieraThread self = lumiera_threadpool_acquire_thread(kind, purpose, flag); + + // set the function and data to be run + // lumiera_thread_set_func_data (self, start_routine, arg, purpose, flag); + + // and let it really run (signal the condition var, it waits there) + LUMIERA_RECCONDITION_SECTION(cond_sync, self->finished) + LUMIERA_RECCONDITION_SIGNAL; + + // NOTE: example only, add solid error handling! + + return self; +} + + /* diff --git a/src/backend/threads.h b/src/backend/threads.h index 9d420fa2f..d50a4fb5d 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -46,10 +46,12 @@ typedef struct lumiera_thread_struct lumiera_thread; typedef lumiera_thread* LumieraThread; + /** * Thread classes. * We define some 'classes' of threads for different purposes to abstract * priorities and other attributes. + ** TODO: rename these to LUMIERA_THREADCLASS_* */ enum lumiera_thread_class { @@ -74,6 +76,36 @@ enum lumiera_thread_class LUMIERA_THREAD_OR_NOT = 1<<16 }; +/** + * Thread state. + * These are the only states our threads can be in. + */ +typedef enum + { + LUMIERA_THREADSTATE_ERROR, + LUMIERA_THREADSTATE_IDLE, + LUMIERA_THREADSTATE_RUNNING + } + lumiera_thread_state; + +#include "threadpool.h" + +/** + * The actual thread data + */ +struct lumiera_thread_struct +{ + llist node; + // the function and argument can be passed to the thread at creation time + // void (*function)(void*); + // void* arg; + LumieraReccondition finished; + enum lumiera_thread_class type; + lumiera_thread_state state; + +}; + + /** * Start a thread. * Threads are implemented as procedures which take a void* and dont return anything. @@ -85,7 +117,7 @@ enum lumiera_thread_class * * Threads shall not handle signals (all signals will be disabled for them) unless explicitly acknowledged * * @param kind class of the thread to start - * @param start_routine pointer to a function to execute in a thread (returning void, not void* as in pthreads) + * @param function pointer to a function to execute in a thread (returning void, not void* as in pthreads) * @param arg generic pointer passed to the thread * @param finished a condition variable to be broadcasted, if not NULL. * The associated mutex should be locked at thread_run time already, else the signal can get lost. @@ -94,14 +126,13 @@ enum lumiera_thread_class */ LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, - void (*start_routine)(void *), + void (*function)(void *), void * arg, LumieraReccondition finished, const char* purpose, struct nobug_flag* flag); - #endif /* // Local Variables: diff --git a/tests/50threadpool.tests b/tests/50threadpool.tests new file mode 100644 index 000000000..1d005e151 --- /dev/null +++ b/tests/50threadpool.tests @@ -0,0 +1,5 @@ +TESTING "Thread Pool" ./test-threadpool + +TEST "Fake test" faketest < + + This program 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; either version 2 of the + License, or (at your option) any later version. + + 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., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#include "tests/test.h" + +#include "backend/threadpool.h" + +#include +#include + +TESTS_BEGIN + + +TEST ("faketest") +{ + printf("this is a fake test\n"); + /* create a threadpool manager, + it will automatically create all the threadpools */ + lumiera_threadpoolmanager_new(); +#if 0 + /* create some jobs */ + LumieraJob myjob = lumiera_job_new(functionpointer, parameters); // create a non timed (immediate job) */ + + /* find a suitable threadpool for a given job */ + LumieraThreadpool somepool = lumiera_threadpool_findpool(myjob); //you want to find a pool which is suitable to run a given job by asking: "hey on which pool can I run this job?" */ + + /* pass the jobs to the thread pool: */ + lumiera_threadpool_start_job(somepool, myjob); +#endif + /* wait for the job to finish */ + + /* retrive the job result from myjob */ +} + +TESTS_END diff --git a/tests/library/test-threadpool.c b/tests/library/test-threadpool.c new file mode 100644 index 000000000..66959f701 --- /dev/null +++ b/tests/library/test-threadpool.c @@ -0,0 +1,53 @@ +/* + test-threadpool.c - test thread pool creation and usage + + Copyright (C) Lumiera.org + 2009, Michael Ploujnikov + + This program 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; either version 2 of the + License, or (at your option) any later version. + + 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., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#include "tests/test.h" + +#include "backend/threads.h" + +#include +#include + +TESTS_BEGIN + + +TEST ("faketest") +{ + printf("this is a fake test\n"); + /* create a threadpool manager, + it will automatically create all the threadpools */ + lumiera_threadpoolmanager_new(); +#if 0 + /* create some jobs */ + LumieraJob myjob = lumiera_job_new(functionpointer, parameters); // create a non timed (immediate job) */ + + /* find a suitable threadpool for a given job */ + LumieraThreadpool somepool = lumiera_threadpool_findpool(myjob); //you want to find a pool which is suitable to run a given job by asking: "hey on which pool can I run this job?" */ + + /* pass the jobs to the thread pool: */ + lumiera_threadpool_start_job(somepool, myjob); +#endif + /* wait for the job to finish */ + + /* retrive the job result from myjob */ +} + +TESTS_END From 775f6cbb5d800c6dc223b3810b92b1e3e9540fdd Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sun, 22 Nov 2009 20:08:08 -0500 Subject: [PATCH 05/68] actually create or use a thread struct, still not associated with a pthread --- src/backend/threadpool.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 27729895b..70dbb528d 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -194,7 +194,16 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, return (LumieraThreadpool)0; } - // TODO: either get a thread from the pool or create a new one + if (llist_is_empty (tp->threads)) + { + // how does this become an actual pthread? + return lumiera_thread_new (kind, NULL, purpose, flag); + } + else + { + // use an existing thread, pick the first one + return (LumieraThread)(llist_head (&tp->threads)); + } } LumieraThreadpool From 9dd838b129f2315ccc88bcc471aea2721659c4b8 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 23 Nov 2009 16:40:31 -0500 Subject: [PATCH 06/68] acquire() test started, so far, everything just compiles --- src/backend/threadpool.c | 188 +++----------------------------- src/backend/threadpool.h | 59 ++++------ src/backend/threads.c | 42 ++++++- src/backend/threads.h | 20 +++- tests/50threadpool.tests | 14 ++- tests/Makefile.am | 6 + tests/backend/test-threadpool.c | 38 ++++--- 7 files changed, 129 insertions(+), 238 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 70dbb528d..608088137 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -28,73 +28,7 @@ #include "backend/threadpool.h" //TODO: internal/static forward declarations// -/** - * Find a suitable thread pool for the given thread type. - */ -static -LumieraThreadpool -lumiera_threadpool_findpool(enum lumiera_thread_class kind); - -typedef struct lumiera_threadpoolmanager_struct lumiera_threadpoolmanager; -typedef lumiera_threadpoolmanager* LumieraThreadpoolmanager; - -struct lumiera_threadpoolmanager_struct -{ - llist pools; -}; - -/** - * Create the singleton threadpool manager - */ -static -void -lumiera_threadpoolmanager_new(void); - -/** - * Delete/remove the singleton threadpool manager - */ -static -void -lumiera_threadpoolmanager_delete(void); - -// singleton threadpool manager instance -static LumieraThreadpoolmanager lumiera_tpmanager; - -typedef struct lumiera_threadpool_struct lumiera_threadpool; -typedef lumiera_threadpool* LumieraThreadpool; - -enum lumiera_threadpool_type - { - // TODO: the following types need to be more thought out: - /** the default kind of threadpool **/ - LUMIERA_DEFAULT_THREADPOOL, - /** a threadpool for special threads **/ - LUMIERA_SPECIAL_THREADPOOL - }; - -struct lumiera_threadpool_struct -{ - /* a list of threadpools for a threadpool manager */ - llist node; - - enum lumiera_threadpool_type type; - - llist threads; -}; - -/** - * Create a thread pool. - */ -static -LumieraThreadpool -lumiera_threadpool_new(enum lumiera_threadpool_type type); - -/** - * Delete/remove a threadpool. - */ -static -void -lumiera_threadpool_delete(LumieraThreadpool threadpool); +static lumiera_threadpool threadpool; //TODO: System includes// #include @@ -119,130 +53,38 @@ void* pool_thread_loop(void * arg) return arg; } -static pthread_once_t attr_once = PTHREAD_ONCE_INIT; -static pthread_attr_t attrs; - -static void thread_attr_init (void) -{ - pthread_attr_init (&attrs); - pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED); - //cancel ... -} - -LumieraThread -lumiera_thread_new (enum lumiera_thread_class kind, - LumieraReccondition finished, - const char* purpose, - struct nobug_flag* flag) -{ - (void) kind; - (void) purpose; - (void) flag; - - if (attr_once == PTHREAD_ONCE_INIT) - pthread_once (&attr_once, thread_attr_init); - - LumieraThread thread = lumiera_malloc (sizeof(*thread)); - - thread->finished = finished; - - pthread_t dummy; - printf("creating a thread\n"); - int error = pthread_create (&dummy, &attrs, &pool_thread_loop, thread); - - if (error) return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? - return thread; -} - -LumieraThreadpool -lumiera_threadpool_new(enum lumiera_threadpool_type type) -{ - LumieraThreadpool self = lumiera_malloc (sizeof (*self)); - self->type = type; - - /* we start with no threads in the list */ - llist_init (&self->threads); - - return self; -} - -LumieraThreadpoolmanager lumiera_tpmanager = NULL; - void -lumiera_threadpoolmanager_new() +lumiera_threadpool_init(void) { - REQUIRE (!lumiera_tpmanager, "Threadpool manager already initialized"); - lumiera_tpmanager = lumiera_malloc (sizeof (lumiera_threadpoolmanager)); - llist_init (&lumiera_tpmanager->pools); - - // create a default threadpool - LumieraThreadpool default_tp = lumiera_threadpool_new(LUMIERA_DEFAULT_THREADPOOL); - llist_insert_head (&lumiera_tpmanager->pools, &default_tp->node); -}; + for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) + { + llist_init(&threadpool.kind[i].pool); + lumiera_mutex_init(&threadpool.kind[i].lock,"pool of threads", NULL); + } +} LumieraThread lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - LumieraThreadpool tp = - lumiera_threadpool_findpool(enum lumiera_thread_class kind); + // TODO: do we need to check that index 'kind' is within range? + llist pool = threadpool.kind[kind].pool; + // TODO: do we need to check that we get a valid list? - if (!tp) + // TODO: do proper locking when manipulating the list + if (llist_is_empty (&pool)) { - // ERROR: could not find a suitable threadpool or there are no threadpools - return (LumieraThreadpool)0; - } - - if (llist_is_empty (tp->threads)) - { - // how does this become an actual pthread? return lumiera_thread_new (kind, NULL, purpose, flag); } else { // use an existing thread, pick the first one - return (LumieraThread)(llist_head (&tp->threads)); + // remove it from the pool's list + return (LumieraThread)(llist_unlink(llist_head (&pool))); } } -LumieraThreadpool -lumiera_threadpool_findpool(enum lumiera_thread_class thread_kind) -{ - if (!lumiera_tpmanager) - { - // create a threadpool manager if it doesn't exist yet - lumiera_threadpoolmanager_new(); - } - if (!lumiera_tpmanager) - { - // ERROR: if the threadpool manager still doesn't exist, something went wrong - return (LumieraThreadpool)0; - } - else - { - // for now, ignore the thread all together and just return the first threadpool - (void) thread_kind; - // BUG: what if there are no pools in the list??? - // where/when should I check for that case? - return (LumieraThreadpool)(llist_head (&lumiera_tpmanager->pools)); - } -} - -void -lumiera_threadpoolmanager_new(void) -{ - REQUIRE (!lumiera_tpmanager, "Threadpool manager already initialized"); - - lumiera_tpmanager = lumiera_malloc (sizeof (lumiera_tpmanager)); - llist_init (&lumiera_tpmanager->pools); - - // start with just one default threadpool in the list - LumieraThreadpool tp = - lumiera_threadpool_new(LUMIERA_DEFAULT_THREADPOOL); - llist_insert_head (&lumiera_tp->pools, &tp->node); -} - /* // Local Variables: // mode: C diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 6d11cb093..f6b359150 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -25,6 +25,7 @@ //TODO: Support library includes// #include "lib/reccondition.h" #include "lib/llist.h" +#include "lib/mutex.h" //TODO: Forward declarations// @@ -43,46 +44,9 @@ //TODO: declarations go here// -typedef struct lumiera_threadpool_struct lumiera_threadpool; -typedef lumiera_threadpool* LumieraThreadpool; - -enum lumiera_threadpool_type - { - // TODO: the following types need to be more thought out: - /** the default kind of threadpool **/ - LUMIERA_DEFAULT_THREADPOOL, - /** a threadpool for special threads **/ - LUMIERA_SPECIAL_THREADPOOL - }; - -struct lumiera_threadpool_struct -{ - /* a list of threadpools for a threadpool manager */ - llist node; - - enum lumiera_threadpool_type type; - - llist threads; -}; - - - -/** - * Create a thread pool. - */ -LumieraThreadpool -lumiera_threadpool_new(enum lumiera_threadpool_type type); - -/** - * Delete/remove a threadpool. - */ -void -lumiera_threadpool_delete(LumieraThreadpool threadpool); - - /** * Acquire a thread from a threadpool. - * This may either pick a thread from the list or create a new one when the list is empty. + * This may either pick a thread from an appropriate pool or create a new one when the pool is empty. * This function doesn't need to be accessible outside of the threadpool implementation. */ LumieraThread @@ -92,12 +56,29 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, /** * Release a thread - * This ends up putting a (parked/idle) thread back on the list of a threadpool. + * This ends up putting a (parked/idle) thread back on the list of an appropriate threadpool. * This function doesn't need to be accessible outside of the threadpool implementation. */ void lumiera_threadpool_release_thread(LumieraThread thread); +typedef struct lumiera_threadpool_struct lumiera_threadpool; +typedef lumiera_threadpool* LumieraThreadpool; + +struct lumiera_threadpool_struct +{ + struct + { + llist pool; + lumiera_mutex lock; + } kind[LUMIERA_THREADCLASS_COUNT]; +}; + +/** + * Initialize the thread pool. + */ +void +lumiera_threadpool_init(void); #endif /* diff --git a/src/backend/threads.c b/src/backend/threads.c index e181b6902..2c8548928 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -22,6 +22,9 @@ //TODO: Support library includes// #include "include/logging.h" +#include "lib/mutex.h" +#include "lib/safeclib.h" + //TODO: Lumiera header includes// #include "threads.h" @@ -50,7 +53,6 @@ struct lumiera_thread_mockup LumieraReccondition finished; }; -#if 0 static void* pthread_runner (void* thread) { pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); @@ -69,7 +71,6 @@ static void* pthread_runner (void* thread) return NULL; } - static pthread_once_t attr_once = PTHREAD_ONCE_INIT; static pthread_attr_t attrs; @@ -79,9 +80,9 @@ static void thread_attr_init (void) pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED); //cancel ... } -#endif #if 0 +// TODO: rewrite this using lumiera_threadpool_acquire() LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*start_routine)(void *), @@ -129,10 +130,10 @@ lumiera_thread_run (enum lumiera_thread_class kind, // ask the threadpool for a thread (it might create a new one) LumieraThread self = lumiera_threadpool_acquire_thread(kind, purpose, flag); - // set the function and data to be run + // TODO: set the function and data to be run // lumiera_thread_set_func_data (self, start_routine, arg, purpose, flag); - // and let it really run (signal the condition var, it waits there) + // and let it really run (signal the condition var, the thread waits on it) LUMIERA_RECCONDITION_SECTION(cond_sync, self->finished) LUMIERA_RECCONDITION_SIGNAL; @@ -141,8 +142,39 @@ lumiera_thread_run (enum lumiera_thread_class kind, return self; } +/** + * Create a new thread structure with a matching pthread + */ +LumieraThread +lumiera_thread_new (enum lumiera_thread_class kind, + LumieraReccondition finished, + const char* purpose, + struct nobug_flag* flag) +{ + (void) purpose; + (void) flag; + if (attr_once == PTHREAD_ONCE_INIT) + pthread_once (&attr_once, thread_attr_init); + LumieraThread self = lumiera_malloc (sizeof (*self)); + llist_init(&self->node); + // self->id = (pthread_t)NULL; initialized by pthread_create() + self->finished = finished; + self->type = kind; + self->state = LUMIERA_THREADSTATE_IDLE; + + printf("creating a thread\n"); + int error = pthread_create (&self->id, &attrs, &pthread_runner, self); + + if (error) + { + free(self); + return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? + } + + return self; +} /* // Local Variables: diff --git a/src/backend/threads.h b/src/backend/threads.h index d50a4fb5d..67c036001 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -60,12 +60,16 @@ enum lumiera_thread_class /** busy at average priority **/ LUMIERA_THREAD_WORKER, /** busy, soft realtime, high priority **/ - LUMIERA_THREAD_URGEND, + LUMIERA_THREAD_URGENT, /** high latency, background jobs **/ LUMIERA_THREAD_BATCH, /** Something to do when there is really nothing else to do **/ LUMIERA_THREAD_IDLE, + /** this just denotes the number of classes listed above, + it is used to create arrays **/ + LUMIERA_THREADCLASS_COUNT, + // .. various thread flags follow /** * flag to let the decision to run the function in a thread open to the backend. * depending on load it might decide to run it sequentially. @@ -82,9 +86,9 @@ enum lumiera_thread_class */ typedef enum { - LUMIERA_THREADSTATE_ERROR, LUMIERA_THREADSTATE_IDLE, - LUMIERA_THREADSTATE_RUNNING + LUMIERA_THREADSTATE_RUNNING, + LUMIERA_THREADSTATE_ERROR } lumiera_thread_state; @@ -99,12 +103,20 @@ struct lumiera_thread_struct // the function and argument can be passed to the thread at creation time // void (*function)(void*); // void* arg; + pthread_t id; LumieraReccondition finished; enum lumiera_thread_class type; lumiera_thread_state state; - }; +/** + * Create a thread structure. + */ +LumieraThread +lumiera_thread_new (enum lumiera_thread_class kind, + LumieraReccondition finished, + const char* purpose, + struct nobug_flag* flag); /** * Start a thread. diff --git a/tests/50threadpool.tests b/tests/50threadpool.tests index 1d005e151..eadb1cd32 100644 --- a/tests/50threadpool.tests +++ b/tests/50threadpool.tests @@ -1,5 +1,15 @@ TESTING "Thread Pool" ./test-threadpool -TEST "Fake test" faketest < Date: Mon, 23 Nov 2009 16:51:18 -0500 Subject: [PATCH 07/68] add lumiera_threadpool_release_thread make the test compile change from type to kind --- src/backend/threadpool.c | 8 ++++++++ src/backend/threads.c | 2 +- src/backend/threads.h | 2 +- tests/50threadpool.tests | 4 ++-- tests/backend/test-threadpool.c | 8 ++++---- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 608088137..a6d58cfe0 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -85,6 +85,14 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, } } +void +lumiera_threadpool_release_thread(LumieraThread thread) +{ + // TODO: do we need to check that index 'kind' is within range? + llist pool = threadpool.kind[thread->kind].pool; + llist_insert_head(&pool, &thread->node); +} + /* // Local Variables: // mode: C diff --git a/src/backend/threads.c b/src/backend/threads.c index 2c8548928..7b18ab70a 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -161,7 +161,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, llist_init(&self->node); // self->id = (pthread_t)NULL; initialized by pthread_create() self->finished = finished; - self->type = kind; + self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; printf("creating a thread\n"); diff --git a/src/backend/threads.h b/src/backend/threads.h index 67c036001..ec93c4df6 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -105,7 +105,7 @@ struct lumiera_thread_struct // void* arg; pthread_t id; LumieraReccondition finished; - enum lumiera_thread_class type; + enum lumiera_thread_class kind; lumiera_thread_state state; }; diff --git a/tests/50threadpool.tests b/tests/50threadpool.tests index eadb1cd32..935fb7169 100644 --- a/tests/50threadpool.tests +++ b/tests/50threadpool.tests @@ -4,9 +4,9 @@ TEST "Acquire/Release test" basic-acquire-release <kind); + printf("thread 1 state=%d\n", t1->state); + printf("thread 2 kind=%d\n", t2->kind); + printf("thread 2 state=%d\n", t2->state); printf("releasing thread 1\n"); lumiera_threadpool_release_thread(t1); From 75696986e4b87380b03a5afa4923f49b389ffc73 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 23 Nov 2009 19:05:57 -0500 Subject: [PATCH 08/68] test passes --- src/backend/threadpool.c | 5 ++--- tests/30backend-threadpool.tests | 13 ++++++++++++- tests/50threadpool.tests | 15 --------------- tests/backend/test-threadpool.c | 26 +++++++++++++++----------- 4 files changed, 29 insertions(+), 30 deletions(-) delete mode 100644 tests/50threadpool.tests diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index a6d58cfe0..063794012 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -37,8 +37,7 @@ static lumiera_threadpool threadpool; * @file * */ - -//NOBUG_DEFINE_FLAG_PARENT (threadpool, lumiera); /*TODO insert a suitable/better parent flag here */ +NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); //code goes here// @@ -59,7 +58,7 @@ lumiera_threadpool_init(void) for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.kind[i].pool); - lumiera_mutex_init(&threadpool.kind[i].lock,"pool of threads", NULL); + lumiera_mutex_init(&threadpool.kind[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); } } diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index dc178ba91..16fd316d4 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -1,6 +1,17 @@ -TESTING "Thread Pools" +TESTING "Thread Pools" ./test-threadpool PLANNED "create" PLANNED "yield" PLANNED "cancel" + +TEST "Acquire/Release test" basic-acquire-release <kind); - printf("thread 1 state=%d\n", t1->state); - printf("thread 2 kind=%d\n", t2->kind); - printf("thread 2 state=%d\n", t2->state); + //ECHO("thread 1 kind=%d", t1->kind); + CHECK(LUMIERA_THREAD_INTERACTIVE == t1->kind); + //ECHO("thread 1 state=%d", t1->state); + CHECK(LUMIERA_THREADSTATE_IDLE == t1->state); + //ECHO("thread 2 kind=%d", t2->kind); + // CHECK(LUMIERA_THREAD_IDLE == t2->kind); + //ECHO("thread 2 state=%d", t2->state); + CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); - printf("releasing thread 1\n"); + ECHO("releasing thread 1"); lumiera_threadpool_release_thread(t1); - printf("thread 1 has been released\n"); + ECHO("thread 1 has been released"); - printf("releasing thread 2\n"); + ECHO("releasing thread 2"); lumiera_threadpool_release_thread(t2); - printf("thread 2 has been released\n"); + ECHO("thread 2 has been released"); } TESTS_END From 414b8b99c341b5fd8a1891074fd1bdfaf1c8095a Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 23 Nov 2009 19:47:20 -0500 Subject: [PATCH 09/68] updated test passes fixed problem with copying lists --- src/backend/threadpool.c | 6 ++---- src/backend/threads.c | 10 ++++++++-- tests/backend/test-threadpool.c | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 063794012..87ca3803c 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -68,11 +68,9 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, struct nobug_flag* flag) { // TODO: do we need to check that index 'kind' is within range? - llist pool = threadpool.kind[kind].pool; // TODO: do we need to check that we get a valid list? - // TODO: do proper locking when manipulating the list - if (llist_is_empty (&pool)) + if (llist_is_empty (&threadpool.kind[kind].pool)) { return lumiera_thread_new (kind, NULL, purpose, flag); } @@ -80,7 +78,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, { // use an existing thread, pick the first one // remove it from the pool's list - return (LumieraThread)(llist_unlink(llist_head (&pool))); + return (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); } } diff --git a/src/backend/threads.c b/src/backend/threads.c index 7b18ab70a..e87853765 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -53,6 +53,12 @@ struct lumiera_thread_mockup LumieraReccondition finished; }; +static void* thread_loop (void* arg) +{ + return arg; +} + +#if 0 static void* pthread_runner (void* thread) { pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); @@ -70,6 +76,7 @@ static void* pthread_runner (void* thread) return NULL; } +#endif static pthread_once_t attr_once = PTHREAD_ONCE_INIT; static pthread_attr_t attrs; @@ -164,8 +171,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; - printf("creating a thread\n"); - int error = pthread_create (&self->id, &attrs, &pthread_runner, self); + int error = pthread_create (&self->id, &attrs, &thread_loop, self); if (error) { diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index 902a948c2..301688f48 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -49,7 +49,7 @@ TEST ("basic-acquire-release") //ECHO("thread 1 state=%d", t1->state); CHECK(LUMIERA_THREADSTATE_IDLE == t1->state); //ECHO("thread 2 kind=%d", t2->kind); - // CHECK(LUMIERA_THREAD_IDLE == t2->kind); + CHECK(LUMIERA_THREAD_IDLE == t2->kind); //ECHO("thread 2 state=%d", t2->state); CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); From d6d81f2e441b43d06021a025aabf06ebb2775fd8 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 24 Nov 2009 22:25:00 -0500 Subject: [PATCH 10/68] added lumiera_threadpool_destroy() added locking and checks to lumiera_threadpool_acquire_thread() added a nobug flag to threads.c added lumiera_thread_destroy() added lumiera_thread_delete() --- src/backend/threadpool.c | 36 +++++++++++++++++++++++++++------ src/backend/threadpool.h | 3 +++ src/backend/threads.c | 27 +++++++++++++++++++++++-- src/backend/threads.h | 6 ++++++ tests/backend/test-threadpool.c | 2 ++ 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 87ca3803c..970e7291c 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -37,7 +37,7 @@ static lumiera_threadpool threadpool; * @file * */ -NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); +NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/better parent flag here */ //code goes here// @@ -62,24 +62,48 @@ lumiera_threadpool_init(void) } } +void +lumiera_threadpool_destroy(void) +{ + for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) + { + // no locking is done at this point + LLIST_FOREACH(&threadpool.kind[i].pool, thread) + { + llist_unlink(thread); + lumiera_thread_delete((LumieraThread)thread); + } + + lumiera_mutex_destroy (&threadpool.kind[i].lock, &NOBUG_FLAG (threadpool)); + } +} + LumieraThread lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - // TODO: do we need to check that index 'kind' is within range? - // TODO: do we need to check that we get a valid list? - // TODO: do proper locking when manipulating the list + LumieraThread ret; + + REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); + REQUIRE (&threadpool.kind[kind].pool, "threadpool kind %d does not exist", kind); if (llist_is_empty (&threadpool.kind[kind].pool)) { - return lumiera_thread_new (kind, NULL, purpose, flag); + // TODO: fill in the reccondition argument, currently NULL + ret = lumiera_thread_new (kind, NULL, purpose, flag); } else { + REQUIRE (&threadpool.kind[kind].lock, "invalid threadpool lock"); // use an existing thread, pick the first one // remove it from the pool's list - return (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); + LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[kind].lock) + { + ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); + } } + REQUIRE(ret, "did not find a valid thread"); + return ret; } void diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index f6b359150..0d2a0257f 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -80,6 +80,9 @@ struct lumiera_threadpool_struct void lumiera_threadpool_init(void); +void +lumiera_threadpool_destroy(void); + #endif /* // Local Variables: diff --git a/src/backend/threads.c b/src/backend/threads.c index e87853765..6104a8011 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -40,7 +40,7 @@ * */ -//NOBUG_DEFINE_FLAG_PARENT (threads, lumiera); /*TODO insert a suitable/better parent flag here */ +NOBUG_DEFINE_FLAG_PARENT (threads, threads_dbg); /*TODO insert a suitable/better parent flag here */ //code goes here// @@ -158,6 +158,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { + // TODO: do something with these: (void) purpose; (void) flag; @@ -166,11 +167,15 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); - // self->id = (pthread_t)NULL; initialized by pthread_create() self->finished = finished; + REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; + REQUIRE (&self->id); + //REQUIRE (&attrs); + //REQUIRE (&thread_loop); + REQUIRE (self); int error = pthread_create (&self->id, &attrs, &thread_loop, self); if (error) @@ -182,6 +187,24 @@ lumiera_thread_new (enum lumiera_thread_class kind, return self; } +LumieraThread +lumiera_thread_destroy (LumieraThread self) +{ + // TODO: stop the pthread + // TODO: empty the list/node? + //finished = NULL; // or free(finished)? + lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG(threads)); + //kind = 0; + //state = 0; + return self; +} + +void +lumiera_thread_delete (LumieraThread self) +{ + lumiera_free (lumiera_thread_destroy (self)); +} + /* // Local Variables: // mode: C diff --git a/src/backend/threads.h b/src/backend/threads.h index ec93c4df6..51000782a 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -118,6 +118,12 @@ lumiera_thread_new (enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag); +LumieraThread +lumiera_thread_destroy (LumieraThread self); + +void +lumiera_thread_delete (LumieraThread self); + /** * Start a thread. * Threads are implemented as procedures which take a void* and dont return anything. diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index 301688f48..be3b4311c 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -60,6 +60,8 @@ TEST ("basic-acquire-release") ECHO("releasing thread 2"); lumiera_threadpool_release_thread(t2); ECHO("thread 2 has been released"); + + lumiera_threadpool_destroy(); } TESTS_END From 8e3d4e7b7aa96cb045b67fe98574a958c92a9249 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 25 Nov 2009 11:06:53 -0500 Subject: [PATCH 11/68] fixed lumiera_threadpool_destroy() by using LLIST_FOREACH added some checks to lumiera_threadpool_release_thread(), maybe too much modified thread_loop() to return a known value - 0 added checks to lumiera_thread_new() added a check to lumiera_thread_destroy() made lumiera_thread_destroy() unlink the thread from a pool list updated test case to match new debug messages --- src/backend/threadpool.c | 21 ++++++++++++--------- src/backend/threads.c | 13 ++++++++++--- tests/30backend-threadpool.tests | 30 ++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 970e7291c..5960c890f 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -65,16 +65,17 @@ lumiera_threadpool_init(void) void lumiera_threadpool_destroy(void) { + ECHO ("destroying threadpool"); for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { + ECHO ("destroying individual pool #%d", i); // no locking is done at this point - LLIST_FOREACH(&threadpool.kind[i].pool, thread) - { - llist_unlink(thread); - lumiera_thread_delete((LumieraThread)thread); - } - + ECHO ("number of threads in the pool=%d", llist_count(&threadpool.kind[i].pool)); + LLIST_WHILE_HEAD(&threadpool.kind[i].pool, thread) + lumiera_thread_delete((LumieraThread)thread); + ECHO ("destroying the pool mutex"); lumiera_mutex_destroy (&threadpool.kind[i].lock, &NOBUG_FLAG (threadpool)); + ECHO ("pool mutex destroyed"); } } @@ -109,9 +110,11 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, void lumiera_threadpool_release_thread(LumieraThread thread) { - // TODO: do we need to check that index 'kind' is within range? - llist pool = threadpool.kind[thread->kind].pool; - llist_insert_head(&pool, &thread->node); + REQUIRE (thread, "invalid thread given"); + REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); + REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); + llist_insert_head(&threadpool.kind[thread->kind].pool, &thread->node); + REQUIRE (!llist_is_empty (&threadpool.kind[thread->kind].pool), "thread pool is still empty after insertion"); } /* diff --git a/src/backend/threads.c b/src/backend/threads.c index 6104a8011..4471fb444 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -55,7 +55,8 @@ struct lumiera_thread_mockup static void* thread_loop (void* arg) { - return arg; + (void)arg; + return 0; } #if 0 @@ -165,10 +166,12 @@ lumiera_thread_new (enum lumiera_thread_class kind, if (attr_once == PTHREAD_ONCE_INIT) pthread_once (&attr_once, thread_attr_init); + REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); + //REQUIRE (finished, "invalid finished flag passed"); + LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); self->finished = finished; - REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; @@ -190,8 +193,11 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread lumiera_thread_destroy (LumieraThread self) { + ECHO ("destroying thread"); + REQUIRE (self, "trying to destroy an invalid thread"); + // TODO: stop the pthread - // TODO: empty the list/node? + llist_unlink(&self->node); //finished = NULL; // or free(finished)? lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG(threads)); //kind = 0; @@ -202,6 +208,7 @@ lumiera_thread_destroy (LumieraThread self) void lumiera_thread_delete (LumieraThread self) { + ECHO ("deleting thread"); lumiera_free (lumiera_thread_destroy (self)); } diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index 16fd316d4..a23ff38ea 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -13,5 +13,35 @@ err: releasing thread 1 err: thread 1 has been released err: releasing thread 2 err: thread 2 has been released +err: destroying threadpool + +err: destroying individual pool #0 +err: number of threads in the pool=1 +err: deleting thread +err: destroying thread +err: destroying the pool mutex +err: pool mutex destroyed + +err: destroying individual pool #1 +err: number of threads in the pool=0 +err: destroying the pool mutex +err: pool mutex destroyed + +err: destroying individual pool #2 +err: number of threads in the pool=0 +err: destroying the pool mutex +err: pool mutex destroyed + +err: destroying individual pool #3 +err: number of threads in the pool=0 +err: destroying the pool mutex +err: pool mutex destroyed + +err: destroying individual pool #4 +err: number of threads in the pool=1 +err: deleting thread +err: destroying thread +err: destroying the pool mutex +err: pool mutex destroyed END From 07962b5314ab9d2b784f29e79fb21dd02a055e2d Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 25 Nov 2009 17:43:22 -0500 Subject: [PATCH 12/68] add comments about locking in lumiera_threadpool_release_thread() output the size of the created thread struct don't print "destroying thread" (lets tests become shorter with repeated matching) add a test which spans many threads in all pools --- src/backend/threadpool.c | 12 +++++++--- src/backend/threads.c | 2 +- tests/30backend-threadpool.tests | 39 ++++++++++++++++++++++++++++++-- tests/backend/test-threadpool.c | 28 +++++++++++++++++++++++ 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 5960c890f..1a48967dd 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -112,9 +112,15 @@ lumiera_threadpool_release_thread(LumieraThread thread) { REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); - llist_insert_head(&threadpool.kind[thread->kind].pool, &thread->node); - REQUIRE (!llist_is_empty (&threadpool.kind[thread->kind].pool), "thread pool is still empty after insertion"); + REQUIRE (&threadpool.kind[thread->kind].lock, "invalid threadpool lock"); + + // TOOD: currently, locking produces memory leaks + // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[thread->kind].lock) + // { + //REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); + llist_insert_head(&threadpool.kind[thread->kind].pool, &thread->node); + // REQUIRE (!llist_is_empty (&threadpool.kind[thread->kind].pool), "thread pool is still empty after insertion"); + // } } /* diff --git a/src/backend/threads.c b/src/backend/threads.c index 4471fb444..52125fcef 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -170,6 +170,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, //REQUIRE (finished, "invalid finished flag passed"); LumieraThread self = lumiera_malloc (sizeof (*self)); + ECHO ("allocated thread struct of size %zd", sizeof (*self)); llist_init(&self->node); self->finished = finished; self->kind = kind; @@ -193,7 +194,6 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread lumiera_thread_destroy (LumieraThread self) { - ECHO ("destroying thread"); REQUIRE (self, "trying to destroy an invalid thread"); // TODO: stop the pthread diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index a23ff38ea..44392f279 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -8,17 +8,19 @@ PLANNED "cancel" TEST "Acquire/Release test" basic-acquire-release < Date: Wed, 25 Nov 2009 17:47:43 -0500 Subject: [PATCH 13/68] remove "allocated thread*" messages from lumiera_thread_new() and update tests --- src/backend/threads.c | 1 - tests/30backend-threadpool.tests | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 52125fcef..5ea2aa56f 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -170,7 +170,6 @@ lumiera_thread_new (enum lumiera_thread_class kind, //REQUIRE (finished, "invalid finished flag passed"); LumieraThread self = lumiera_malloc (sizeof (*self)); - ECHO ("allocated thread struct of size %zd", sizeof (*self)); llist_init(&self->node); self->finished = finished; self->kind = kind; diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index 44392f279..0ce7ca67a 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -8,9 +8,7 @@ PLANNED "cancel" TEST "Acquire/Release test" basic-acquire-release < Date: Wed, 25 Nov 2009 18:58:15 -0500 Subject: [PATCH 14/68] check that we're creating valid lumiera_thread_structures die on pthread_create errors that we shouldn't be handling --- src/backend/threadpool.c | 3 ++- src/backend/threads.c | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 1a48967dd..3dbfd0c92 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -92,6 +92,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, { // TODO: fill in the reccondition argument, currently NULL ret = lumiera_thread_new (kind, NULL, purpose, flag); + ENSURE (ret, "did not create a valid thread"); } else { @@ -102,8 +103,8 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, { ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); } + ENSURE (ret, "did not find a valid thread"); } - REQUIRE(ret, "did not find a valid thread"); return ret; } diff --git a/src/backend/threads.c b/src/backend/threads.c index 5ea2aa56f..6cf4559be 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -34,6 +34,7 @@ //TODO: System includes// #include +#include /** * @file @@ -180,13 +181,16 @@ lumiera_thread_new (enum lumiera_thread_class kind, //REQUIRE (&thread_loop); REQUIRE (self); int error = pthread_create (&self->id, &attrs, &thread_loop, self); - + ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); + FIXME("handle EAGAIN"); if (error) { - free(self); +#if 0 return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? +#endif } + REQUIRE (self, "returning an invalid thread structure"); return self; } From bd076e4210d7623f9c0544d2fc2f5bafa82e5c69 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 25 Nov 2009 19:58:54 -0500 Subject: [PATCH 15/68] reduce the number of threads spawned in a test --- tests/30backend-threadpool.tests | 10 +++++----- tests/backend/test-threadpool.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index 0ce7ca67a..03808507a 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -48,31 +48,31 @@ TEST "Many Acquires/Releases test" many-acquire-release < Date: Wed, 25 Nov 2009 21:46:03 -0500 Subject: [PATCH 16/68] begin implementing a 'soft' thread count limit per pool add LUMIERA_DIE in cases where this soft limit is reached add LUMIERA_DIE when pthread_create fails to create threads - serious add a test which tries to break the soft limit --- src/backend/threadpool.c | 37 +++++++++++++++++++++++++++----- src/backend/threadpool.h | 6 +++++- src/backend/threads.c | 7 +++--- tests/30backend-threadpool.tests | 6 ++++++ tests/backend/test-threadpool.c | 32 +++++++++++++++++++++++++-- 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 3dbfd0c92..c77225f78 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -53,11 +53,14 @@ void* pool_thread_loop(void * arg) } void -lumiera_threadpool_init(void) +lumiera_threadpool_init(unsigned limit) { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.kind[i].pool); + threadpool.kind[i].max_threads = limit; + threadpool.kind[i].working_thread_count = 0; + threadpool.kind[i].idle_thread_count = 0; lumiera_mutex_init(&threadpool.kind[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); } } @@ -91,17 +94,34 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, if (llist_is_empty (&threadpool.kind[kind].pool)) { // TODO: fill in the reccondition argument, currently NULL - ret = lumiera_thread_new (kind, NULL, purpose, flag); - ENSURE (ret, "did not create a valid thread"); + FIXME ("this max thread logic needs to be deeply thought about and made more efficient as well as rebust"); + if (threadpool.kind[kind].working_thread_count + + threadpool.kind[kind].idle_thread_count + < threadpool.kind[kind].max_threads) { + ret = lumiera_thread_new (kind, NULL, purpose, flag); + threadpool.kind[kind].working_thread_count++; + ENSURE (ret, "did not create a valid thread"); + } + else + { + //ERROR (threadpool, "did not create a new thread because per-pool limit was reached: %d", threadpool.kind[kind].max_threads); + LUMIERA_DIE(ERRNO); + } } else { - REQUIRE (&threadpool.kind[kind].lock, "invalid threadpool lock"); // use an existing thread, pick the first one // remove it from the pool's list LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[kind].lock) { ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); + threadpool.kind[kind].working_thread_count++; + threadpool.kind[kind].idle_thread_count--; // cheaper than using llist_count + ENSURE (threadpool.kind[kind].idle_thread_count == + llist_count(&threadpool.kind[kind].pool), + "idle thread count %d is wrong, should be %d", + threadpool.kind[kind].idle_thread_count, + llist_count(&threadpool.kind[kind].pool)); } ENSURE (ret, "did not find a valid thread"); } @@ -118,8 +138,15 @@ lumiera_threadpool_release_thread(LumieraThread thread) // TOOD: currently, locking produces memory leaks // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[thread->kind].lock) // { - //REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); + REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); llist_insert_head(&threadpool.kind[thread->kind].pool, &thread->node); + threadpool.kind[thread->kind].working_thread_count--; + threadpool.kind[thread->kind].idle_thread_count++; // cheaper than using llist_count + ENSURE (threadpool.kind[thread->kind].idle_thread_count == + llist_count(&threadpool.kind[thread->kind].pool), + "idle thread count %d is wrong, should be %d", + threadpool.kind[thread->kind].idle_thread_count, + llist_count(&threadpool.kind[thread->kind].pool)); // REQUIRE (!llist_is_empty (&threadpool.kind[thread->kind].pool), "thread pool is still empty after insertion"); // } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 0d2a0257f..fbd45c8a4 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -71,14 +71,18 @@ struct lumiera_threadpool_struct { llist pool; lumiera_mutex lock; + unsigned max_threads; + unsigned working_thread_count; + unsigned idle_thread_count; } kind[LUMIERA_THREADCLASS_COUNT]; }; /** * Initialize the thread pool. + * @param limit the maximum number of threads (idle+working) allowed per pool */ void -lumiera_threadpool_init(void); +lumiera_threadpool_init(unsigned limit); void lumiera_threadpool_destroy(void); diff --git a/src/backend/threads.c b/src/backend/threads.c index 6cf4559be..f4277fd47 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -182,12 +182,11 @@ lumiera_thread_new (enum lumiera_thread_class kind, REQUIRE (self); int error = pthread_create (&self->id, &attrs, &thread_loop, self); ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); - FIXME("handle EAGAIN"); if (error) { -#if 0 - return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? -#endif + // error here can only be EAGAIN, given the above ENSURE + FIXME ("error is %d:%s, see if this can be improved", error, strerror(error)); + LUMIERA_DIE (ERRNO); } REQUIRE (self, "returning an invalid thread structure"); diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index 03808507a..a92b32059 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -77,3 +77,9 @@ err: deleting thread err: destroying the pool mutex err: pool mutex destroyed END + +TEST "Too Many Acquires/Releases test" toomany-acquire-release < Date: Thu, 26 Nov 2009 11:15:31 -0500 Subject: [PATCH 17/68] this was replaced with tests/backend/test-threadpool.c --- tests/library/test-threadpool.c | 53 --------------------------------- 1 file changed, 53 deletions(-) delete mode 100644 tests/library/test-threadpool.c diff --git a/tests/library/test-threadpool.c b/tests/library/test-threadpool.c deleted file mode 100644 index 66959f701..000000000 --- a/tests/library/test-threadpool.c +++ /dev/null @@ -1,53 +0,0 @@ -/* - test-threadpool.c - test thread pool creation and usage - - Copyright (C) Lumiera.org - 2009, Michael Ploujnikov - - This program 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; either version 2 of the - License, or (at your option) any later version. - - 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., 675 Mass Ave, Cambridge, MA 02139, USA. -*/ - -#include "tests/test.h" - -#include "backend/threads.h" - -#include -#include - -TESTS_BEGIN - - -TEST ("faketest") -{ - printf("this is a fake test\n"); - /* create a threadpool manager, - it will automatically create all the threadpools */ - lumiera_threadpoolmanager_new(); -#if 0 - /* create some jobs */ - LumieraJob myjob = lumiera_job_new(functionpointer, parameters); // create a non timed (immediate job) */ - - /* find a suitable threadpool for a given job */ - LumieraThreadpool somepool = lumiera_threadpool_findpool(myjob); //you want to find a pool which is suitable to run a given job by asking: "hey on which pool can I run this job?" */ - - /* pass the jobs to the thread pool: */ - lumiera_threadpool_start_job(somepool, myjob); -#endif - /* wait for the job to finish */ - - /* retrive the job result from myjob */ -} - -TESTS_END From 99eb7900276d2ee629081251e4412644eef55bf0 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 26 Nov 2009 11:21:31 -0500 Subject: [PATCH 18/68] rename LUMIERA_THREAD_* to LUMIERA_THREADCLASS_* in enum lumiera_thread_class --- src/backend/thread-wrapper.hpp | 4 ++-- src/backend/threads.h | 13 ++++++------- tests/backend/test-threadpool.c | 8 ++++---- tests/backend/test-threads.c | 6 +++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/backend/thread-wrapper.hpp b/src/backend/thread-wrapper.hpp index 730b4338b..3f897b582 100644 --- a/src/backend/thread-wrapper.hpp +++ b/src/backend/thread-wrapper.hpp @@ -207,7 +207,7 @@ namespace backend { : started_(false), operation_(operation) { - start_thread (LUMIERA_THREAD_INTERACTIVE, purpose, logging_flag); + start_thread (LUMIERA_THREADCLASS_INTERACTIVE, purpose, logging_flag); } /** Variant of the standard case, used to register a JoinHandle in addition to starting a thread. @@ -220,7 +220,7 @@ namespace backend { : started_(false), operation_(operation) { - start_thread (LUMIERA_THREAD_INTERACTIVE, purpose, logging_flag, + start_thread (LUMIERA_THREADCLASS_INTERACTIVE, purpose, logging_flag, join.accessLockedCondition()); } }; diff --git a/src/backend/threads.h b/src/backend/threads.h index 51000782a..3a45bc5e5 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -51,20 +51,19 @@ typedef lumiera_thread* LumieraThread; * Thread classes. * We define some 'classes' of threads for different purposes to abstract * priorities and other attributes. - ** TODO: rename these to LUMIERA_THREADCLASS_* */ enum lumiera_thread_class { /** mostly idle, low latency **/ - LUMIERA_THREAD_INTERACTIVE, + LUMIERA_THREADCLASS_INTERACTIVE, /** busy at average priority **/ - LUMIERA_THREAD_WORKER, + LUMIERA_THREADCLASS_WORKER, /** busy, soft realtime, high priority **/ - LUMIERA_THREAD_URGENT, + LUMIERA_THREADCLASS_URGENT, /** high latency, background jobs **/ - LUMIERA_THREAD_BATCH, + LUMIERA_THREADCLASS_BATCH, /** Something to do when there is really nothing else to do **/ - LUMIERA_THREAD_IDLE, + LUMIERA_THREADCLASS_IDLE, /** this just denotes the number of classes listed above, it is used to create arrays **/ LUMIERA_THREADCLASS_COUNT, @@ -99,7 +98,7 @@ typedef enum */ struct lumiera_thread_struct { - llist node; + llist node; // this should be first for easy casting // the function and argument can be passed to the thread at creation time // void (*function)(void*); // void* arg; diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index 530fc10de..66de5c1a3 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -35,21 +35,21 @@ TEST ("basic-acquire-release") lumiera_threadpool_init(100); ECHO("acquiring thread 1"); LumieraThread t1 = - lumiera_threadpool_acquire_thread(LUMIERA_THREAD_INTERACTIVE, + lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_INTERACTIVE, "test purpose", NULL); ECHO("acquiring thread 2"); LumieraThread t2 = - lumiera_threadpool_acquire_thread(LUMIERA_THREAD_IDLE, + lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_IDLE, "test purpose", NULL); //ECHO("thread 1 kind=%d", t1->kind); - CHECK(LUMIERA_THREAD_INTERACTIVE == t1->kind); + CHECK(LUMIERA_THREADCLASS_INTERACTIVE == t1->kind); //ECHO("thread 1 state=%d", t1->state); CHECK(LUMIERA_THREADSTATE_IDLE == t1->state); //ECHO("thread 2 kind=%d", t2->kind); - CHECK(LUMIERA_THREAD_IDLE == t2->kind); + CHECK(LUMIERA_THREADCLASS_IDLE == t2->kind); //ECHO("thread 2 state=%d", t2->state); CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); diff --git a/tests/backend/test-threads.c b/tests/backend/test-threads.c index 14237f7a2..76da2f450 100644 --- a/tests/backend/test-threads.c +++ b/tests/backend/test-threads.c @@ -88,7 +88,7 @@ TEST ("simple_thread") { fprintf (stderr, "main before thread %s\n", NOBUG_THREAD_ID_GET); - lumiera_thread_run (LUMIERA_THREAD_WORKER, + lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, threadfn, NULL, NULL, @@ -109,7 +109,7 @@ TEST ("thread_synced") { ECHO ("main before thread %s", NOBUG_THREAD_ID_GET); - lumiera_thread_run (LUMIERA_THREAD_WORKER, + lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, threadsyncfn, &cnd, &cnd, @@ -140,7 +140,7 @@ TEST ("mutex_thread") { fprintf (stderr, "main before thread %s\n", NOBUG_THREAD_ID_GET); - lumiera_thread_run (LUMIERA_THREAD_WORKER, + lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, mutexfn, NULL, NULL, From afd2035983205017bf12aa44eb5e0d9e44735bd6 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 26 Nov 2009 11:27:50 -0500 Subject: [PATCH 19/68] rename threadpool.kind[i].pool to threadpool.kind[i].list rename threadpool.kind to threadpool.pool --- src/backend/threadpool.c | 66 ++++++++++++++++++++-------------------- src/backend/threadpool.h | 4 +-- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index c77225f78..e5273d4c0 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -57,11 +57,11 @@ lumiera_threadpool_init(unsigned limit) { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { - llist_init(&threadpool.kind[i].pool); - threadpool.kind[i].max_threads = limit; - threadpool.kind[i].working_thread_count = 0; - threadpool.kind[i].idle_thread_count = 0; - lumiera_mutex_init(&threadpool.kind[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); + llist_init(&threadpool.pool[i].list); + threadpool.pool[i].max_threads = limit; + threadpool.pool[i].working_thread_count = 0; + threadpool.pool[i].idle_thread_count = 0; + lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); } } @@ -73,11 +73,11 @@ lumiera_threadpool_destroy(void) { ECHO ("destroying individual pool #%d", i); // no locking is done at this point - ECHO ("number of threads in the pool=%d", llist_count(&threadpool.kind[i].pool)); - LLIST_WHILE_HEAD(&threadpool.kind[i].pool, thread) + ECHO ("number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); + LLIST_WHILE_HEAD(&threadpool.pool[i].list, thread) lumiera_thread_delete((LumieraThread)thread); ECHO ("destroying the pool mutex"); - lumiera_mutex_destroy (&threadpool.kind[i].lock, &NOBUG_FLAG (threadpool)); + lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); ECHO ("pool mutex destroyed"); } } @@ -90,21 +90,21 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, LumieraThread ret; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - REQUIRE (&threadpool.kind[kind].pool, "threadpool kind %d does not exist", kind); - if (llist_is_empty (&threadpool.kind[kind].pool)) + REQUIRE (&threadpool.pool[kind].list, "threadpool kind %d does not exist", kind); + if (llist_is_empty (&threadpool.pool[kind].list)) { // TODO: fill in the reccondition argument, currently NULL FIXME ("this max thread logic needs to be deeply thought about and made more efficient as well as rebust"); - if (threadpool.kind[kind].working_thread_count - + threadpool.kind[kind].idle_thread_count - < threadpool.kind[kind].max_threads) { + if (threadpool.pool[kind].working_thread_count + + threadpool.pool[kind].idle_thread_count + < threadpool.pool[kind].max_threads) { ret = lumiera_thread_new (kind, NULL, purpose, flag); - threadpool.kind[kind].working_thread_count++; + threadpool.pool[kind].working_thread_count++; ENSURE (ret, "did not create a valid thread"); } else { - //ERROR (threadpool, "did not create a new thread because per-pool limit was reached: %d", threadpool.kind[kind].max_threads); + //ERROR (threadpool, "did not create a new thread because per-pool limit was reached: %d", threadpool.pool[kind].max_threads); LUMIERA_DIE(ERRNO); } } @@ -112,16 +112,16 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, { // use an existing thread, pick the first one // remove it from the pool's list - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[kind].lock) + LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[kind].lock) { - ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.kind[kind].pool))); - threadpool.kind[kind].working_thread_count++; - threadpool.kind[kind].idle_thread_count--; // cheaper than using llist_count - ENSURE (threadpool.kind[kind].idle_thread_count == - llist_count(&threadpool.kind[kind].pool), + ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.pool[kind].list))); + threadpool.pool[kind].working_thread_count++; + threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count + ENSURE (threadpool.pool[kind].idle_thread_count == + llist_count(&threadpool.pool[kind].list), "idle thread count %d is wrong, should be %d", - threadpool.kind[kind].idle_thread_count, - llist_count(&threadpool.kind[kind].pool)); + threadpool.pool[kind].idle_thread_count, + llist_count(&threadpool.pool[kind].list)); } ENSURE (ret, "did not find a valid thread"); } @@ -133,21 +133,21 @@ lumiera_threadpool_release_thread(LumieraThread thread) { REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - REQUIRE (&threadpool.kind[thread->kind].lock, "invalid threadpool lock"); + REQUIRE (&threadpool.pool[thread->kind].lock, "invalid threadpool lock"); // TOOD: currently, locking produces memory leaks - // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.kind[thread->kind].lock) + // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) // { REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); - llist_insert_head(&threadpool.kind[thread->kind].pool, &thread->node); - threadpool.kind[thread->kind].working_thread_count--; - threadpool.kind[thread->kind].idle_thread_count++; // cheaper than using llist_count - ENSURE (threadpool.kind[thread->kind].idle_thread_count == - llist_count(&threadpool.kind[thread->kind].pool), + llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); + threadpool.pool[thread->kind].working_thread_count--; + threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count + ENSURE (threadpool.pool[thread->kind].idle_thread_count == + llist_count(&threadpool.pool[thread->kind].list), "idle thread count %d is wrong, should be %d", - threadpool.kind[thread->kind].idle_thread_count, - llist_count(&threadpool.kind[thread->kind].pool)); - // REQUIRE (!llist_is_empty (&threadpool.kind[thread->kind].pool), "thread pool is still empty after insertion"); + threadpool.pool[thread->kind].idle_thread_count, + llist_count(&threadpool.pool[thread->kind].list)); + // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); // } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index fbd45c8a4..b978e9e57 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -69,12 +69,12 @@ struct lumiera_threadpool_struct { struct { - llist pool; + llist list; lumiera_mutex lock; unsigned max_threads; unsigned working_thread_count; unsigned idle_thread_count; - } kind[LUMIERA_THREADCLASS_COUNT]; + } pool[LUMIERA_THREADCLASS_COUNT]; }; /** From 01223ef6ba927a8c4d40f2d40c604ec538f6bc49 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 26 Nov 2009 11:45:34 -0500 Subject: [PATCH 20/68] kind vs class naming rationale --- src/backend/threads.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/threads.h b/src/backend/threads.h index 3a45bc5e5..4c15a8ad8 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -104,6 +104,9 @@ struct lumiera_thread_struct // void* arg; pthread_t id; LumieraReccondition finished; + // the following member could have been called "class" except that it would conflict with C++ keyword + // as consequence, it's been decided to leave the type name containing the word "class", + // while all members/variables called "kind" enum lumiera_thread_class kind; lumiera_thread_state state; }; From c31bc2791b3c8f4fe5e20b3f5e001b4216205aff Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 26 Nov 2009 11:52:19 -0500 Subject: [PATCH 21/68] remove unnecessary REQUIREs based on my new understanding of nobug --- src/backend/threadpool.c | 2 -- src/backend/threads.c | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index e5273d4c0..ece74d92e 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -90,7 +90,6 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, LumieraThread ret; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - REQUIRE (&threadpool.pool[kind].list, "threadpool kind %d does not exist", kind); if (llist_is_empty (&threadpool.pool[kind].list)) { // TODO: fill in the reccondition argument, currently NULL @@ -133,7 +132,6 @@ lumiera_threadpool_release_thread(LumieraThread thread) { REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - REQUIRE (&threadpool.pool[thread->kind].lock, "invalid threadpool lock"); // TOOD: currently, locking produces memory leaks // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) diff --git a/src/backend/threads.c b/src/backend/threads.c index f4277fd47..5d849943f 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -163,11 +163,11 @@ lumiera_thread_new (enum lumiera_thread_class kind, // TODO: do something with these: (void) purpose; (void) flag; + REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); if (attr_once == PTHREAD_ONCE_INIT) pthread_once (&attr_once, thread_attr_init); - REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); //REQUIRE (finished, "invalid finished flag passed"); LumieraThread self = lumiera_malloc (sizeof (*self)); @@ -176,10 +176,8 @@ lumiera_thread_new (enum lumiera_thread_class kind, self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; - REQUIRE (&self->id); //REQUIRE (&attrs); //REQUIRE (&thread_loop); - REQUIRE (self); int error = pthread_create (&self->id, &attrs, &thread_loop, self); ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); if (error) @@ -188,8 +186,6 @@ lumiera_thread_new (enum lumiera_thread_class kind, FIXME ("error is %d:%s, see if this can be improved", error, strerror(error)); LUMIERA_DIE (ERRNO); } - - REQUIRE (self, "returning an invalid thread structure"); return self; } From 98519c57dad86d78632ab9d27a9fea03dfc49e34 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Fri, 27 Nov 2009 11:39:23 -0500 Subject: [PATCH 22/68] reduce the number of threads spawned in tests to avoid EAGAIN errors from pthread_create (at least on my 2.6.31.5-127.fc12.x86_64 Core2Duo T9600 system) --- tests/30backend-threadpool.tests | 10 +++++----- tests/backend/test-threadpool.c | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index a92b32059..ca2464bfe 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -48,31 +48,31 @@ TEST "Many Acquires/Releases test" many-acquire-release < Date: Sun, 29 Nov 2009 17:55:20 -0500 Subject: [PATCH 23/68] convert lumiera_thread_class to using the enum string trick and use it in a test --- src/backend/threads.c | 7 +++++++ src/backend/threads.h | 31 +++++++++++++++++++++---------- tests/30backend-threadpool.tests | 2 ++ tests/backend/test-threadpool.c | 6 +++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 5d849943f..4fe0c8ac5 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -46,6 +46,13 @@ NOBUG_DEFINE_FLAG_PARENT (threads, threads_dbg); /*TODO insert a suitable/better //code goes here// +#define LUMIERA_THREAD_CLASS(name) #name, +// enum string trick: expands as an array of thread class name strings +const char* lumiera_threadclass_names[] = { + LUMIERA_THREAD_CLASSES +}; + +#undef LUMIERA_THREAD_CLASS struct lumiera_thread_mockup { diff --git a/src/backend/threads.h b/src/backend/threads.h index 4c15a8ad8..ce9a7605b 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -46,6 +46,21 @@ typedef struct lumiera_thread_struct lumiera_thread; typedef lumiera_thread* LumieraThread; +// this is used for an enum string trick +#define LUMIERA_THREAD_CLASSES \ + /** mostly idle, low latency **/ \ + LUMIERA_THREAD_CLASS(INTERACTIVE) \ + /** busy at average priority **/ \ + LUMIERA_THREAD_CLASS(WORKER) \ + /** busy, soft realtime, high priority **/ \ + LUMIERA_THREAD_CLASS(URGENT) \ + /** high latency, background jobs **/ \ + LUMIERA_THREAD_CLASS(BATCH) \ + /** Something to do when there is really nothing else to do **/ \ + LUMIERA_THREAD_CLASS(IDLE) + +// enum string trick: expands as an enum of thread classes +#define LUMIERA_THREAD_CLASS(name) LUMIERA_THREADCLASS_##name, /** * Thread classes. @@ -54,16 +69,7 @@ typedef lumiera_thread* LumieraThread; */ enum lumiera_thread_class { - /** mostly idle, low latency **/ - LUMIERA_THREADCLASS_INTERACTIVE, - /** busy at average priority **/ - LUMIERA_THREADCLASS_WORKER, - /** busy, soft realtime, high priority **/ - LUMIERA_THREADCLASS_URGENT, - /** high latency, background jobs **/ - LUMIERA_THREADCLASS_BATCH, - /** Something to do when there is really nothing else to do **/ - LUMIERA_THREADCLASS_IDLE, + LUMIERA_THREAD_CLASSES /** this just denotes the number of classes listed above, it is used to create arrays **/ LUMIERA_THREADCLASS_COUNT, @@ -79,6 +85,11 @@ enum lumiera_thread_class LUMIERA_THREAD_OR_NOT = 1<<16 }; +#undef LUMIERA_THREAD_CLASS + +// defined in threads.c +extern const char* lumiera_threadclass_names[]; + /** * Thread state. * These are the only states our threads can be in. diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index ca2464bfe..e4f4e4005 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -9,6 +9,8 @@ TEST "Acquire/Release test" basic-acquire-release <kind); + ECHO("thread 1 kind=%s", lumiera_threadclass_names[t1->kind]); CHECK(LUMIERA_THREADCLASS_INTERACTIVE == t1->kind); //ECHO("thread 1 state=%d", t1->state); CHECK(LUMIERA_THREADSTATE_IDLE == t1->state); - //ECHO("thread 2 kind=%d", t2->kind); - CHECK(LUMIERA_THREADCLASS_IDLE == t2->kind); + ECHO("thread 2 kind=%s", lumiera_threadclass_names[t2->kind]); + CHECK(LUMIERA_THREADCLASS_IDLE == t2->kind); //ECHO("thread 2 state=%d", t2->state); CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); From eedfafb0d07f3c17756d8d8c9c9c3e7177489e1b Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sun, 29 Nov 2009 18:06:14 -0500 Subject: [PATCH 24/68] convert lumiera_thread_state to using the enum string trick and use it in a test --- src/backend/threads.c | 6 ++++++ src/backend/threads.h | 16 +++++++++++++--- tests/30backend-threadpool.tests | 2 ++ tests/backend/test-threadpool.c | 4 ++-- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 4fe0c8ac5..b943d55fc 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -54,6 +54,12 @@ const char* lumiera_threadclass_names[] = { #undef LUMIERA_THREAD_CLASS +#define LUMIERA_THREAD_STATE(name) #name, +const char* lumiera_threadstate_names[] = { + LUMIERA_THREAD_STATES +}; +#undef LUMIERA_THREAD_STATE + struct lumiera_thread_mockup { void (*fn)(void*); diff --git a/src/backend/threads.h b/src/backend/threads.h index ce9a7605b..416e160a0 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -90,18 +90,28 @@ enum lumiera_thread_class // defined in threads.c extern const char* lumiera_threadclass_names[]; +#define LUMIERA_THREAD_STATES \ + LUMIERA_THREAD_STATE(IDLE) \ + LUMIERA_THREAD_STATE(RUNNING) \ + LUMIERA_THREAD_STATE(ERROR) + +#define LUMIERA_THREAD_STATE(name) LUMIERA_THREADSTATE_##name, + /** * Thread state. * These are the only states our threads can be in. */ typedef enum { - LUMIERA_THREADSTATE_IDLE, - LUMIERA_THREADSTATE_RUNNING, - LUMIERA_THREADSTATE_ERROR + LUMIERA_THREAD_STATES } lumiera_thread_state; +#undef LUMIERA_THREAD_STATE + +// defined in threads.c +extern const char* lumiera_threadstate_names[]; + #include "threadpool.h" /** diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index e4f4e4005..c136aff42 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -10,7 +10,9 @@ err: start by initializing the threadpool err: acquiring thread 1 err: acquiring thread 2 err: thread 1 kind=INTERACTIVE +err: thread 1 state=IDLE err: thread 2 kind=IDLE +err: thread 2 state=IDLE err: releasing thread 1 err: thread 1 has been released err: releasing thread 2 diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index 0a4319bfb..0c516e306 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -46,11 +46,11 @@ TEST ("basic-acquire-release") ECHO("thread 1 kind=%s", lumiera_threadclass_names[t1->kind]); CHECK(LUMIERA_THREADCLASS_INTERACTIVE == t1->kind); - //ECHO("thread 1 state=%d", t1->state); + ECHO("thread 1 state=%s", lumiera_threadstate_names[t1->state]); CHECK(LUMIERA_THREADSTATE_IDLE == t1->state); ECHO("thread 2 kind=%s", lumiera_threadclass_names[t2->kind]); CHECK(LUMIERA_THREADCLASS_IDLE == t2->kind); - //ECHO("thread 2 state=%d", t2->state); + ECHO("thread 2 state=%s", lumiera_threadstate_names[t2->state]); CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); ECHO("releasing thread 1"); From afe135f43e9e89a7eaea022294705f99edf40677 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 2 Dec 2009 13:48:08 -0500 Subject: [PATCH 25/68] add a lock around thread pool list access in lumiera_threadpool_release_thread() --- src/backend/threadpool.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index ece74d92e..7c7d48fd8 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -133,9 +133,8 @@ lumiera_threadpool_release_thread(LumieraThread thread) REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - // TOOD: currently, locking produces memory leaks - // LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) - // { + LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) + { REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); threadpool.pool[thread->kind].working_thread_count--; @@ -146,7 +145,7 @@ lumiera_threadpool_release_thread(LumieraThread thread) threadpool.pool[thread->kind].idle_thread_count, llist_count(&threadpool.pool[thread->kind].list)); // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); - // } + } } /* From 098d801d00e87ecea9590ada88b2571a30d9c9f9 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 2 Dec 2009 22:21:49 -0500 Subject: [PATCH 26/68] give each thread pool a separate pthread_attr_t structure to configure --- src/backend/threadpool.c | 10 +++++++++- src/backend/threadpool.h | 1 + src/backend/threads.c | 23 ++++++----------------- src/backend/threads.h | 3 ++- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 7c7d48fd8..66ecd4e33 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -61,6 +61,12 @@ lumiera_threadpool_init(unsigned limit) threadpool.pool[i].max_threads = limit; threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; + + //TODO: configure each pools' pthread_attrs appropriately + pthread_attr_init (&threadpool.pool[i].pthread_attrs); + pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); + //cancel... + lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); } } @@ -79,6 +85,7 @@ lumiera_threadpool_destroy(void) ECHO ("destroying the pool mutex"); lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); ECHO ("pool mutex destroyed"); + pthread_attr_destroy (&threadpool.pool[i].pthread_attrs); } } @@ -97,7 +104,8 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, if (threadpool.pool[kind].working_thread_count + threadpool.pool[kind].idle_thread_count < threadpool.pool[kind].max_threads) { - ret = lumiera_thread_new (kind, NULL, purpose, flag); + ret = lumiera_thread_new (kind, NULL, purpose, flag, + &threadpool.pool[kind].pthread_attrs); threadpool.pool[kind].working_thread_count++; ENSURE (ret, "did not create a valid thread"); } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index b978e9e57..03b56a4fe 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -74,6 +74,7 @@ struct lumiera_threadpool_struct unsigned max_threads; unsigned working_thread_count; unsigned idle_thread_count; + pthread_attr_t pthread_attrs; } pool[LUMIERA_THREADCLASS_COUNT]; }; diff --git a/src/backend/threads.c b/src/backend/threads.c index b943d55fc..62f0d14a6 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -93,16 +93,6 @@ static void* pthread_runner (void* thread) } #endif -static pthread_once_t attr_once = PTHREAD_ONCE_INIT; -static pthread_attr_t attrs; - -static void thread_attr_init (void) -{ - pthread_attr_init (&attrs); - pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED); - //cancel ... -} - #if 0 // TODO: rewrite this using lumiera_threadpool_acquire() LumieraThread @@ -171,27 +161,26 @@ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, LumieraReccondition finished, const char* purpose, - struct nobug_flag* flag) + struct nobug_flag* flag, + pthread_attr_t* attrs) { // TODO: do something with these: (void) purpose; (void) flag; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); - - if (attr_once == PTHREAD_ONCE_INIT) - pthread_once (&attr_once, thread_attr_init); + REQUIRE (attrs, "invalid pthread attributes structure passed"); //REQUIRE (finished, "invalid finished flag passed"); + LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); self->finished = finished; self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; - //REQUIRE (&attrs); - //REQUIRE (&thread_loop); - int error = pthread_create (&self->id, &attrs, &thread_loop, self); + //REQUIRE (thread_loop); + int error = pthread_create (&self->id, attrs, &thread_loop, self); ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); if (error) { diff --git a/src/backend/threads.h b/src/backend/threads.h index 416e160a0..e8b774299 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -139,7 +139,8 @@ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, LumieraReccondition finished, const char* purpose, - struct nobug_flag* flag); + struct nobug_flag* flag, + pthread_attr_t* attrs); LumieraThread lumiera_thread_destroy (LumieraThread self); From 41bb8b7e78b8df8f6724ce1b78edb0059d87324a Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 3 Dec 2009 09:29:56 -0500 Subject: [PATCH 27/68] make a TODO note about the finished condition variable --- src/backend/threads.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/threads.h b/src/backend/threads.h index e8b774299..ac8a163fd 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -124,6 +124,7 @@ struct lumiera_thread_struct // void (*function)(void*); // void* arg; pthread_t id; + // TODO: maybe this condition variable should be renamed when we have a better understanding of how it will be used LumieraReccondition finished; // the following member could have been called "class" except that it would conflict with C++ keyword // as consequence, it's been decided to leave the type name containing the word "class", From 5c7abbded99585d7334f075f57c1b43437b3909a Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Thu, 17 Dec 2009 03:49:02 +0100 Subject: [PATCH 28/68] Fix: race conditions with the nobug resource tracker * requires new nobug version * 40components.test "Type-based contexts" TypedCounter_test hangs for unknown reason, temporary disabled --- src/lib/condition.h | 40 ++++++++------- src/lib/mutex.h | 24 +++++---- src/lib/reccondition.h | 34 +++++++------ src/lib/rwlock.h | 105 ++++++++++++++++++++------------------- src/lib/sectionlock.h | 10 ++-- tests/40components.tests | 4 +- 6 files changed, 115 insertions(+), 102 deletions(-) diff --git a/src/lib/condition.h b/src/lib/condition.h index f90d079d8..efb3136ab 100644 --- a/src/lib/condition.h +++ b/src/lib/condition.h @@ -52,11 +52,12 @@ ({ \ lumiera_cond_section_.lock = (cnd); \ NOBUG_IF_ALPHA(lumiera_cond_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (cnd)->rh, "acquire condmutex", \ - NOBUG_RESOURCE_WAITING, lumiera_cond_section_.rh); \ - if (pthread_mutex_lock (&(cnd)->cndmutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ + RESOURCE_WAIT (nobugflag, (cnd)->rh, "acquire condmutex", lumiera_cond_section_.rh) \ + { \ + if (pthread_mutex_lock (&(cnd)->cndmutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ + } \ }); \ lumiera_cond_section_.lock; \ ({ \ @@ -74,12 +75,13 @@ REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ lumiera_cond_section_.lock = cnd; \ NOBUG_IF_ALPHA(lumiera_cond_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (cnd)->rh, "acquire condmutex", \ - NOBUG_RESOURCE_WAITING, lumiera_cond_section_.rh); \ - if (pthread_mutex_lock (&(cnd)->cndmutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + RESOURCE_ENTER (nobugflag, (cnd)->rh, "acquire condmutex", lumiera_cond_section_.rh) \ + { \ + if (pthread_mutex_lock (&(cnd)->cndmutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ + LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + } \ }); \ lumiera_cond_section_.lock; \ ({ \ @@ -97,13 +99,15 @@ * Must be used inside a CONDITION_SECTION. * @param expr Conditon which must become true, else the condition variable goes back into sleep */ -#define LUMIERA_CONDITION_WAIT(expr) \ - do { \ - REQUIRE (lumiera_cond_section_.lock, "Condition mutex not locked"); \ - NOBUG_RESOURCE_STATE_RAW (lumiera_cond_section_.flag, NOBUG_RESOURCE_WAITING, lumiera_cond_section_.rh); \ - pthread_cond_wait (&((LumieraCondition)lumiera_cond_section_.lock)->cond, \ - &((LumieraCondition)lumiera_cond_section_.lock)->cndmutex); \ - NOBUG_RESOURCE_STATE_RAW (lumiera_cond_section_.flag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ +#define LUMIERA_CONDITION_WAIT(expr) \ + do { \ + REQUIRE (lumiera_cond_section_.lock, "Condition mutex not locked"); \ + NOBUG_RESOURCE_STATE_RAW (lumiera_cond_section_.flag, NOBUG_RESOURCE_WAITING, lumiera_cond_section_.rh) \ + { \ + pthread_cond_wait (&((LumieraCondition)lumiera_cond_section_.lock)->cond, \ + &((LumieraCondition)lumiera_cond_section_.lock)->cndmutex); \ + NOBUG_RESOURCE_STATE_RAW (lumiera_cond_section_.flag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_cond_section_.rh); \ + } \ } while (!(expr)) diff --git a/src/lib/mutex.h b/src/lib/mutex.h index cead7dec0..1a85d970a 100644 --- a/src/lib/mutex.h +++ b/src/lib/mutex.h @@ -46,11 +46,12 @@ ({ \ lumiera_lock_section_.lock = (mtx); \ NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (mtx)->rh, "acquire mutex", \ - NOBUG_RESOURCE_WAITING, lumiera_lock_section_.rh); \ - if (pthread_mutex_lock (&(mtx)->mutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ + RESOURCE_WAIT (nobugflag, (mtx)->rh, "acquire mutex", lumiera_lock_section_.rh) \ + { \ + if (pthread_mutex_lock (&(mtx)->mutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ + } \ }); \ lumiera_lock_section_.lock; \ ({ \ @@ -76,12 +77,13 @@ REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ lumiera_lock_section_.lock = mtx; \ NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (mtx)->rh, "acquire mutex", \ - NOBUG_RESOURCE_WAITING, lumiera_lock_section_.rh); \ - if (pthread_mutex_lock (&(mtx)->mutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + RESOURCE_WAIT (nobugflag, (mtx)->rh, "acquire mutex", lumiera_lock_section_.rh) \ + { \ + if (pthread_mutex_lock (&(mtx)->mutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ + LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + } \ }); \ lumiera_lock_section_.lock; \ ({ \ diff --git a/src/lib/reccondition.h b/src/lib/reccondition.h index 8ba3479ac..f9bcd59d1 100644 --- a/src/lib/reccondition.h +++ b/src/lib/reccondition.h @@ -50,11 +50,12 @@ ({ \ lumiera_reccond_section_.lock = (cnd); \ NOBUG_IF_ALPHA(lumiera_reccond_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (cnd)->rh, "acquire reccondmutex", \ - NOBUG_RESOURCE_WAITING, lumiera_reccond_section_.rh); \ - if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ + RESOURCE_WAIT (nobugflag, (cnd)->rh, "acquire reccondmutex", lumiera_reccond_section_.rh) \ + { \ + if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ + } \ }); \ lumiera_reccond_section_.lock; \ ({ \ @@ -73,12 +74,13 @@ REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ lumiera_reccond_section_.lock = (cnd); \ NOBUG_IF_ALPHA(lumiera_reccond_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (cnd)->rh, "acquire reccondmutex", \ - NOBUG_RESOURCE_WAITING, lumiera_reccond_section_.rh); \ - if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_) \ + RESOURCE_WAIT (nobugflag, (cnd)->rh, "acquire reccondmutex", lumiera_reccond_section_.rh) \ + { \ + if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ + LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + } \ }); \ lumiera_reccond_section_.lock; \ ({ \ @@ -98,10 +100,12 @@ #define LUMIERA_RECCONDITION_WAIT(expr) \ do { \ REQUIRE (lumiera_reccond_section_.lock, "Condition recmutex not locked"); \ - NOBUG_RESOURCE_STATE_RAW (lumiera_reccond_section_.flag, NOBUG_RESOURCE_WAITING, lumiera_reccond_section_.rh); \ - pthread_cond_wait (&((LumieraReccondition)lumiera_reccond_section_.lock)->cond, \ - &((LumieraReccondition)lumiera_reccond_section_.lock)->reccndmutex); \ - NOBUG_RESOURCE_STATE_RAW (lumiera_reccond_section_.flag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ + NOBUG_RESOURCE_STATE_RAW (lumiera_reccond_section_.flag, NOBUG_RESOURCE_WAITING, lumiera_reccond_section_.rh) \ + { \ + pthread_cond_wait (&((LumieraReccondition)lumiera_reccond_section_.lock)->cond, \ + &((LumieraReccondition)lumiera_reccond_section_.lock)->reccndmutex); \ + NOBUG_RESOURCE_STATE_RAW (lumiera_reccond_section_.flag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ + } \ } while (!(expr)) diff --git a/src/lib/rwlock.h b/src/lib/rwlock.h index 1e72347cd..2cd8d006a 100644 --- a/src/lib/rwlock.h +++ b/src/lib/rwlock.h @@ -54,13 +54,12 @@ LUMIERA_ERROR_DECLARE(RWLOCK_WRLOCK); ({ \ lumiera_lock_section_.lock = (rwlck); \ NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (rwlck)->rh, "acquire readlock", \ - NOBUG_RESOURCE_WAITING, \ - lumiera_lock_section_.rh); \ - if (pthread_rwlock_rdlock (&(rwlck)->rwlock)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - TODO ("implement NOBUG_RESOURCE_SHARED"); \ - /*RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_SHARED, lumiera_lock_section_.rh); */ \ + RESOURCE_WAIT (nobugflag, (rwlck)->rh, "acquire readlock", lumiera_lock_section_.rh) \ + { \ + if (pthread_rwlock_rdlock (&(rwlck)->rwlock)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_SHARED, lumiera_lock_section_.rh); \ + } \ }); \ lumiera_lock_section_.lock; \ ({ \ @@ -78,12 +77,13 @@ LUMIERA_ERROR_DECLARE(RWLOCK_WRLOCK); REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ lumiera_lock_section_.lock = (rwlck); \ NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (rwlck)->rh, "acquire readlock", \ - NOBUG_RESOURCE_WAITING, lumiera_lock_section_.rh); \ - if (pthread_rwlock_rdlock (&(rwlck)->rwlock)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - /*RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_SHARED, lumiera_lock_section_.rh); */ \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + RESOURCE_WAIT (nobugflag, (rwlck)->rh, "acquire readlock", lumiera_lock_section_.rh) \ + { \ + if (pthread_rwlock_rdlock (&(rwlck)->rwlock)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_SHARED, lumiera_lock_section_.rh); \ + LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + } \ }); \ lumiera_lock_section_.lock; \ ({ \ @@ -96,48 +96,49 @@ LUMIERA_ERROR_DECLARE(RWLOCK_WRLOCK); /** * Write locked section. */ -#define LUMIERA_WRLOCK_SECTION(nobugflag, rwlck) \ - for (lumiera_sectionlock NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) \ - lumiera_lock_section_ = { \ - (void*)1, lumiera_rwlock_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ - lumiera_lock_section_.lock;) \ - for ( \ - ({ \ - lumiera_lock_section_.lock = (rwlck); \ - NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (rwlck)->rh, "acquire writelock", \ - NOBUG_RESOURCE_WAITING, \ - lumiera_lock_section_.rh); \ - if (pthread_rwlock_wrlock (&(rwlck)->rwlock)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ - }); \ - lumiera_lock_section_.lock; \ - ({ \ - LUMIERA_RWLOCK_SECTION_UNLOCK; \ +#define LUMIERA_WRLOCK_SECTION(nobugflag, rwlck) \ + for (lumiera_sectionlock NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) \ + lumiera_lock_section_ = { \ + (void*)1, lumiera_rwlock_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ + lumiera_lock_section_.lock;) \ + for ( \ + ({ \ + lumiera_lock_section_.lock = (rwlck); \ + NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ + RESOURCE_WAIT (nobugflag, (rwlck)->rh, "acquire writelock", lumiera_lock_section_.rh) \ + { \ + if (pthread_rwlock_wrlock (&(rwlck)->rwlock)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ + } \ + }); \ + lumiera_lock_section_.lock; \ + ({ \ + LUMIERA_RWLOCK_SECTION_UNLOCK; \ })) -#define LUMIERA_WRLOCK_SECTION_CHAIN(nobugflag, rwlck) \ - for (lumiera_sectionlock *lumiera_lock_section_old_ = &lumiera_lock_section_, \ - NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) lumiera_lock_section_ = { \ - (void*)1, lumiera_rwlock_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ - lumiera_lock_section_.lock;) \ - for ( \ - ({ \ - REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ - lumiera_lock_section_.lock = (rwlck); \ - NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ - RESOURCE_ENTER (nobugflag, (rwlck)->rh, "acquire writelock", \ - NOBUG_RESOURCE_WAITING, lumiera_lock_section_.rh); \ - if (pthread_rwlock_wrlock (&(twlck)->rwlock)) \ - LUMIERA_DIE (LOCK_ACQUIRE); \ - RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ - }); \ - lumiera_lock_section_.lock; \ - ({ \ - LUMIERA_MUTEX_SECTION_UNLOCK; \ +#define LUMIERA_WRLOCK_SECTION_CHAIN(nobugflag, rwlck) \ + for (lumiera_sectionlock *lumiera_lock_section_old_ = &lumiera_lock_section_, \ + NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) lumiera_lock_section_ = { \ + (void*)1, lumiera_rwlock_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ + lumiera_lock_section_.lock;) \ + for ( \ + ({ \ + REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ + lumiera_lock_section_.lock = (rwlck); \ + NOBUG_IF_ALPHA(lumiera_lock_section_.flag = &NOBUG_FLAG(nobugflag);) \ + RESOURCE_WAIT (nobugflag, (rwlck)->rh, "acquire writelock", lumiera_lock_section_.rh) \ + { \ + if (pthread_rwlock_wrlock (&(twlck)->rwlock)) \ + LUMIERA_DIE (LOCK_ACQUIRE); \ + RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_EXCLUSIVE, lumiera_lock_section_.rh); \ + LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + } \ + }); \ + lumiera_lock_section_.lock; \ + ({ \ + LUMIERA_MUTEX_SECTION_UNLOCK; \ })) diff --git a/src/lib/sectionlock.h b/src/lib/sectionlock.h index c50ecfc8d..975a5fbc2 100644 --- a/src/lib/sectionlock.h +++ b/src/lib/sectionlock.h @@ -65,10 +65,12 @@ lumiera_sectionlock_ensureunlocked (LumieraSectionlock self) #define LUMIERA_SECTION_UNLOCK_(section) \ do if ((section)->lock) \ { \ - if ((section)->unlock((section)->lock)) \ - LUMIERA_DIE (LOCK_RELEASE); \ - (section)->lock = NULL; \ - NOBUG_RESOURCE_LEAVE_RAW((section)->flag, (section)->rh); \ + NOBUG_RESOURCE_LEAVE_RAW((section)->flag, (section)->rh) \ + { \ + if ((section)->unlock((section)->lock)) \ + LUMIERA_DIE (LOCK_RELEASE); \ + (section)->lock = NULL; \ + } \ } while (0) diff --git a/tests/40components.tests b/tests/40components.tests index 01d959a34..7475363bd 100644 --- a/tests/40components.tests +++ b/tests/40components.tests @@ -857,8 +857,8 @@ TEST "TypedAllocationManager" TypedAllocationManager_test < Date: Fri, 18 Dec 2009 03:14:47 +0100 Subject: [PATCH 29/68] Sectionlock needs a 'user' handle with new nobug --- src/lib/sectionlock.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/sectionlock.h b/src/lib/sectionlock.h index 975a5fbc2..42e719de7 100644 --- a/src/lib/sectionlock.h +++ b/src/lib/sectionlock.h @@ -42,7 +42,7 @@ struct lumiera_sectionlock_struct void* lock; lumiera_sectionlock_unlock_fn unlock; NOBUG_IF_ALPHA(struct nobug_flag* flag); - RESOURCE_HANDLE (rh); + RESOURCE_USER (rh); }; typedef struct lumiera_sectionlock_struct lumiera_sectionlock; typedef struct lumiera_sectionlock_struct* LumieraSectionlock; From 24e87c815ddf6522ccbe8af87b7f722c3a6f62e1 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 17 Dec 2009 22:08:57 -0500 Subject: [PATCH 30/68] remove old functions pthread_runner and lumiera_thread_run --- src/backend/threads.c | 53 ------------------------------------------- 1 file changed, 53 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 62f0d14a6..0520de288 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -73,59 +73,6 @@ static void* thread_loop (void* arg) return 0; } -#if 0 -static void* pthread_runner (void* thread) -{ - pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); - - struct lumiera_thread_mockup* starter = (struct lumiera_thread_mockup*) thread; - LumieraReccondition thread_end_notification = starter->finished; - - starter->fn (starter->arg); - - if (!thread_end_notification) - return NULL; // no signalling of thread termination desired - - LUMIERA_RECCONDITION_SECTION(cond_sync, thread_end_notification) - LUMIERA_RECCONDITION_BROADCAST; - - return NULL; -} -#endif - -#if 0 -// TODO: rewrite this using lumiera_threadpool_acquire() -LumieraThread -lumiera_thread_run (enum lumiera_thread_class kind, - void (*start_routine)(void *), - void * arg, - LumieraReccondition finished, - const char* purpose, - struct nobug_flag* flag) -{ - (void) kind; - (void) purpose; - (void) flag; - - if (attr_once == PTHREAD_ONCE_INIT) - pthread_once (&attr_once, thread_attr_init); - - static struct lumiera_thread_mockup thread; - - thread.fn = start_routine; - thread.arg = arg; - thread.finished = finished; - - pthread_t dummy; - int error = pthread_create (&dummy, &attrs, pthread_runner, &thread); - - if (error) return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? - return (LumieraThread) 1; -} -#endif - -// TODO: new implementation, remove the above one -// maybe this shouldn't return LumieraThread at all // when this is called it should have already been decided that the function // shall run in parallel, as a thread LumieraThread From f2406c23a1ffda8a2938690ff389bf8660ab7b5e Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 23 Dec 2009 13:10:31 -0500 Subject: [PATCH 31/68] bork bork bork by pulling this you agree to... --- src/backend/thread-wrapper.hpp | 2 +- src/backend/threadpool.c | 96 ++++++++++++++++++-------------- src/backend/threadpool.h | 17 ++++-- src/backend/threads.c | 81 ++++++++++++++++++--------- src/backend/threads.h | 23 +++++--- tests/30backend-threadpool.tests | 63 +++------------------ tests/backend/test-threadpool.c | 70 +++++++++++++++++++++-- tests/backend/test-threads.c | 3 - tests/test.sh | 2 +- 9 files changed, 208 insertions(+), 149 deletions(-) diff --git a/src/backend/thread-wrapper.hpp b/src/backend/thread-wrapper.hpp index 3f897b582..f8ec2bc87 100644 --- a/src/backend/thread-wrapper.hpp +++ b/src/backend/thread-wrapper.hpp @@ -177,10 +177,10 @@ namespace backend { lumiera_thread_run ( kind , &run // invoking the run helper and.. , this // passing this start context as parameter - , joinCond // maybe wait-blocking for the thread to terminate , purpose.c() , logging_flag ); + (void)joinCond; if (!res) throw lumiera::error::State("failed to create new thread."); diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 66ecd4e33..2a33978d6 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -53,21 +53,22 @@ void* pool_thread_loop(void * arg) } void -lumiera_threadpool_init(unsigned limit) +lumiera_threadpool_init() { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.pool[i].list); - threadpool.pool[i].max_threads = limit; - threadpool.pool[i].working_thread_count = 0; + threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; //TODO: configure each pools' pthread_attrs appropriately pthread_attr_init (&threadpool.pool[i].pthread_attrs); - pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); + // cehteh prefers that threads are joinable by default + //pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); //cancel... lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); + lumiera_reccondition_init (&threadpool.pool[i].signal, "thread-signal", &NOBUG_FLAG(threadpool)); } } @@ -78,10 +79,16 @@ lumiera_threadpool_destroy(void) for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { ECHO ("destroying individual pool #%d", i); - // no locking is done at this point - ECHO ("number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); - LLIST_WHILE_HEAD(&threadpool.pool[i].list, thread) - lumiera_thread_delete((LumieraThread)thread); + LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[i].lock) + { + REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are running", threadpool.pool[i].working_thread_count); + // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list + ECHO ("number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); + LLIST_WHILE_HEAD(&threadpool.pool[i].list, t) + { + lumiera_thread_delete((LumieraThread)t); + } + } ECHO ("destroying the pool mutex"); lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); ECHO ("pool mutex destroyed"); @@ -89,6 +96,15 @@ lumiera_threadpool_destroy(void) } } +void lumiera_threadpool_unlink(LumieraThread thread) +{ + REQUIRE (thread, "invalid thread given"); + REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); + llist_unlink(&thread->node); + ENSURE (llist_is_empty(&thread->node), "failed to unlink the thread"); +} + + LumieraThread lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, const char* purpose, @@ -97,52 +113,45 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, LumieraThread ret; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - if (llist_is_empty (&threadpool.pool[kind].list)) - { - // TODO: fill in the reccondition argument, currently NULL - FIXME ("this max thread logic needs to be deeply thought about and made more efficient as well as rebust"); - if (threadpool.pool[kind].working_thread_count - + threadpool.pool[kind].idle_thread_count - < threadpool.pool[kind].max_threads) { - ret = lumiera_thread_new (kind, NULL, purpose, flag, + LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[kind].signal) + { + if (llist_is_empty (&threadpool.pool[kind].list)) + { + + ret = lumiera_thread_new (kind, purpose, flag, &threadpool.pool[kind].pthread_attrs); - threadpool.pool[kind].working_thread_count++; + threadpool.pool[kind].idle_thread_count++; ENSURE (ret, "did not create a valid thread"); + LUMIERA_RECCONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); } - else - { - //ERROR (threadpool, "did not create a new thread because per-pool limit was reached: %d", threadpool.pool[kind].max_threads); - LUMIERA_DIE(ERRNO); - } - } - else - { - // use an existing thread, pick the first one - // remove it from the pool's list - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[kind].lock) - { - ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.pool[kind].list))); - threadpool.pool[kind].working_thread_count++; - threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count - ENSURE (threadpool.pool[kind].idle_thread_count == - llist_count(&threadpool.pool[kind].list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[kind].idle_thread_count, - llist_count(&threadpool.pool[kind].list)); - } - ENSURE (ret, "did not find a valid thread"); - } - return ret; + // use an existing thread, pick the first one + // remove it from the pool's list + ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.pool[kind].list))); + REQUIRE (ret->state == LUMIERA_THREADSTATE_IDLE, "trying to return a non-idle thread (state=%s)", lumiera_threadstate_names[ret->state]); + threadpool.pool[kind].working_thread_count++; + threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count + ENSURE (threadpool.pool[kind].idle_thread_count == + llist_count(&threadpool.pool[kind].list), + "idle thread count %d is wrong, should be %d", + threadpool.pool[kind].idle_thread_count, + llist_count(&threadpool.pool[kind].list)); + ENSURE (ret, "did not find a valid thread"); + } + return ret; } +// TODO: rename to lumiera_threadpool_park_thread void -lumiera_threadpool_release_thread(LumieraThread thread) +lumiera_threadpool_park_thread(LumieraThread thread) { REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) + REQUIRE (thread->state != LUMIERA_THREADSTATE_IDLE, "trying to park an already idle thread"); + + LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].signal) { + thread->state = LUMIERA_THREADSTATE_IDLE; REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); threadpool.pool[thread->kind].working_thread_count--; @@ -153,6 +162,7 @@ lumiera_threadpool_release_thread(LumieraThread thread) threadpool.pool[thread->kind].idle_thread_count, llist_count(&threadpool.pool[thread->kind].list)); // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + LUMIERA_RECCONDITION_BROADCAST; } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 03b56a4fe..de545876c 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -55,12 +55,12 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, struct nobug_flag* flag); /** - * Release a thread - * This ends up putting a (parked/idle) thread back on the list of an appropriate threadpool. + * Park a thread + * This ends up putting a finished thread back on the list of an appropriate threadpool. * This function doesn't need to be accessible outside of the threadpool implementation. */ void -lumiera_threadpool_release_thread(LumieraThread thread); +lumiera_threadpool_park_thread(LumieraThread thread); typedef struct lumiera_threadpool_struct lumiera_threadpool; typedef lumiera_threadpool* LumieraThreadpool; @@ -71,23 +71,28 @@ struct lumiera_threadpool_struct { llist list; lumiera_mutex lock; - unsigned max_threads; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; + lumiera_reccondition signal; } pool[LUMIERA_THREADCLASS_COUNT]; }; /** * Initialize the thread pool. - * @param limit the maximum number of threads (idle+working) allowed per pool */ void -lumiera_threadpool_init(unsigned limit); +lumiera_threadpool_init(); void lumiera_threadpool_destroy(void); +/** + * Just remove the thread structure from an associated pool list. + */ +void +lumiera_threadpool_unlink(LumieraThread thread); + #endif /* // Local Variables: diff --git a/src/backend/threads.c b/src/backend/threads.c index 0520de288..a75b7f81b 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -67,9 +67,29 @@ struct lumiera_thread_mockup LumieraReccondition finished; }; -static void* thread_loop (void* arg) +static void* thread_loop (void* thread) { - (void)arg; + LumieraThread t = (LumieraThread)thread; + + pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); + + REQUIRE (t, "thread does not exist"); + + ECHO ("entering section 1"); + // this seems to deadlock unexpectedly: + LUMIERA_RECCONDITION_SECTION (threads, &t->signal) + { + do { + // NULL function means: no work to do + if (t->function) + t->function (t->arguments); + lumiera_threadpool_park_thread(t); + LUMIERA_RECCONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); + } while (t->state != LUMIERA_THREADSTATE_SHUTDOWN); + // SHUTDOWN state + + ECHO ("thread quitting"); + } return 0; } @@ -79,22 +99,23 @@ LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*function)(void *), void * arg, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag) { - (void)finished; - (void)function; - (void)arg; + REQUIRE (function, "invalid function"); + // ask the threadpool for a thread (it might create a new one) LumieraThread self = lumiera_threadpool_acquire_thread(kind, purpose, flag); - // TODO: set the function and data to be run - // lumiera_thread_set_func_data (self, start_routine, arg, purpose, flag); + // set the function and data to be run + self->function = function; + self->arguments = arg; // and let it really run (signal the condition var, the thread waits on it) - LUMIERA_RECCONDITION_SECTION(cond_sync, self->finished) - LUMIERA_RECCONDITION_SIGNAL; + self->state = LUMIERA_THREADSTATE_WAKEUP; + ECHO ("entering section 2"); + LUMIERA_RECCONDITION_SECTION(threads, &self->signal) + LUMIERA_RECCONDITION_BROADCAST; // NOTE: example only, add solid error handling! @@ -106,29 +127,25 @@ lumiera_thread_run (enum lumiera_thread_class kind, */ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag, pthread_attr_t* attrs) { // TODO: do something with these: (void) purpose; - (void) flag; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); REQUIRE (attrs, "invalid pthread attributes structure passed"); - //REQUIRE (finished, "invalid finished flag passed"); - - LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); - self->finished = finished; + lumiera_reccondition_init (&self->signal, "thread-control", flag); self->kind = kind; - self->state = LUMIERA_THREADSTATE_IDLE; + self->state = LUMIERA_THREADSTATE_STARTUP; + self->function = NULL; + self->arguments = NULL; - //REQUIRE (thread_loop); int error = pthread_create (&self->id, attrs, &thread_loop, self); - ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); + ENSURE(error == 0 || EAGAIN == error, "pthread_create returned %d:%s", error, strerror(error)); if (error) { // error here can only be EAGAIN, given the above ENSURE @@ -143,12 +160,26 @@ lumiera_thread_destroy (LumieraThread self) { REQUIRE (self, "trying to destroy an invalid thread"); - // TODO: stop the pthread - llist_unlink(&self->node); - //finished = NULL; // or free(finished)? - lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG(threads)); - //kind = 0; - //state = 0; + lumiera_threadpool_unlink(self); + + // get the pthread out of the processing loop + // need to signal to the thread that it should start quitting + // should this be within the section? + LUMIERA_RECCONDITION_SECTION(threads, &self->signal) + { + REQUIRE (self->state == LUMIERA_THREADSTATE_IDLE, "trying to delete a thread in state other than IDLE (%s)", lumiera_threadstate_names[self->state]); + self->state = LUMIERA_THREADSTATE_SHUTDOWN; + self->function = NULL; + self->arguments = NULL; + LUMIERA_RECCONDITION_SIGNAL; + } + + int error = pthread_join(self->id, NULL); + ENSURE (0 == error, "pthread_join returned %d:%s", error, strerror(error)); + + // condition has to be destroyed after joining with the thread + lumiera_reccondition_destroy (&self->signal, &NOBUG_FLAG(threads)); + return self; } diff --git a/src/backend/threads.h b/src/backend/threads.h index ac8a163fd..a55926123 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -90,10 +90,16 @@ enum lumiera_thread_class // defined in threads.c extern const char* lumiera_threadclass_names[]; -#define LUMIERA_THREAD_STATES \ - LUMIERA_THREAD_STATE(IDLE) \ - LUMIERA_THREAD_STATE(RUNNING) \ - LUMIERA_THREAD_STATE(ERROR) +// there is some confusion between the meaning of this +// on one hand it could be used to tell the current state of the thread +// on the other, it is used to tell the thread which state to enter on next iteration +#define LUMIERA_THREAD_STATES \ + LUMIERA_THREAD_STATE(IDLE) \ + LUMIERA_THREAD_STATE(ERROR) \ + LUMIERA_THREAD_STATE(RUNNING) \ + LUMIERA_THREAD_STATE(WAKEUP) \ + LUMIERA_THREAD_STATE(SHUTDOWN) \ + LUMIERA_THREAD_STATE(STARTUP) #define LUMIERA_THREAD_STATE(name) LUMIERA_THREADSTATE_##name, @@ -125,12 +131,15 @@ struct lumiera_thread_struct // void* arg; pthread_t id; // TODO: maybe this condition variable should be renamed when we have a better understanding of how it will be used - LumieraReccondition finished; + lumiera_reccondition signal; // control signal, state change signal // the following member could have been called "class" except that it would conflict with C++ keyword // as consequence, it's been decided to leave the type name containing the word "class", // while all members/variables called "kind" enum lumiera_thread_class kind; + // this is used both as a command and as a state tracker lumiera_thread_state state; + void (*function)(void *); + void * arguments; }; /** @@ -138,7 +147,6 @@ struct lumiera_thread_struct */ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag, pthread_attr_t* attrs); @@ -162,8 +170,6 @@ lumiera_thread_delete (LumieraThread self); * @param kind class of the thread to start * @param function pointer to a function to execute in a thread (returning void, not void* as in pthreads) * @param arg generic pointer passed to the thread - * @param finished a condition variable to be broadcasted, if not NULL. - * The associated mutex should be locked at thread_run time already, else the signal can get lost. * @param purpose descriptive name of this thread, used by NoBug * @param flag NoBug flag used for logging the thread startup and return */ @@ -171,7 +177,6 @@ LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*function)(void *), void * arg, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag); diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index c136aff42..2cad43477 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -5,24 +5,11 @@ PLANNED "create" PLANNED "yield" PLANNED "cancel" -TEST "Acquire/Release test" basic-acquire-release < #include +#include + +void is_prime(void * arg) +{ + int number = *(int *)arg; + int prime = 1; + + for (int x = number; x >= sqrt(number); --x) + { + if (number % x == 0) + { + prime = 0; + break; + } + } + *(int *)arg = prime; +} TESTS_BEGIN +TEST ("threadpool-basic") +{ + lumiera_threadpool_init(100); + lumiera_threadpool_destroy(); +} + +TEST ("threadpool1") +{ + ECHO("start by initializing the threadpool"); + lumiera_threadpool_init(100); + LumieraThread t1 = + lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_INTERACTIVE, + "test purpose", + &NOBUG_FLAG(NOBUG_ON)); + // lumiera_threadpool_release_thread(t1); + ECHO("acquired thread 1 %p",t1); + lumiera_threadpool_destroy(); +} + TEST ("basic-acquire-release") { @@ -37,12 +73,12 @@ TEST ("basic-acquire-release") LumieraThread t1 = lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_INTERACTIVE, "test purpose", - NULL); + &NOBUG_FLAG(NOBUG_ON)); ECHO("acquiring thread 2"); LumieraThread t2 = lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_IDLE, "test purpose", - NULL); + &NOBUG_FLAG(NOBUG_ON)); ECHO("thread 1 kind=%s", lumiera_threadclass_names[t1->kind]); CHECK(LUMIERA_THREADCLASS_INTERACTIVE == t1->kind); @@ -54,16 +90,17 @@ TEST ("basic-acquire-release") CHECK(LUMIERA_THREADSTATE_IDLE == t2->state); ECHO("releasing thread 1"); - lumiera_threadpool_release_thread(t1); + //lumiera_threadpool_release_thread(t1); ECHO("thread 1 has been released"); ECHO("releasing thread 2"); - lumiera_threadpool_release_thread(t2); + //lumiera_threadpool_release_thread(t2); ECHO("thread 2 has been released"); lumiera_threadpool_destroy(); } +#if 0 TEST ("many-acquire-release") { @@ -79,7 +116,7 @@ TEST ("many-acquire-release") threads[i+kind*threads_per_pool_count] = lumiera_threadpool_acquire_thread(kind, "test purpose", - NULL); + &NOBUG_FLAG(NOBUG_ON)); } } @@ -107,7 +144,7 @@ TEST ("toomany-acquire-release") threads[i+kind*threads_per_pool_count] = lumiera_threadpool_acquire_thread(kind, "test purpose", - NULL); + &NOBUG_FLAG(NOBUG_ON)); } } @@ -119,5 +156,26 @@ TEST ("toomany-acquire-release") lumiera_threadpool_destroy(); } +#endif + +TEST ("process-function") +{ + // this is what the scheduler would do once it figures out what function a job needs to run + LumieraThread t; + int number = 440616; + + lumiera_threadpool_init(10); + + ECHO ("the input to the function is %d", number); + + t = lumiera_thread_run (LUMIERA_THREADCLASS_INTERACTIVE, + &is_prime, + (void *)&number, //void * arg, + "process my test function", + &NOBUG_FLAG(NOBUG_ON)); // struct nobug_flag* flag) + + // cleanup + lumiera_threadpool_destroy(); +} TESTS_END diff --git a/tests/backend/test-threads.c b/tests/backend/test-threads.c index 76da2f450..21786ae8b 100644 --- a/tests/backend/test-threads.c +++ b/tests/backend/test-threads.c @@ -91,7 +91,6 @@ TEST ("simple_thread") lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, threadfn, NULL, - NULL, argv[1], NULL); @@ -112,7 +111,6 @@ TEST ("thread_synced") lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, threadsyncfn, &cnd, - &cnd, argv[1], NULL); @@ -143,7 +141,6 @@ TEST ("mutex_thread") lumiera_thread_run (LUMIERA_THREADCLASS_WORKER, mutexfn, NULL, - NULL, argv[1], NULL); diff --git a/tests/test.sh b/tests/test.sh index 951d2fcea..b1fd0d99b 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -26,7 +26,7 @@ # stop testing on the first failure export LC_ALL=C -NOBUG_LOGREGEX='^\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: \(TRACE\|INFO\|NOTICE\|WARNING\|ERR\|TODO\|PLANNED\|FIXME\|DEPRECATED\|UNIMPLEMENTED\):' +NOBUG_LOGREGEX='^\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: \(TRACE\|INFO\|NOTICE\|WARNING\|ERR\|TODO\|PLANNED\|FIXME\|DEPRECATED\|UNIMPLEMENTED\|RESOURCE_ENTER\|RESOURCE_LEAVE\|RESOURCE_STATE\):' arg0="$0" srcdir="$(dirname "$arg0")" From 6f5578ba1002b8e2290e64d98408e7aa82c5dfff Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 23 Dec 2009 18:14:29 -0500 Subject: [PATCH 32/68] fix a copy+paste mistake in LUMIERA_RECCONDITION_SECTION_CHAIN and add a test-case for it --- src/lib/reccondition.h | 6 +++--- tests/15locking.tests | 4 ++++ tests/library/test-locking.c | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/lib/reccondition.h b/src/lib/reccondition.h index f9bcd59d1..51ce6ef07 100644 --- a/src/lib/reccondition.h +++ b/src/lib/reccondition.h @@ -65,13 +65,13 @@ #define LUMIERA_RECCONDITION_SECTION_CHAIN(nobugflag, cnd) \ - for (lumiera_sectionlock *lumiera_lock_section_old_ = &lumiera_lock_section_, \ + for (lumiera_sectionlock *lumiera_reccond_section_old_ = &lumiera_reccond_section_, \ NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) lumiera_reccond_section_ = { \ (void*)1, lumiera_reccondition_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ lumiera_reccond_section_.lock;) \ for ( \ ({ \ - REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ + REQUIRE (lumiera_reccond_section_old_->lock, "section prematurely unlocked"); \ lumiera_reccond_section_.lock = (cnd); \ NOBUG_IF_ALPHA(lumiera_reccond_section_.flag = &NOBUG_FLAG(nobugflag);) \ RESOURCE_WAIT (nobugflag, (cnd)->rh, "acquire reccondmutex", lumiera_reccond_section_.rh) \ @@ -79,7 +79,7 @@ if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ LUMIERA_DIE (LOCK_ACQUIRE); \ RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + LUMIERA_SECTION_UNLOCK_(lumiera_reccond_section_old_); \ } \ }); \ lumiera_reccond_section_.lock; \ diff --git a/tests/15locking.tests b/tests/15locking.tests index 0936499f2..c72f88855 100644 --- a/tests/15locking.tests +++ b/tests/15locking.tests @@ -86,3 +86,7 @@ PLANNED "reccondition broadcasting" < Date: Thu, 24 Dec 2009 01:45:44 +0100 Subject: [PATCH 33/68] remove threadpool_unlink() thread removes itself from the list, this is trival in place --- src/backend/threadpool.c | 8 -------- src/backend/threadpool.h | 6 ------ src/backend/threads.c | 2 +- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 2a33978d6..d7c13551e 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -96,14 +96,6 @@ lumiera_threadpool_destroy(void) } } -void lumiera_threadpool_unlink(LumieraThread thread) -{ - REQUIRE (thread, "invalid thread given"); - REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - llist_unlink(&thread->node); - ENSURE (llist_is_empty(&thread->node), "failed to unlink the thread"); -} - LumieraThread lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index de545876c..572727e3e 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -87,12 +87,6 @@ lumiera_threadpool_init(); void lumiera_threadpool_destroy(void); -/** - * Just remove the thread structure from an associated pool list. - */ -void -lumiera_threadpool_unlink(LumieraThread thread); - #endif /* // Local Variables: diff --git a/src/backend/threads.c b/src/backend/threads.c index a75b7f81b..9c4cc9f1f 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -160,7 +160,7 @@ lumiera_thread_destroy (LumieraThread self) { REQUIRE (self, "trying to destroy an invalid thread"); - lumiera_threadpool_unlink(self); + llist_unlink (&self->node); // get the pthread out of the processing loop // need to signal to the thread that it should start quitting From 026fab07dc845f0a7f696405fa8286cd456f0125 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Thu, 24 Dec 2009 01:46:43 +0100 Subject: [PATCH 34/68] cosmetics, deadcode removal --- src/backend/threadpool.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index d7c13551e..1f2a1cc71 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -42,23 +42,13 @@ NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/bet //code goes here// -void* pool_thread_loop(void * arg) -{ - (void) arg; - while (1) - { - ; - } - return arg; -} - void lumiera_threadpool_init() { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.pool[i].list); - threadpool.pool[i].working_thread_count = 0; + threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; //TODO: configure each pools' pthread_attrs appropriately From 6b4415d8fa4bfb86757a5abf16f10a9f3f585372 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Thu, 24 Dec 2009 01:50:59 +0100 Subject: [PATCH 35/68] nobugify declare and init the nobug flags and use them for logging diagnostics --- src/backend/threadpool.c | 16 +++++++++++----- src/backend/threads.c | 10 ++++++++-- src/backend/threads.h | 1 + 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 1f2a1cc71..706f5bb3b 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -45,6 +45,10 @@ NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/bet void lumiera_threadpool_init() { + NOBUG_INIT_FLAG(threadpool); + NOBUG_INIT_FLAG(threads); + TRACE(threadpool); + for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.pool[i].list); @@ -65,15 +69,16 @@ lumiera_threadpool_init() void lumiera_threadpool_destroy(void) { - ECHO ("destroying threadpool"); + TRACE(threadpool); + for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { - ECHO ("destroying individual pool #%d", i); + TRACE (threadpool, "destroying individual pool #%d", i); LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[i].lock) { REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are running", threadpool.pool[i].working_thread_count); // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list - ECHO ("number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); + INFO (threadpool, "number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); LLIST_WHILE_HEAD(&threadpool.pool[i].list, t) { lumiera_thread_delete((LumieraThread)t); @@ -92,8 +97,8 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - LumieraThread ret; - + TRACE(threadpool); + LumieraThread ret = NULL; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[kind].signal) { @@ -126,6 +131,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, void lumiera_threadpool_park_thread(LumieraThread thread) { + TRACE(threadpool); REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); diff --git a/src/backend/threads.c b/src/backend/threads.c index 9c4cc9f1f..a34dcde4a 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -69,6 +69,7 @@ struct lumiera_thread_mockup static void* thread_loop (void* thread) { + TRACE(threads); LumieraThread t = (LumieraThread)thread; pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); @@ -81,14 +82,16 @@ static void* thread_loop (void* thread) { do { // NULL function means: no work to do + INFO(threads, "function %p", t->function); if (t->function) t->function (t->arguments); + INFO(threads, "Thread awaken with state %d", t->state); lumiera_threadpool_park_thread(t); LUMIERA_RECCONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); } while (t->state != LUMIERA_THREADSTATE_SHUTDOWN); // SHUTDOWN state - ECHO ("thread quitting"); + INFO(threads, "Thread Shutdown"); } return 0; } @@ -102,7 +105,8 @@ lumiera_thread_run (enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - REQUIRE (function, "invalid function"); + TRACE(threads); + // REQUIRE (function, "invalid function"); // ask the threadpool for a thread (it might create a new one) LumieraThread self = lumiera_threadpool_acquire_thread(kind, purpose, flag); @@ -158,6 +162,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread lumiera_thread_destroy (LumieraThread self) { + TRACE(threads); REQUIRE (self, "trying to destroy an invalid thread"); llist_unlink (&self->node); @@ -186,6 +191,7 @@ lumiera_thread_destroy (LumieraThread self) void lumiera_thread_delete (LumieraThread self) { + TRACE(threads); ECHO ("deleting thread"); lumiera_free (lumiera_thread_destroy (self)); } diff --git a/src/backend/threads.h b/src/backend/threads.h index a55926123..9a4e6bbdf 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -34,6 +34,7 @@ //TODO: System includes// #include +NOBUG_DECLARE_FLAG (threads); /** From 88195087d6817cf631beb5dc07b3be9ef400f415 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Thu, 24 Dec 2009 01:54:49 +0100 Subject: [PATCH 36/68] recondition to condition, remove the mutex from the pool rename park to release :P --- src/backend/threadpool.c | 58 ++++++++++++++++++------------------ src/backend/threadpool.h | 8 ++--- src/backend/threads.c | 26 ++++++++-------- src/backend/threads.h | 4 +-- tests/backend/test-threads.c | 22 +++++++------- 5 files changed, 57 insertions(+), 61 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 706f5bb3b..8b86617ff 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -57,12 +57,10 @@ lumiera_threadpool_init() //TODO: configure each pools' pthread_attrs appropriately pthread_attr_init (&threadpool.pool[i].pthread_attrs); - // cehteh prefers that threads are joinable by default - //pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); + // cehteh says that threads must be joinable //cancel... - lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); - lumiera_reccondition_init (&threadpool.pool[i].signal, "thread-signal", &NOBUG_FLAG(threadpool)); + lumiera_condition_init (&threadpool.pool[i].sync, "threadpool", &NOBUG_FLAG(cond_sync)); } } @@ -74,7 +72,7 @@ lumiera_threadpool_destroy(void) for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { TRACE (threadpool, "destroying individual pool #%d", i); - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[i].lock) + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) { REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are running", threadpool.pool[i].working_thread_count); // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list @@ -84,9 +82,8 @@ lumiera_threadpool_destroy(void) lumiera_thread_delete((LumieraThread)t); } } - ECHO ("destroying the pool mutex"); - lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); - ECHO ("pool mutex destroyed"); + + lumiera_condition_destroy (&threadpool.pool[i].sync, &NOBUG_FLAG (cond_sync)); pthread_attr_destroy (&threadpool.pool[i].pthread_attrs); } } @@ -100,16 +97,17 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, TRACE(threadpool); LumieraThread ret = NULL; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[kind].signal) - { - if (llist_is_empty (&threadpool.pool[kind].list)) - { - ret = lumiera_thread_new (kind, purpose, flag, - &threadpool.pool[kind].pthread_attrs); - threadpool.pool[kind].idle_thread_count++; - ENSURE (ret, "did not create a valid thread"); - LUMIERA_RECCONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[kind].sync) + { + if (llist_is_empty (&threadpool.pool[kind].list)) + { + + ret = lumiera_thread_new (kind, purpose, flag, + &threadpool.pool[kind].pthread_attrs); + ENSURE (ret, "did not create a valid thread"); + TODO("no error handling but let the resourcecollector do, no need for return the thread"); + LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); } // use an existing thread, pick the first one // remove it from the pool's list @@ -129,7 +127,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, // TODO: rename to lumiera_threadpool_park_thread void -lumiera_threadpool_park_thread(LumieraThread thread) +lumiera_threadpool_release_thread(LumieraThread thread) { TRACE(threadpool); REQUIRE (thread, "invalid thread given"); @@ -137,20 +135,22 @@ lumiera_threadpool_park_thread(LumieraThread thread) REQUIRE (thread->state != LUMIERA_THREADSTATE_IDLE, "trying to park an already idle thread"); - LUMIERA_RECCONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].signal) + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].sync) { thread->state = LUMIERA_THREADSTATE_IDLE; - REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); + REQUIRE (!llist_is_empty(&thread->node), "thread already belongs to some list"); llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); - threadpool.pool[thread->kind].working_thread_count--; - threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count - ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count(&threadpool.pool[thread->kind].list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[thread->kind].idle_thread_count, - llist_count(&threadpool.pool[thread->kind].list)); - // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); - LUMIERA_RECCONDITION_BROADCAST; + + threadpool.pool[thread->kind].working_thread_count--; + threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count + + ENSURE (threadpool.pool[thread->kind].idle_thread_count == + llist_count(&threadpool.pool[thread->kind].list), + "idle thread count %d is wrong, should be %d", + threadpool.pool[thread->kind].idle_thread_count, + llist_count(&threadpool.pool[thread->kind].list)); + REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + LUMIERA_CONDITION_BROADCAST; } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 572727e3e..a8fa457c5 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -23,9 +23,8 @@ #define LUMIERA_THREADPOOL_H //TODO: Support library includes// -#include "lib/reccondition.h" +#include "lib/condition.h" #include "lib/llist.h" -#include "lib/mutex.h" //TODO: Forward declarations// @@ -60,7 +59,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, * This function doesn't need to be accessible outside of the threadpool implementation. */ void -lumiera_threadpool_park_thread(LumieraThread thread); +lumiera_threadpool_release_thread(LumieraThread thread); typedef struct lumiera_threadpool_struct lumiera_threadpool; typedef lumiera_threadpool* LumieraThreadpool; @@ -70,11 +69,10 @@ struct lumiera_threadpool_struct struct { llist list; - lumiera_mutex lock; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; - lumiera_reccondition signal; + lumiera_condition sync; } pool[LUMIERA_THREADCLASS_COUNT]; }; diff --git a/src/backend/threads.c b/src/backend/threads.c index a34dcde4a..a7f09b763 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -22,7 +22,6 @@ //TODO: Support library includes// #include "include/logging.h" -#include "lib/mutex.h" #include "lib/safeclib.h" @@ -64,7 +63,7 @@ struct lumiera_thread_mockup { void (*fn)(void*); void* arg; - LumieraReccondition finished; + LumieraCondition finished; }; static void* thread_loop (void* thread) @@ -73,21 +72,20 @@ static void* thread_loop (void* thread) LumieraThread t = (LumieraThread)thread; pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); - + REQUIRE (t, "thread does not exist"); - ECHO ("entering section 1"); // this seems to deadlock unexpectedly: - LUMIERA_RECCONDITION_SECTION (threads, &t->signal) + LUMIERA_CONDITION_SECTION (threads, &t->signal) { do { // NULL function means: no work to do INFO(threads, "function %p", t->function); if (t->function) t->function (t->arguments); + lumiera_threadpool_release_thread(t); + LUMIERA_CONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); INFO(threads, "Thread awaken with state %d", t->state); - lumiera_threadpool_park_thread(t); - LUMIERA_RECCONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); } while (t->state != LUMIERA_THREADSTATE_SHUTDOWN); // SHUTDOWN state @@ -117,9 +115,9 @@ lumiera_thread_run (enum lumiera_thread_class kind, // and let it really run (signal the condition var, the thread waits on it) self->state = LUMIERA_THREADSTATE_WAKEUP; - ECHO ("entering section 2"); - LUMIERA_RECCONDITION_SECTION(threads, &self->signal) - LUMIERA_RECCONDITION_BROADCAST; + + LUMIERA_CONDITION_SECTION(threads, &self->signal) + LUMIERA_CONDITION_SIGNAL; // NOTE: example only, add solid error handling! @@ -142,7 +140,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init(&self->node); - lumiera_reccondition_init (&self->signal, "thread-control", flag); + lumiera_condition_init (&self->signal, "thread-control", flag); self->kind = kind; self->state = LUMIERA_THREADSTATE_STARTUP; self->function = NULL; @@ -170,20 +168,20 @@ lumiera_thread_destroy (LumieraThread self) // get the pthread out of the processing loop // need to signal to the thread that it should start quitting // should this be within the section? - LUMIERA_RECCONDITION_SECTION(threads, &self->signal) + LUMIERA_CONDITION_SECTION(threads, &self->signal) { REQUIRE (self->state == LUMIERA_THREADSTATE_IDLE, "trying to delete a thread in state other than IDLE (%s)", lumiera_threadstate_names[self->state]); self->state = LUMIERA_THREADSTATE_SHUTDOWN; self->function = NULL; self->arguments = NULL; - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; } int error = pthread_join(self->id, NULL); ENSURE (0 == error, "pthread_join returned %d:%s", error, strerror(error)); // condition has to be destroyed after joining with the thread - lumiera_reccondition_destroy (&self->signal, &NOBUG_FLAG(threads)); + lumiera_condition_destroy (&self->signal, &NOBUG_FLAG(threads)); return self; } diff --git a/src/backend/threads.h b/src/backend/threads.h index 9a4e6bbdf..d4282da17 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -23,7 +23,7 @@ #define LUMIERA_THREADS_H //TODO: Support library includes// -#include "lib/reccondition.h" +#include "lib/condition.h" //TODO: Forward declarations// @@ -132,7 +132,7 @@ struct lumiera_thread_struct // void* arg; pthread_t id; // TODO: maybe this condition variable should be renamed when we have a better understanding of how it will be used - lumiera_reccondition signal; // control signal, state change signal + lumiera_condition signal; // control signal, state change signal // the following member could have been called "class" except that it would conflict with C++ keyword // as consequence, it's been decided to leave the type name containing the word "class", // while all members/variables called "kind" diff --git a/tests/backend/test-threads.c b/tests/backend/test-threads.c index 21786ae8b..ddf43214e 100644 --- a/tests/backend/test-threads.c +++ b/tests/backend/test-threads.c @@ -47,15 +47,15 @@ void threadfn(void* blah) void threadsyncfn(void* blah) { struct timespec wait = {0,200000000}; - LumieraReccondition sync = (LumieraReccondition) blah; + LumieraCondition sync = (LumieraCondition) blah; ECHO ("thread starting up %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SECTION(cond_sync, sync) + LUMIERA_CONDITION_SECTION(cond_sync, sync) { ECHO ("send startup signal %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; ECHO ("wait for trigger %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); } ECHO ("thread running %s", NOBUG_THREAD_ID_GET); @@ -101,10 +101,10 @@ TEST ("simple_thread") TEST ("thread_synced") { - lumiera_reccondition cnd; - lumiera_reccondition_init (&cnd, "threadsync", &NOBUG_FLAG(NOBUG_ON)); + lumiera_condition cnd; + lumiera_condition_init (&cnd, "threadsync", &NOBUG_FLAG(NOBUG_ON)); - LUMIERA_RECCONDITION_SECTION(cond_sync, &cnd) + LUMIERA_CONDITION_SECTION(cond_sync, &cnd) { ECHO ("main before thread %s", NOBUG_THREAD_ID_GET); @@ -115,17 +115,17 @@ TEST ("thread_synced") NULL); ECHO ("main wait for thread being ready %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); ECHO ("main trigger thread %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_SIGNAL; + LUMIERA_CONDITION_SIGNAL; ECHO ("wait for thread end %s", NOBUG_THREAD_ID_GET); - LUMIERA_RECCONDITION_WAIT(1); + LUMIERA_CONDITION_WAIT(1); ECHO ("thread ended %s", NOBUG_THREAD_ID_GET); } - lumiera_reccondition_destroy (&cnd, &NOBUG_FLAG(NOBUG_ON)); + lumiera_condition_destroy (&cnd, &NOBUG_FLAG(NOBUG_ON)); } From a698128cfc75aba631f0dec8da1d45bfc63b0464 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Thu, 24 Dec 2009 01:55:24 +0100 Subject: [PATCH 37/68] new no-function test, sleep a bit before destroying the threadpool --- tests/backend/test-threadpool.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index ea5d60a76..b83920330 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -26,6 +26,7 @@ #include #include #include +#include void is_prime(void * arg) { @@ -158,6 +159,25 @@ TEST ("toomany-acquire-release") } #endif +TEST ("no-function") +{ + LumieraThread t; + + lumiera_threadpool_init(10); + + t = lumiera_thread_run (LUMIERA_THREADCLASS_INTERACTIVE, + NULL, + NULL, + "process my test function", + &NOBUG_FLAG(NOBUG_ON)); + + // cleanup + ECHO("wait 1 sec"); + usleep(1000000); + ECHO("finished waiting"); + lumiera_threadpool_destroy(); +} + TEST ("process-function") { // this is what the scheduler would do once it figures out what function a job needs to run @@ -175,6 +195,9 @@ TEST ("process-function") &NOBUG_FLAG(NOBUG_ON)); // struct nobug_flag* flag) // cleanup + ECHO("wait 1 sec"); + usleep(1000000); + ECHO("finished waiting"); lumiera_threadpool_destroy(); } From 12a2eed583634732479e0c976b63e79e8d66f8d2 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 29 Dec 2009 16:52:39 -0500 Subject: [PATCH 38/68] fix variable names in the LUMIERA_RECCONDITION_SECTION_CHAIN macro (seems like a copy+paste error from recmutex.h) --- src/lib/reccondition.h | 6 +++--- tests/15locking.tests | 9 +++++++++ tests/library/test-locking.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/lib/reccondition.h b/src/lib/reccondition.h index f9bcd59d1..51ce6ef07 100644 --- a/src/lib/reccondition.h +++ b/src/lib/reccondition.h @@ -65,13 +65,13 @@ #define LUMIERA_RECCONDITION_SECTION_CHAIN(nobugflag, cnd) \ - for (lumiera_sectionlock *lumiera_lock_section_old_ = &lumiera_lock_section_, \ + for (lumiera_sectionlock *lumiera_reccond_section_old_ = &lumiera_reccond_section_, \ NOBUG_CLEANUP(lumiera_sectionlock_ensureunlocked) lumiera_reccond_section_ = { \ (void*)1, lumiera_reccondition_unlock_cb NOBUG_ALPHA_COMMA_NULL NOBUG_ALPHA_COMMA_NULL}; \ lumiera_reccond_section_.lock;) \ for ( \ ({ \ - REQUIRE (lumiera_lock_section_old_->lock, "section prematurely unlocked"); \ + REQUIRE (lumiera_reccond_section_old_->lock, "section prematurely unlocked"); \ lumiera_reccond_section_.lock = (cnd); \ NOBUG_IF_ALPHA(lumiera_reccond_section_.flag = &NOBUG_FLAG(nobugflag);) \ RESOURCE_WAIT (nobugflag, (cnd)->rh, "acquire reccondmutex", lumiera_reccond_section_.rh) \ @@ -79,7 +79,7 @@ if (pthread_mutex_lock (&(cnd)->reccndmutex)) \ LUMIERA_DIE (LOCK_ACQUIRE); \ RESOURCE_STATE (nobugflag, NOBUG_RESOURCE_RECURSIVE, lumiera_reccond_section_.rh); \ - LUMIERA_SECTION_UNLOCK_(lumiera_lock_section_old_); \ + LUMIERA_SECTION_UNLOCK_(lumiera_reccond_section_old_); \ } \ }); \ lumiera_reccond_section_.lock; \ diff --git a/tests/15locking.tests b/tests/15locking.tests index 0936499f2..7401d4b58 100644 --- a/tests/15locking.tests +++ b/tests/15locking.tests @@ -86,3 +86,12 @@ PLANNED "reccondition broadcasting" < Date: Thu, 31 Dec 2009 07:18:29 -0500 Subject: [PATCH 39/68] remove unused function pool_thread_loop() --- src/backend/threadpool.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 66ecd4e33..acde943e9 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -42,16 +42,6 @@ NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/bet //code goes here// -void* pool_thread_loop(void * arg) -{ - (void) arg; - while (1) - { - ; - } - return arg; -} - void lumiera_threadpool_init(unsigned limit) { From 9c60d88c565dc8006cc71ca52223df1b63d9c197 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 07:24:07 -0500 Subject: [PATCH 40/68] remove thread limiting logic from the threadpool --- src/backend/threadpool.c | 21 +++++---------------- src/backend/threadpool.h | 4 +--- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index acde943e9..3ab589179 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -43,12 +43,11 @@ NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/bet //code goes here// void -lumiera_threadpool_init(unsigned limit) +lumiera_threadpool_init(void) { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init(&threadpool.pool[i].list); - threadpool.pool[i].max_threads = limit; threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; @@ -90,20 +89,10 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, if (llist_is_empty (&threadpool.pool[kind].list)) { // TODO: fill in the reccondition argument, currently NULL - FIXME ("this max thread logic needs to be deeply thought about and made more efficient as well as rebust"); - if (threadpool.pool[kind].working_thread_count - + threadpool.pool[kind].idle_thread_count - < threadpool.pool[kind].max_threads) { - ret = lumiera_thread_new (kind, NULL, purpose, flag, - &threadpool.pool[kind].pthread_attrs); - threadpool.pool[kind].working_thread_count++; - ENSURE (ret, "did not create a valid thread"); - } - else - { - //ERROR (threadpool, "did not create a new thread because per-pool limit was reached: %d", threadpool.pool[kind].max_threads); - LUMIERA_DIE(ERRNO); - } + ret = lumiera_thread_new (kind, NULL, purpose, flag, + &threadpool.pool[kind].pthread_attrs); + threadpool.pool[kind].working_thread_count++; + ENSURE (ret, "did not create a valid thread"); } else { diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 03b56a4fe..0c56de62d 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -71,7 +71,6 @@ struct lumiera_threadpool_struct { llist list; lumiera_mutex lock; - unsigned max_threads; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; @@ -80,10 +79,9 @@ struct lumiera_threadpool_struct /** * Initialize the thread pool. - * @param limit the maximum number of threads (idle+working) allowed per pool */ void -lumiera_threadpool_init(unsigned limit); +lumiera_threadpool_init(void); void lumiera_threadpool_destroy(void); From d63a6066a0b8c5dfd592db321c5a12bd00ad81d5 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 07:27:45 -0500 Subject: [PATCH 41/68] insert a space between the function name and the ( in each function call --- src/backend/threadpool.c | 24 ++++++++++++------------ src/backend/threads.c | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 3ab589179..55ae77842 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -47,7 +47,7 @@ lumiera_threadpool_init(void) { for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { - llist_init(&threadpool.pool[i].list); + llist_init (&threadpool.pool[i].list); threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; @@ -56,7 +56,7 @@ lumiera_threadpool_init(void) pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); //cancel... - lumiera_mutex_init(&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG(threadpool)); + lumiera_mutex_init (&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG (threadpool)); } } @@ -68,9 +68,9 @@ lumiera_threadpool_destroy(void) { ECHO ("destroying individual pool #%d", i); // no locking is done at this point - ECHO ("number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); - LLIST_WHILE_HEAD(&threadpool.pool[i].list, thread) - lumiera_thread_delete((LumieraThread)thread); + ECHO ("number of threads in the pool=%d", llist_count (&threadpool.pool[i].list)); + LLIST_WHILE_HEAD (&threadpool.pool[i].list, thread) + lumiera_thread_delete ((LumieraThread)thread); ECHO ("destroying the pool mutex"); lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); ECHO ("pool mutex destroyed"); @@ -100,14 +100,14 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, // remove it from the pool's list LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[kind].lock) { - ret = (LumieraThread)(llist_unlink(llist_head (&threadpool.pool[kind].list))); + ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].list))); threadpool.pool[kind].working_thread_count++; threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count ENSURE (threadpool.pool[kind].idle_thread_count == - llist_count(&threadpool.pool[kind].list), + llist_count (&threadpool.pool[kind].list), "idle thread count %d is wrong, should be %d", threadpool.pool[kind].idle_thread_count, - llist_count(&threadpool.pool[kind].list)); + llist_count (&threadpool.pool[kind].list)); } ENSURE (ret, "did not find a valid thread"); } @@ -122,15 +122,15 @@ lumiera_threadpool_release_thread(LumieraThread thread) LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) { - REQUIRE (llist_is_single(&thread->node), "thread already belongs to some list"); - llist_insert_head(&threadpool.pool[thread->kind].list, &thread->node); + REQUIRE (llist_is_single (&thread->node), "thread already belongs to some list"); + llist_insert_head (&threadpool.pool[thread->kind].list, &thread->node); threadpool.pool[thread->kind].working_thread_count--; threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count(&threadpool.pool[thread->kind].list), + llist_count (&threadpool.pool[thread->kind].list), "idle thread count %d is wrong, should be %d", threadpool.pool[thread->kind].idle_thread_count, - llist_count(&threadpool.pool[thread->kind].list)); + llist_count (&threadpool.pool[thread->kind].list)); // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); } } diff --git a/src/backend/threads.c b/src/backend/threads.c index 62f0d14a6..ef45b1ee8 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -62,7 +62,7 @@ const char* lumiera_threadstate_names[] = { struct lumiera_thread_mockup { - void (*fn)(void*); + void (*fn) (void*); void* arg; LumieraReccondition finished; }; @@ -86,7 +86,7 @@ static void* pthread_runner (void* thread) if (!thread_end_notification) return NULL; // no signalling of thread termination desired - LUMIERA_RECCONDITION_SECTION(cond_sync, thread_end_notification) + LUMIERA_RECCONDITION_SECTION (cond_sync, thread_end_notification) LUMIERA_RECCONDITION_BROADCAST; return NULL; @@ -140,13 +140,13 @@ lumiera_thread_run (enum lumiera_thread_class kind, (void)function; (void)arg; // ask the threadpool for a thread (it might create a new one) - LumieraThread self = lumiera_threadpool_acquire_thread(kind, purpose, flag); + LumieraThread self = lumiera_threadpool_acquire_thread (kind, purpose, flag); // TODO: set the function and data to be run // lumiera_thread_set_func_data (self, start_routine, arg, purpose, flag); // and let it really run (signal the condition var, the thread waits on it) - LUMIERA_RECCONDITION_SECTION(cond_sync, self->finished) + LUMIERA_RECCONDITION_SECTION (cond_sync, self->finished) LUMIERA_RECCONDITION_SIGNAL; // NOTE: example only, add solid error handling! @@ -174,14 +174,14 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread self = lumiera_malloc (sizeof (*self)); - llist_init(&self->node); + llist_init (&self->node); self->finished = finished; self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; //REQUIRE (thread_loop); int error = pthread_create (&self->id, attrs, &thread_loop, self); - ENSURE(error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror(error)); + ENSURE (error == 0 || EAGAIN == error, "pthread returned %d:%s", error, strerror (error)); if (error) { // error here can only be EAGAIN, given the above ENSURE @@ -197,9 +197,9 @@ lumiera_thread_destroy (LumieraThread self) REQUIRE (self, "trying to destroy an invalid thread"); // TODO: stop the pthread - llist_unlink(&self->node); + llist_unlink (&self->node); //finished = NULL; // or free(finished)? - lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG(threads)); + lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG (threads)); //kind = 0; //state = 0; return self; From ad404bd41a91f1958ae76a620b8c07eaa722a868 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 07:30:46 -0500 Subject: [PATCH 42/68] remove unused old code --- src/backend/threads.c | 58 ------------------------------------------- 1 file changed, 58 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index ef45b1ee8..3a20901d8 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -60,70 +60,12 @@ const char* lumiera_threadstate_names[] = { }; #undef LUMIERA_THREAD_STATE -struct lumiera_thread_mockup -{ - void (*fn) (void*); - void* arg; - LumieraReccondition finished; -}; - static void* thread_loop (void* arg) { (void)arg; return 0; } -#if 0 -static void* pthread_runner (void* thread) -{ - pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); - - struct lumiera_thread_mockup* starter = (struct lumiera_thread_mockup*) thread; - LumieraReccondition thread_end_notification = starter->finished; - - starter->fn (starter->arg); - - if (!thread_end_notification) - return NULL; // no signalling of thread termination desired - - LUMIERA_RECCONDITION_SECTION (cond_sync, thread_end_notification) - LUMIERA_RECCONDITION_BROADCAST; - - return NULL; -} -#endif - -#if 0 -// TODO: rewrite this using lumiera_threadpool_acquire() -LumieraThread -lumiera_thread_run (enum lumiera_thread_class kind, - void (*start_routine)(void *), - void * arg, - LumieraReccondition finished, - const char* purpose, - struct nobug_flag* flag) -{ - (void) kind; - (void) purpose; - (void) flag; - - if (attr_once == PTHREAD_ONCE_INIT) - pthread_once (&attr_once, thread_attr_init); - - static struct lumiera_thread_mockup thread; - - thread.fn = start_routine; - thread.arg = arg; - thread.finished = finished; - - pthread_t dummy; - int error = pthread_create (&dummy, &attrs, pthread_runner, &thread); - - if (error) return 0; /////TODO temporary addition by Ichthyo; probably we'll set lumiera_error? - return (LumieraThread) 1; -} -#endif - // TODO: new implementation, remove the above one // maybe this shouldn't return LumieraThread at all // when this is called it should have already been decided that the function From c4e1fdaf9a239fa88af6bdfcdb8237db57c6f189 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 07:32:55 -0500 Subject: [PATCH 43/68] document lumiera_thread_destroy() and lumiera_thread_delete() --- src/backend/threads.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/backend/threads.h b/src/backend/threads.h index e8b774299..303b9acf2 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -142,9 +142,17 @@ lumiera_thread_new (enum lumiera_thread_class kind, struct nobug_flag* flag, pthread_attr_t* attrs); +/** + * Destroy and de-initialize a thread structure. + * Memory is not freed by this function. + */ LumieraThread lumiera_thread_destroy (LumieraThread self); +/** + * Actually free the memory used by the thread structure. + * Make sure to destroy the structure first. + */ void lumiera_thread_delete (LumieraThread self); From 03fe6dd65831a6c6d52a23048cc916586de5e552 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 07:37:28 -0500 Subject: [PATCH 44/68] properly initialize and de-initialize the thread condition variable --- src/backend/threads.c | 9 +-------- src/backend/threads.h | 4 ---- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 3a20901d8..7b03f57fe 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -74,11 +74,9 @@ LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*function)(void *), void * arg, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag) { - (void)finished; (void)function; (void)arg; // ask the threadpool for a thread (it might create a new one) @@ -101,7 +99,6 @@ lumiera_thread_run (enum lumiera_thread_class kind, */ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag, pthread_attr_t* attrs) @@ -112,12 +109,9 @@ lumiera_thread_new (enum lumiera_thread_class kind, REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "invalid thread kind specified: %d", kind); REQUIRE (attrs, "invalid pthread attributes structure passed"); - //REQUIRE (finished, "invalid finished flag passed"); - - LumieraThread self = lumiera_malloc (sizeof (*self)); llist_init (&self->node); - self->finished = finished; + lumiera_reccondition_init (&self->finished, "thread-control-condition", flag); self->kind = kind; self->state = LUMIERA_THREADSTATE_IDLE; @@ -140,7 +134,6 @@ lumiera_thread_destroy (LumieraThread self) // TODO: stop the pthread llist_unlink (&self->node); - //finished = NULL; // or free(finished)? lumiera_reccondition_destroy (self->finished, &NOBUG_FLAG (threads)); //kind = 0; //state = 0; diff --git a/src/backend/threads.h b/src/backend/threads.h index 303b9acf2..d91ae7bee 100644 --- a/src/backend/threads.h +++ b/src/backend/threads.h @@ -137,7 +137,6 @@ struct lumiera_thread_struct */ LumieraThread lumiera_thread_new (enum lumiera_thread_class kind, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag, pthread_attr_t* attrs); @@ -169,8 +168,6 @@ lumiera_thread_delete (LumieraThread self); * @param kind class of the thread to start * @param function pointer to a function to execute in a thread (returning void, not void* as in pthreads) * @param arg generic pointer passed to the thread - * @param finished a condition variable to be broadcasted, if not NULL. - * The associated mutex should be locked at thread_run time already, else the signal can get lost. * @param purpose descriptive name of this thread, used by NoBug * @param flag NoBug flag used for logging the thread startup and return */ @@ -178,7 +175,6 @@ LumieraThread lumiera_thread_run (enum lumiera_thread_class kind, void (*function)(void *), void * arg, - LumieraReccondition finished, const char* purpose, struct nobug_flag* flag); From 7a507a0ceeb8298719b0d16f47987256d0e8d8cb Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 31 Dec 2009 15:09:22 -0500 Subject: [PATCH 45/68] don't put threads in a detached state --- src/backend/threadpool.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 55ae77842..fc6041b5f 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -53,7 +53,6 @@ lumiera_threadpool_init(void) //TODO: configure each pools' pthread_attrs appropriately pthread_attr_init (&threadpool.pool[i].pthread_attrs); - pthread_attr_setdetachstate (&threadpool.pool[i].pthread_attrs, PTHREAD_CREATE_DETACHED); //cancel... lumiera_mutex_init (&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG (threadpool)); From 223e79cc4ad74bb4c308fee55adba710c45f87a3 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 4 Jan 2010 17:03:47 -0500 Subject: [PATCH 46/68] replace mutex with condition variable in threadpool use TRACE instead of ECHO (based on cehteh's suggested code) --- src/backend/threadpool.c | 99 +++++++++++++++++++++++----------------- src/backend/threadpool.h | 5 +- 2 files changed, 58 insertions(+), 46 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index fc6041b5f..6a4f90289 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -45,6 +45,9 @@ NOBUG_DEFINE_FLAG_PARENT (threadpool, threads_dbg); /*TODO insert a suitable/bet void lumiera_threadpool_init(void) { + NOBUG_INIT_FLAG (threadpool); + TRACE (threadpool); + for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { llist_init (&threadpool.pool[i].list); @@ -55,24 +58,28 @@ lumiera_threadpool_init(void) pthread_attr_init (&threadpool.pool[i].pthread_attrs); //cancel... - lumiera_mutex_init (&threadpool.pool[i].lock,"pool of threads", &NOBUG_FLAG (threadpool)); + lumiera_condition_init (&threadpool.pool[i].sync,"pool of threads", &NOBUG_FLAG (threadpool)); } } void lumiera_threadpool_destroy(void) { - ECHO ("destroying threadpool"); + TRACE (threadpool); for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { - ECHO ("destroying individual pool #%d", i); - // no locking is done at this point - ECHO ("number of threads in the pool=%d", llist_count (&threadpool.pool[i].list)); - LLIST_WHILE_HEAD (&threadpool.pool[i].list, thread) - lumiera_thread_delete ((LumieraThread)thread); - ECHO ("destroying the pool mutex"); - lumiera_mutex_destroy (&threadpool.pool[i].lock, &NOBUG_FLAG (threadpool)); - ECHO ("pool mutex destroyed"); + TRACE (threadpool, "destroying individual pool #%d", i); + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) + { + REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are still running", threadpool.pool[i].working_thread_count); + // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list + INFO (threadpool, "number of threads in the pool=%d", llist_count(&threadpool.pool[i].list)); + LLIST_WHILE_HEAD (&threadpool.pool[i].list, t) + { + lumiera_thread_delete ((LumieraThread)t); + } + } + lumiera_condition_destroy (&threadpool.pool[i].sync, &NOBUG_FLAG (threadpool)); pthread_attr_destroy (&threadpool.pool[i].pthread_attrs); } } @@ -82,55 +89,61 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - LumieraThread ret; + TRACE (threadpool); + LumieraThread ret = NULL; REQUIRE (kind < LUMIERA_THREADCLASS_COUNT, "unknown pool kind specified: %d", kind); - if (llist_is_empty (&threadpool.pool[kind].list)) + + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[kind].sync) { - // TODO: fill in the reccondition argument, currently NULL - ret = lumiera_thread_new (kind, NULL, purpose, flag, - &threadpool.pool[kind].pthread_attrs); + if (llist_is_empty (&threadpool.pool[kind].list)) + { + ret = lumiera_thread_new (kind, purpose, flag, + &threadpool.pool[kind].pthread_attrs); + ENSURE (ret, "did not create a valid thread"); + TODO ("no error handing, let the resourcecollector do it, no need when returning the thread"); + LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); + } + // use an existing thread, pick the first one + // remove it from the pool's list + ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].list))); + REQUIRE (ret->state == LUMIERA_THREADSTATE_IDLE, "trying to return a non-idle thread (state=%s)", lumiera_threadstate_names[ret->state]); threadpool.pool[kind].working_thread_count++; - ENSURE (ret, "did not create a valid thread"); + threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count + ENSURE (threadpool.pool[kind].idle_thread_count == + llist_count (&threadpool.pool[kind].list), + "idle thread count %d is wrong, should be %d", + threadpool.pool[kind].idle_thread_count, + llist_count (&threadpool.pool[kind].list)); + ENSURE (ret, "did not find a valid thread"); } - else - { - // use an existing thread, pick the first one - // remove it from the pool's list - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[kind].lock) - { - ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].list))); - threadpool.pool[kind].working_thread_count++; - threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count - ENSURE (threadpool.pool[kind].idle_thread_count == - llist_count (&threadpool.pool[kind].list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[kind].idle_thread_count, - llist_count (&threadpool.pool[kind].list)); - } - ENSURE (ret, "did not find a valid thread"); - } return ret; } void lumiera_threadpool_release_thread(LumieraThread thread) { + TRACE (threadpool); REQUIRE (thread, "invalid thread given"); REQUIRE (thread->kind < LUMIERA_THREADCLASS_COUNT, "thread belongs to an unknown pool kind: %d", thread->kind); - LUMIERA_MUTEX_SECTION (threadpool, &threadpool.pool[thread->kind].lock) + REQUIRE (thread->state != LUMIERA_THREADSTATE_IDLE, "trying to park an already idle thread"); + LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].sync) { - REQUIRE (llist_is_single (&thread->node), "thread already belongs to some list"); + thread->state = LUMIERA_THREADSTATE_IDLE; + REQUIRE (llist_is_empty (&thread->node), "thread already belongs to some list"); llist_insert_head (&threadpool.pool[thread->kind].list, &thread->node); - threadpool.pool[thread->kind].working_thread_count--; - threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count - ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count (&threadpool.pool[thread->kind].list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[thread->kind].idle_thread_count, - llist_count (&threadpool.pool[thread->kind].list)); - // REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + + threadpool.pool[thread->kind].working_thread_count--; + threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count + + ENSURE (threadpool.pool[thread->kind].idle_thread_count == + llist_count (&threadpool.pool[thread->kind].list), + "idle thread count %d is wrong, should be %d", + threadpool.pool[thread->kind].idle_thread_count, + llist_count (&threadpool.pool[thread->kind].list)); + REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + LUMIERA_CONDITION_BROADCAST; } } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index 0c56de62d..a14c87b50 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -23,9 +23,8 @@ #define LUMIERA_THREADPOOL_H //TODO: Support library includes// -#include "lib/reccondition.h" +#include "lib/condition.h" #include "lib/llist.h" -#include "lib/mutex.h" //TODO: Forward declarations// @@ -70,7 +69,7 @@ struct lumiera_threadpool_struct struct { llist list; - lumiera_mutex lock; + lumiera_condition sync; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; From a668095f601cfad7a6c58c81d8fff6e30b34ae02 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 5 Jan 2010 20:11:34 -0500 Subject: [PATCH 47/68] ignore automake-related files in the m4 directory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c89dffc5e..f66bd5b09 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ autom4te.cache semantic.cache wiki/backups/* doc/devel/draw/*.png +m4/* From c707f949295a85881f56bf312b6b8c165f8e7b4a Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Wed, 6 Jan 2010 07:42:42 -0500 Subject: [PATCH 48/68] update c++ wrapper to match the C API --- src/backend/thread-wrapper.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/thread-wrapper.hpp b/src/backend/thread-wrapper.hpp index 3f897b582..7259e4fbc 100644 --- a/src/backend/thread-wrapper.hpp +++ b/src/backend/thread-wrapper.hpp @@ -177,10 +177,11 @@ namespace backend { lumiera_thread_run ( kind , &run // invoking the run helper and.. , this // passing this start context as parameter - , joinCond // maybe wait-blocking for the thread to terminate , purpose.c() , logging_flag ); + (void)joinCond; // TODO: this is a temporary fix to match the C API + // we might have to re-write more of this file or even remove it later if (!res) throw lumiera::error::State("failed to create new thread."); From 56cf53adb3fe2602e9a1e4ebed4dd84f3a35352e Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 7 Jan 2010 18:36:51 -0500 Subject: [PATCH 49/68] fix compilation --- src/backend/threads.c | 2 +- tests/backend/test-threadpool.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index d7d7a242f..773568f7b 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -109,7 +109,7 @@ lumiera_thread_run (enum lumiera_thread_class kind, // and let it really run (signal the condition var, the thread waits on it) self->state = LUMIERA_THREADSTATE_WAKEUP; - LUMIERA_CONDITION_SECTION (cond_sync, self->finished) + LUMIERA_CONDITION_SECTION (cond_sync, &self->signal) LUMIERA_CONDITION_SIGNAL; // NOTE: example only, add solid error handling! diff --git a/tests/backend/test-threadpool.c b/tests/backend/test-threadpool.c index b83920330..2acb8b861 100644 --- a/tests/backend/test-threadpool.c +++ b/tests/backend/test-threadpool.c @@ -48,14 +48,14 @@ TESTS_BEGIN TEST ("threadpool-basic") { - lumiera_threadpool_init(100); + lumiera_threadpool_init(); lumiera_threadpool_destroy(); } TEST ("threadpool1") { ECHO("start by initializing the threadpool"); - lumiera_threadpool_init(100); + lumiera_threadpool_init(); LumieraThread t1 = lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_INTERACTIVE, "test purpose", @@ -69,7 +69,7 @@ TEST ("threadpool1") TEST ("basic-acquire-release") { ECHO("start by initializing the threadpool"); - lumiera_threadpool_init(100); + lumiera_threadpool_init(); ECHO("acquiring thread 1"); LumieraThread t1 = lumiera_threadpool_acquire_thread(LUMIERA_THREADCLASS_INTERACTIVE, @@ -163,7 +163,7 @@ TEST ("no-function") { LumieraThread t; - lumiera_threadpool_init(10); + lumiera_threadpool_init(); t = lumiera_thread_run (LUMIERA_THREADCLASS_INTERACTIVE, NULL, @@ -184,7 +184,7 @@ TEST ("process-function") LumieraThread t; int number = 440616; - lumiera_threadpool_init(10); + lumiera_threadpool_init(); ECHO ("the input to the function is %d", number); From b679bfa236f1e33a41e6e18e82f91ba3db1b8376 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Fri, 8 Jan 2010 07:33:25 -0500 Subject: [PATCH 50/68] don't expect any more output from the basic test --- tests/30backend-threadpool.tests | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index 2cad43477..bd59e0792 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -6,32 +6,6 @@ PLANNED "yield" PLANNED "cancel" TEST "Most basic threadpool test" threadpool-basic < Date: Fri, 8 Jan 2010 16:38:35 -0500 Subject: [PATCH 51/68] merge ECHO with TRACE --- src/backend/threads.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 773568f7b..1781832e3 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -182,8 +182,7 @@ lumiera_thread_destroy (LumieraThread self) void lumiera_thread_delete (LumieraThread self) { - TRACE(threads); - ECHO ("deleting thread"); + TRACE(threads, "deleting thread"); lumiera_free (lumiera_thread_destroy (self)); } From 925442b3d91321d508fd225afb9fb3efe81ffa44 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 9 Jan 2010 21:36:20 -0500 Subject: [PATCH 52/68] begin adding a second list to store working threads --- src/backend/threadpool.c | 13 +++++++++---- src/backend/threadpool.h | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index b423d3e5a..346a847d2 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -52,7 +52,8 @@ lumiera_threadpool_init(void) for (int i = 0; i < LUMIERA_THREADCLASS_COUNT; ++i) { - llist_init (&threadpool.pool[i].list); + llist_init (&threadpool.pool[i].working_list); + llist_init (&threadpool.pool[i].idle_list); threadpool.pool[i].working_thread_count = 0; threadpool.pool[i].idle_thread_count = 0; @@ -74,10 +75,14 @@ lumiera_threadpool_destroy(void) TRACE (threadpool, "destroying individual pool #%d", i); LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) { - REQUIRE (0 == threadpool.pool[i].working_thread_count, "%d threads are still running", threadpool.pool[i].working_thread_count); + REQUIRE (0 == threadpool.pool[i].working_thread_count && + 0 = llist_count (&threadpool.pool[i].working_list), + "%d(%d) threads are still running", + threadpool.pool[i].working_thread_count, + llist_count (&threadpool.pool[i].working_list)); // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list - INFO (threadpool, "number of threads in the pool=%d", llist_count (&threadpool.pool[i].list)); - LLIST_WHILE_HEAD (&threadpool.pool[i].list, t) + INFO (threadpool, "working threads count=%d, idle threads count=%d", llist_count (&threadpool.pool[i].working_list), llist_count (&threadpool.pool[i].idle_list)); + LLIST_WHILE_HEAD (&threadpool.pool[i].idle_list, t) { lumiera_thread_delete ((LumieraThread)t); } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index c2ec10785..b3fb5a18e 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -68,7 +68,8 @@ struct lumiera_threadpool_struct { struct { - llist list; + llist working_list; + llist working_idle; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; From f890b35a039134145dddb224a49bf4c71bc51d3e Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Mon, 11 Jan 2010 15:04:52 -0500 Subject: [PATCH 53/68] continuation of working_list introduction --- src/backend/threadpool.c | 31 ++++++++++++++++++++++++------- src/backend/threadpool.h | 2 +- src/backend/threads.c | 3 +++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 346a847d2..f08bfd0e0 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -105,26 +105,37 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[kind].sync) { - if (llist_is_empty (&threadpool.pool[kind].list)) + if (llist_is_empty (&threadpool.pool[kind].idle_list)) { ret = lumiera_thread_new (kind, purpose, flag, &threadpool.pool[kind].pthread_attrs); ENSURE (ret, "did not create a valid thread"); TODO ("no error handing, let the resourcecollector do it, no need when returning the thread"); - LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].list)); + LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].idel_list)); } // use an existing thread, pick the first one // remove it from the pool's list - ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].list))); + ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].idle_list))); + + ENSURE (ret, "did not find a valid thread"); + REQUIRE (ret->state == LUMIERA_THREADSTATE_IDLE, "trying to return a non-idle thread (state=%s)", lumiera_threadstate_names[ret->state]); + + llist_insert_head (&threadpool.pool[kind].working_list, ret); + threadpool.pool[kind].working_thread_count++; threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count + ENSURE (threadpool.pool[kind].working_thread_count == + llist_count (&threadpool.pool[kind].working_list), + "working thread count %d is wrong, should be %d", + threadpool.pool[kind].working_thread_count, + llist_count (&threadpool.pool[kind].working_list)); + ENSURE (threadpool.pool[kind].idle_thread_count == - llist_count (&threadpool.pool[kind].list), + llist_count (&threadpool.pool[kind].idle_list), "idle thread count %d is wrong, should be %d", threadpool.pool[kind].idle_thread_count, - llist_count (&threadpool.pool[kind].list)); - ENSURE (ret, "did not find a valid thread"); + llist_count (&threadpool.pool[kind].idle_list)); } return ret; } @@ -142,7 +153,8 @@ lumiera_threadpool_release_thread(LumieraThread thread) { thread->state = LUMIERA_THREADSTATE_IDLE; REQUIRE (llist_is_empty (&thread->node), "thread already belongs to some list"); - llist_insert_head (&threadpool.pool[thread->kind].list, &thread->node); + llist_unlink (&thread); // remove thread from the working_list + llist_insert_head (&threadpool.pool[thread->kind].idle_list, &thread->node); threadpool.pool[thread->kind].working_thread_count--; threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count @@ -152,6 +164,11 @@ lumiera_threadpool_release_thread(LumieraThread thread) "idle thread count %d is wrong, should be %d", threadpool.pool[thread->kind].idle_thread_count, llist_count (&threadpool.pool[thread->kind].list)); + ENSURE (threadpool.pool[thread->kind].working_thread_count == + llist_count (&threadpool.pool[thread->kind].working_list), + "working thread count %d is wrong, should be %d", + threadpool.pool[thread->kind].working_thread_count, + llist_count (&threadpool.pool[thread->kind].workinglist)); REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); LUMIERA_CONDITION_BROADCAST; } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index b3fb5a18e..ac3a52ad2 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -69,7 +69,7 @@ struct lumiera_threadpool_struct struct { llist working_list; - llist working_idle; + llist idle_list; unsigned working_thread_count; unsigned idle_thread_count; pthread_attr_t pthread_attrs; diff --git a/src/backend/threads.c b/src/backend/threads.c index 1781832e3..5c8d69eba 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -76,6 +76,9 @@ static void* thread_loop (void* thread) INFO(threads, "function %p", t->function); if (t->function) t->function (t->arguments); + // TODO: problem: + // calling release on a not-yet properly setup thread + // threadpool.pool[thread->kind].working_thread_count == 0 lumiera_threadpool_release_thread(t); LUMIERA_CONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); INFO(threads, "Thread awaken with state %d", t->state); From ecbcfdefd78adbcef60f7a2b62f8f98d77494976 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 12 Jan 2010 07:39:12 -0500 Subject: [PATCH 54/68] remote unnecessary calls to llist_unlink() insert is enough --- src/backend/threadpool.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index f08bfd0e0..c0757a28d 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -115,12 +115,13 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, } // use an existing thread, pick the first one // remove it from the pool's list - ret = (LumieraThread)(llist_unlink (llist_head (&threadpool.pool[kind].idle_list))); + ret = (LumieraThread) (llist_head (&threadpool.pool[kind].idle_list)); ENSURE (ret, "did not find a valid thread"); REQUIRE (ret->state == LUMIERA_THREADSTATE_IDLE, "trying to return a non-idle thread (state=%s)", lumiera_threadstate_names[ret->state]); + // move thread to the working_list llist_insert_head (&threadpool.pool[kind].working_list, ret); threadpool.pool[kind].working_thread_count++; @@ -153,7 +154,7 @@ lumiera_threadpool_release_thread(LumieraThread thread) { thread->state = LUMIERA_THREADSTATE_IDLE; REQUIRE (llist_is_empty (&thread->node), "thread already belongs to some list"); - llist_unlink (&thread); // remove thread from the working_list + // move thread to the idle_list llist_insert_head (&threadpool.pool[thread->kind].idle_list, &thread->node); threadpool.pool[thread->kind].working_thread_count--; From b49a5abff271723479b3807c191b6574aecb4431 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 12 Jan 2010 07:47:28 -0500 Subject: [PATCH 55/68] mark thread as worker --- src/backend/threads.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/threads.c b/src/backend/threads.c index 5c8d69eba..ea565ca0a 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -62,6 +62,7 @@ const char* lumiera_threadstate_names[] = { static void* thread_loop (void* thread) { TRACE(threads); + NOBUG_THREAD_ID_SET("worker"); LumieraThread t = (LumieraThread)thread; pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); From 09fd15d5f84529b9baae60136c8e4cd00656fcc0 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Tue, 12 Jan 2010 08:01:54 -0500 Subject: [PATCH 56/68] die regardless of what type of failure pthread_create() encounters --- src/backend/threads.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index ea565ca0a..74efb6687 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -144,11 +144,8 @@ lumiera_thread_new (enum lumiera_thread_class kind, self->arguments = NULL; int error = pthread_create (&self->id, attrs, &thread_loop, self); - ENSURE(error == 0 || EAGAIN == error, "pthread_create returned %d:%s", error, strerror(error)); if (error) { - // error here can only be EAGAIN, given the above ENSURE - FIXME ("error is %d:%s, see if this can be improved", error, strerror(error)); LUMIERA_DIE (ERRNO); } return self; From f62513dea89550c74a9e09c1bcd5e7d3fe5dc1e5 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 14 Jan 2010 16:46:27 -0500 Subject: [PATCH 57/68] fix compilation errors --- src/backend/threadpool.c | 14 +++++++------- src/lib/condition.h | 4 ++-- src/proc/mobject/placement.hpp | 1 + src/proc/mobject/session/placement-index.cpp | 3 ++- src/proc/mobject/session/placement-index.hpp | 3 ++- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index c0757a28d..ce8a7d3e9 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -76,7 +76,7 @@ lumiera_threadpool_destroy(void) LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) { REQUIRE (0 == threadpool.pool[i].working_thread_count && - 0 = llist_count (&threadpool.pool[i].working_list), + 0 == llist_count (&threadpool.pool[i].working_list), "%d(%d) threads are still running", threadpool.pool[i].working_thread_count, llist_count (&threadpool.pool[i].working_list)); @@ -111,7 +111,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, &threadpool.pool[kind].pthread_attrs); ENSURE (ret, "did not create a valid thread"); TODO ("no error handing, let the resourcecollector do it, no need when returning the thread"); - LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].idel_list)); + LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].idle_list)); } // use an existing thread, pick the first one // remove it from the pool's list @@ -122,7 +122,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, REQUIRE (ret->state == LUMIERA_THREADSTATE_IDLE, "trying to return a non-idle thread (state=%s)", lumiera_threadstate_names[ret->state]); // move thread to the working_list - llist_insert_head (&threadpool.pool[kind].working_list, ret); + llist_insert_head (&threadpool.pool[kind].working_list, &ret->node); threadpool.pool[kind].working_thread_count++; threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count @@ -161,16 +161,16 @@ lumiera_threadpool_release_thread(LumieraThread thread) threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count (&threadpool.pool[thread->kind].list), + llist_count (&threadpool.pool[thread->kind].idle_list), "idle thread count %d is wrong, should be %d", threadpool.pool[thread->kind].idle_thread_count, - llist_count (&threadpool.pool[thread->kind].list)); + llist_count (&threadpool.pool[thread->kind].idle_list)); ENSURE (threadpool.pool[thread->kind].working_thread_count == llist_count (&threadpool.pool[thread->kind].working_list), "working thread count %d is wrong, should be %d", threadpool.pool[thread->kind].working_thread_count, - llist_count (&threadpool.pool[thread->kind].workinglist)); - REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].list), "thread pool is still empty after insertion"); + llist_count (&threadpool.pool[thread->kind].working_list)); + REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].idle_list), "thread pool is still empty after insertion"); LUMIERA_CONDITION_BROADCAST; } } diff --git a/src/lib/condition.h b/src/lib/condition.h index 293d0143e..d75844144 100644 --- a/src/lib/condition.h +++ b/src/lib/condition.h @@ -123,7 +123,7 @@ */ #define LUMIERA_CONDITION_SIGNAL \ do { \ - REQUIRE (lumiera_cond_section_.lock, "Condition mutex not locked"); \ + REQUIRE (lumiera_lock_section_.lock, "Condition mutex not locked"); \ lumiera_condition_signal (lumiera_lock_section_.lock, \ lumiera_lock_section_.flag); \ } while (0) @@ -136,7 +136,7 @@ */ #define LUMIERA_CONDITION_BROADCAST \ do { \ - REQUIRE (lumiera_cond_section_.lock, "Condition mutex not locked"); \ + REQUIRE (lumiera_lock_section_.lock, "Condition mutex not locked"); \ lumiera_condition_broadcast (lumiera_lock_section_.lock, \ lumiera_lock_section_.flag); \ } while (0) diff --git a/src/proc/mobject/placement.hpp b/src/proc/mobject/placement.hpp index 1ebb966ac..569dc6686 100644 --- a/src/proc/mobject/placement.hpp +++ b/src/proc/mobject/placement.hpp @@ -242,6 +242,7 @@ namespace mobject { /** @todo cleanup uses of ref-to-placement. See Ticket #115 */ typedef Placement PlacementMO; + //‘typedef class mobject::Placement mobject::PlacementMO’ typedef Placement PMO; diff --git a/src/proc/mobject/session/placement-index.cpp b/src/proc/mobject/session/placement-index.cpp index f55e6fc3a..adbad89b8 100644 --- a/src/proc/mobject/session/placement-index.cpp +++ b/src/proc/mobject/session/placement-index.cpp @@ -95,7 +95,8 @@ namespace session { /* some type shorthands */ - typedef PlacementIndex::PlacementMO PlacementMO; + //typedef PlacementIndex::PlacementMO PlacementMO; + typedef mobject::PlacementMO PlacementMO; typedef PlacementIndex::PRef PRef; typedef PlacementIndex::ID ID; diff --git a/src/proc/mobject/session/placement-index.hpp b/src/proc/mobject/session/placement-index.hpp index a8df50b2a..537212a8d 100644 --- a/src/proc/mobject/session/placement-index.hpp +++ b/src/proc/mobject/session/placement-index.hpp @@ -160,7 +160,8 @@ namespace session { public: - typedef Placement PlacementMO; + //typedef Placement PlacementMO; + // ‘typedef class mobject::Placement mobject::session::PlacementIndex::PlacementMO’ typedef PlacementRef PRef; typedef PlacementMO::ID const& ID; From 2f27c7d71b87105f2a58f25bddbbfe1efc9612df Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Thu, 14 Jan 2010 20:57:45 -0500 Subject: [PATCH 58/68] Use a fully qualified name for PlacementMO in PlacementIndex This seems to satisfy g++ 4.4.2, which otherwise complains like this: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In file included from ../src/proc/mobject/session/session-impl.hpp:53, from ../src/proc/mobject/session/sess-manager-impl.hpp:28, from ../src/proc/mobject/session/sess-manager-impl.cpp:41: ../src/proc/mobject/session/placement-index.hpp:163: error: declaration of ‘typedef class mobject::Placement mobject::session::PlacementIndex::PlacementMO’ ../src/proc/mobject/placement.hpp:244: error: changes meaning of ‘PlacementMO’ from ‘typedef class mobject::Placement mobject::PlacementMO’ --- src/proc/mobject/session/placement-index.cpp | 2 +- src/proc/mobject/session/placement-index.hpp | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/proc/mobject/session/placement-index.cpp b/src/proc/mobject/session/placement-index.cpp index f55e6fc3a..036fff3f4 100644 --- a/src/proc/mobject/session/placement-index.cpp +++ b/src/proc/mobject/session/placement-index.cpp @@ -95,7 +95,7 @@ namespace session { /* some type shorthands */ - typedef PlacementIndex::PlacementMO PlacementMO; + typedef mobject::PlacementMO PlacementMO; typedef PlacementIndex::PRef PRef; typedef PlacementIndex::ID ID; diff --git a/src/proc/mobject/session/placement-index.hpp b/src/proc/mobject/session/placement-index.hpp index a8df50b2a..2da6540b0 100644 --- a/src/proc/mobject/session/placement-index.hpp +++ b/src/proc/mobject/session/placement-index.hpp @@ -160,7 +160,6 @@ namespace session { public: - typedef Placement PlacementMO; typedef PlacementRef PRef; typedef PlacementMO::ID const& ID; From 1db2a4733997e3937eb77f4fe48addd229206424 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 15 Jan 2010 03:27:09 +0100 Subject: [PATCH 59/68] python-2.6 fix: loading the icon_rener.py script (Ticket #222) --- SConstruct | 5 +++-- admin/{render-icon.py => render_icon.py} | 0 admin/scons/Buildhelper.py | 5 +++-- icons/Makefile.am | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) rename admin/{render-icon.py => render_icon.py} (100%) diff --git a/SConstruct b/SConstruct index 1f1673f6c..c40c90ce2 100644 --- a/SConstruct +++ b/SConstruct @@ -33,7 +33,7 @@ TESTDIR = 'tests' ICONDIR = 'icons' VERSION = '0.1+pre.01' TOOLDIR = './admin/scons' -SVGRENDERER = 'admin/render-icon' +SCRIPTDIR = './admin' #-----------------------------------Configuration # NOTE: scons -h for help. @@ -47,6 +47,7 @@ import os import sys sys.path.append(TOOLDIR) +sys.path.append(SCRIPTDIR) from Buildhelper import * from LumieraEnvironment import * @@ -84,7 +85,7 @@ def setupBasicEnvironment(): , CCFLAGS='-Wall -Wextra ' , CFLAGS='-std=gnu99' ) - RegisterIcon_Builder(env,SVGRENDERER) + RegisterIcon_Builder(env) handleNoBugSwitches(env) env.Append(CPPDEFINES = '_GNU_SOURCE') diff --git a/admin/render-icon.py b/admin/render_icon.py similarity index 100% rename from admin/render-icon.py rename to admin/render_icon.py diff --git a/admin/scons/Buildhelper.py b/admin/scons/Buildhelper.py index 159dc4721..f8e349a5e 100644 --- a/admin/scons/Buildhelper.py +++ b/admin/scons/Buildhelper.py @@ -196,12 +196,13 @@ def checkCommandOption(env, optID, val=None, cmdName=None): -def RegisterIcon_Builder(env, renderer): +def RegisterIcon_Builder(env): """ Registers Custom Builders for generating and installing Icons. Additionally you need to build the tool (rsvg-convert.c) used to generate png from the svg source using librsvg. """ - renderer = __import__(renderer) # load python script for invoking the render + + import render_icon as renderer # load Joel's python script for invoking the rsvg-convert (SVG render) renderer.rsvgPath = env.subst("$BINDIR/rsvg-convert") def invokeRenderer(target, source, env): diff --git a/icons/Makefile.am b/icons/Makefile.am index 97d817ab4..ce0990f26 100644 --- a/icons/Makefile.am +++ b/icons/Makefile.am @@ -18,7 +18,7 @@ svgdir = $(top_srcdir)/icons/svg prerendereddir = $(top_srcdir)/icons/prerendered icondir = $(top_builddir) -iconcommand = python $(top_srcdir)/admin/render-icon.py +iconcommand = python $(top_srcdir)/admin/render_icon.py 16x16 = $(icondir)/16x16 22x22 = $(icondir)/22x22 From e9ae8c8601ed98dab865c605b2c2b788d5f54613 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Fri, 15 Jan 2010 07:35:28 -0500 Subject: [PATCH 60/68] add a thread state check and remove an old comment --- src/backend/threadpool.c | 1 + src/backend/threads.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index ce8a7d3e9..8166e39f8 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -110,6 +110,7 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, ret = lumiera_thread_new (kind, purpose, flag, &threadpool.pool[kind].pthread_attrs); ENSURE (ret, "did not create a valid thread"); + ENSURE (ret->state == LUMIERA_THREADSTATE_IDLE, "a non-idle thread found in the idle pool"); TODO ("no error handing, let the resourcecollector do it, no need when returning the thread"); LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].idle_list)); } diff --git a/src/backend/threads.c b/src/backend/threads.c index 74efb6687..0675e6187 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -69,7 +69,6 @@ static void* thread_loop (void* thread) REQUIRE (t, "thread does not exist"); - // this seems to deadlock unexpectedly: LUMIERA_CONDITION_SECTION (threads, &t->signal) { do { From 55859bcb14fb772050206ce57540784953bb93a4 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Fri, 15 Jan 2010 17:15:58 -0500 Subject: [PATCH 61/68] match the filename in the header comment --- admin/render_icon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/render_icon.py b/admin/render_icon.py index 0f9cee294..e3f76c129 100755 --- a/admin/render_icon.py +++ b/admin/render_icon.py @@ -1,6 +1,6 @@ #!/usr/bin/python # -# render-icons.py - Icon rendering utility script +# render_icons.py - Icon rendering utility script # # Copyright (C) Lumiera.org # 2008, Joel Holdsworth From e60c10a01d90ee544287effb6c9504a85b9b16ac Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 10:39:45 -0500 Subject: [PATCH 62/68] remove redundant info from TRACE and add spaces for formatting --- src/backend/threads.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/threads.c b/src/backend/threads.c index 0675e6187..5038c76dc 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -61,7 +61,7 @@ const char* lumiera_threadstate_names[] = { static void* thread_loop (void* thread) { - TRACE(threads); + TRACE (threads); NOBUG_THREAD_ID_SET("worker"); LumieraThread t = (LumieraThread)thread; @@ -99,7 +99,7 @@ lumiera_thread_run (enum lumiera_thread_class kind, const char* purpose, struct nobug_flag* flag) { - TRACE(threads); + TRACE (threads); // REQUIRE (function, "invalid function"); // ask the threadpool for a thread (it might create a new one) @@ -153,7 +153,7 @@ lumiera_thread_new (enum lumiera_thread_class kind, LumieraThread lumiera_thread_destroy (LumieraThread self) { - TRACE(threads); + TRACE (threads); REQUIRE (self, "trying to destroy an invalid thread"); llist_unlink (&self->node); @@ -182,7 +182,7 @@ lumiera_thread_destroy (LumieraThread self) void lumiera_thread_delete (LumieraThread self) { - TRACE(threads, "deleting thread"); + TRACE (threads); lumiera_free (lumiera_thread_destroy (self)); } From 02c9ee33c701b7b6a616cea9111c010e953730e0 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 11:48:48 -0500 Subject: [PATCH 63/68] fix the code by re-merging some of cehteh's changes test still fails --- src/backend/threadpool.c | 64 ++++++++++++++++---------------- src/backend/threadpool.h | 4 +- src/backend/threads.c | 3 -- tests/30backend-threadpool.tests | 5 +-- 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 8166e39f8..3f475d9d7 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -54,7 +54,7 @@ lumiera_threadpool_init(void) { llist_init (&threadpool.pool[i].working_list); llist_init (&threadpool.pool[i].idle_list); - threadpool.pool[i].working_thread_count = 0; + threadpool.pool[i].thread_count = 0; threadpool.pool[i].idle_thread_count = 0; //TODO: configure each pools' pthread_attrs appropriately @@ -75,16 +75,24 @@ lumiera_threadpool_destroy(void) TRACE (threadpool, "destroying individual pool #%d", i); LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[i].sync) { - REQUIRE (0 == threadpool.pool[i].working_thread_count && - 0 == llist_count (&threadpool.pool[i].working_list), - "%d(%d) threads are still running", - threadpool.pool[i].working_thread_count, + REQUIRE (threadpool.pool[i].thread_count + == threadpool.pool[i].idle_thread_count + && 0 == llist_count (&threadpool.pool[i].working_list), + "%d(llist_count=%d) threads are still running", + threadpool.pool[i].thread_count + - threadpool.pool[i].idle_thread_count, llist_count (&threadpool.pool[i].working_list)); - // TODO need to have a stronger assertion that no threads are really running because they will not even be in the list - INFO (threadpool, "working threads count=%d, idle threads count=%d", llist_count (&threadpool.pool[i].working_list), llist_count (&threadpool.pool[i].idle_list)); + REQUIRE ((int)(llist_count (&threadpool.pool[i].working_list) + + llist_count (&threadpool.pool[i].idle_list)) + == threadpool.pool[i].thread_count, + "threadpool counter miscalculation (working_list count = %u, idle_list count = %u, thread_count = %d )", + llist_count (&threadpool.pool[i].working_list), + llist_count (&threadpool.pool[i].idle_list), + threadpool.pool[i].thread_count); LLIST_WHILE_HEAD (&threadpool.pool[i].idle_list, t) { lumiera_thread_delete ((LumieraThread)t); + threadpool.pool[i].thread_count--; } } lumiera_condition_destroy (&threadpool.pool[i].sync, &NOBUG_FLAG (threadpool)); @@ -110,8 +118,8 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, ret = lumiera_thread_new (kind, purpose, flag, &threadpool.pool[kind].pthread_attrs); ENSURE (ret, "did not create a valid thread"); - ENSURE (ret->state == LUMIERA_THREADSTATE_IDLE, "a non-idle thread found in the idle pool"); TODO ("no error handing, let the resourcecollector do it, no need when returning the thread"); + threadpool.pool[kind].thread_count++; LUMIERA_CONDITION_WAIT (!llist_is_empty (&threadpool.pool[kind].idle_list)); } // use an existing thread, pick the first one @@ -125,19 +133,14 @@ lumiera_threadpool_acquire_thread(enum lumiera_thread_class kind, // move thread to the working_list llist_insert_head (&threadpool.pool[kind].working_list, &ret->node); - threadpool.pool[kind].working_thread_count++; threadpool.pool[kind].idle_thread_count--; // cheaper than using llist_count - ENSURE (threadpool.pool[kind].working_thread_count == - llist_count (&threadpool.pool[kind].working_list), - "working thread count %d is wrong, should be %d", - threadpool.pool[kind].working_thread_count, - llist_count (&threadpool.pool[kind].working_list)); - - ENSURE (threadpool.pool[kind].idle_thread_count == - llist_count (&threadpool.pool[kind].idle_list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[kind].idle_thread_count, - llist_count (&threadpool.pool[kind].idle_list)); + REQUIRE ((int)(llist_count (&threadpool.pool[kind].working_list) + + llist_count (&threadpool.pool[kind].idle_list)) + == threadpool.pool[kind].thread_count, + "threadpool counter miscalculation (working_list count = %u, idle_list count = %u, thread_count = %d )", + llist_count (&threadpool.pool[kind].working_list), + llist_count (&threadpool.pool[kind].idle_list), + threadpool.pool[kind].thread_count); } return ret; } @@ -154,23 +157,20 @@ lumiera_threadpool_release_thread(LumieraThread thread) LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].sync) { thread->state = LUMIERA_THREADSTATE_IDLE; - REQUIRE (llist_is_empty (&thread->node), "thread already belongs to some list"); + REQUIRE (!llist_is_member (&threadpool.pool[thread->kind].idle_list, &thread->node), "thread is already in the idle list"); + // REQUIRE (llist_is_member (&threadpool.pool[thread->kind].working_list, &thread->node), "thread is not in the working list"); // does not make sense for when the thread was just created, check for state==STARTUP // move thread to the idle_list llist_insert_head (&threadpool.pool[thread->kind].idle_list, &thread->node); - threadpool.pool[thread->kind].working_thread_count--; threadpool.pool[thread->kind].idle_thread_count++; // cheaper than using llist_count + REQUIRE ((int)(llist_count (&threadpool.pool[thread->kind].working_list) + + llist_count (&threadpool.pool[thread->kind].idle_list)) + == threadpool.pool[thread->kind].thread_count, + "threadpool counter miscalculation (working_list count = %u, idle_list count = %u, thread_count = %d )", + llist_count (&threadpool.pool[thread->kind].working_list), + llist_count (&threadpool.pool[thread->kind].idle_list), + threadpool.pool[thread->kind].thread_count); - ENSURE (threadpool.pool[thread->kind].idle_thread_count == - llist_count (&threadpool.pool[thread->kind].idle_list), - "idle thread count %d is wrong, should be %d", - threadpool.pool[thread->kind].idle_thread_count, - llist_count (&threadpool.pool[thread->kind].idle_list)); - ENSURE (threadpool.pool[thread->kind].working_thread_count == - llist_count (&threadpool.pool[thread->kind].working_list), - "working thread count %d is wrong, should be %d", - threadpool.pool[thread->kind].working_thread_count, - llist_count (&threadpool.pool[thread->kind].working_list)); REQUIRE (!llist_is_empty (&threadpool.pool[thread->kind].idle_list), "thread pool is still empty after insertion"); LUMIERA_CONDITION_BROADCAST; } diff --git a/src/backend/threadpool.h b/src/backend/threadpool.h index ac3a52ad2..47d90e6fc 100644 --- a/src/backend/threadpool.h +++ b/src/backend/threadpool.h @@ -70,8 +70,8 @@ struct lumiera_threadpool_struct { llist working_list; llist idle_list; - unsigned working_thread_count; - unsigned idle_thread_count; + int thread_count; + int idle_thread_count; pthread_attr_t pthread_attrs; lumiera_condition sync; } pool[LUMIERA_THREADCLASS_COUNT]; diff --git a/src/backend/threads.c b/src/backend/threads.c index 5038c76dc..6eb31533f 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -76,9 +76,6 @@ static void* thread_loop (void* thread) INFO(threads, "function %p", t->function); if (t->function) t->function (t->arguments); - // TODO: problem: - // calling release on a not-yet properly setup thread - // threadpool.pool[thread->kind].working_thread_count == 0 lumiera_threadpool_release_thread(t); LUMIERA_CONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); INFO(threads, "Thread awaken with state %d", t->state); diff --git a/tests/30backend-threadpool.tests b/tests/30backend-threadpool.tests index bd59e0792..b3727bb48 100644 --- a/tests/30backend-threadpool.tests +++ b/tests/30backend-threadpool.tests @@ -10,7 +10,6 @@ END TEST "process a function" process-function < Date: Sat, 16 Jan 2010 12:52:30 -0500 Subject: [PATCH 64/68] ignore RESOURCE_ANNOUNCE in tests makes my threadpool process-function test pass :) --- tests/test.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test.conf b/tests/test.conf index c86e03260..38cdca9cc 100755 --- a/tests/test.conf +++ b/tests/test.conf @@ -1 +1 @@ -LOGSUPPRESS='^\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: \(TRACE\|INFO\|NOTICE\|WARNING\|ERR\|TODO\|PLANNED\|FIXME\|DEPRECATED\|UNIMPLEMENTED\):' +LOGSUPPRESS='^\(\*\*[0-9]*\*\* \)\?[0-9]\{10,\}: \(TRACE\|INFO\|NOTICE\|WARNING\|ERR\|TODO\|PLANNED\|FIXME\|DEPRECATED\|UNIMPLEMENTED\|RESOURCE_ANNOUNCE\):' From fa85a01818a339fee647ca24984d9c99f77eab30 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 13:13:02 -0500 Subject: [PATCH 65/68] add a stronger REQUIRE check --- src/backend/threadpool.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 3f475d9d7..7633229f2 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -156,9 +156,12 @@ lumiera_threadpool_release_thread(LumieraThread thread) REQUIRE (thread->state != LUMIERA_THREADSTATE_IDLE, "trying to park an already idle thread"); LUMIERA_CONDITION_SECTION (threadpool, &threadpool.pool[thread->kind].sync) { - thread->state = LUMIERA_THREADSTATE_IDLE; REQUIRE (!llist_is_member (&threadpool.pool[thread->kind].idle_list, &thread->node), "thread is already in the idle list"); - // REQUIRE (llist_is_member (&threadpool.pool[thread->kind].working_list, &thread->node), "thread is not in the working list"); // does not make sense for when the thread was just created, check for state==STARTUP + REQUIRE (llist_is_member (&threadpool.pool[thread->kind].working_list, &thread->node) + || thread->state == LUMIERA_THREADSTATE_STARTUP, + "thread is not in the working list (state=%s)", + lumiera_threadstate_names[thread->state]); + thread->state = LUMIERA_THREADSTATE_IDLE; // move thread to the idle_list llist_insert_head (&threadpool.pool[thread->kind].idle_list, &thread->node); From c78571be55982c51e0ce708cd4389741acec1039 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 13:53:42 -0500 Subject: [PATCH 66/68] more formatting fixes to put spaces before function/macro call opening brackets --- src/backend/threadpool.c | 2 +- src/backend/threads.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend/threadpool.c b/src/backend/threadpool.c index 7633229f2..ea88b4503 100644 --- a/src/backend/threadpool.c +++ b/src/backend/threadpool.c @@ -46,7 +46,7 @@ void lumiera_threadpool_init(void) { NOBUG_INIT_FLAG (threadpool); - NOBUG_INIT_FLAG(threads); + NOBUG_INIT_FLAG (threads); TRACE (threadpool); NOBUG_INIT_FLAG (threads); diff --git a/src/backend/threads.c b/src/backend/threads.c index 6eb31533f..3dd1bdbae 100644 --- a/src/backend/threads.c +++ b/src/backend/threads.c @@ -62,7 +62,7 @@ const char* lumiera_threadstate_names[] = { static void* thread_loop (void* thread) { TRACE (threads); - NOBUG_THREAD_ID_SET("worker"); + NOBUG_THREAD_ID_SET ("worker"); LumieraThread t = (LumieraThread)thread; pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); @@ -73,16 +73,16 @@ static void* thread_loop (void* thread) { do { // NULL function means: no work to do - INFO(threads, "function %p", t->function); + INFO (threads, "function %p", t->function); if (t->function) t->function (t->arguments); lumiera_threadpool_release_thread(t); - LUMIERA_CONDITION_WAIT(t->state != LUMIERA_THREADSTATE_IDLE); - INFO(threads, "Thread awaken with state %d", t->state); + LUMIERA_CONDITION_WAIT (t->state != LUMIERA_THREADSTATE_IDLE); + INFO (threads, "Thread awaken with state %d", t->state); } while (t->state != LUMIERA_THREADSTATE_SHUTDOWN); // SHUTDOWN state - INFO(threads, "Thread Shutdown"); + INFO (threads, "Thread Shutdown"); } return 0; } @@ -158,7 +158,7 @@ lumiera_thread_destroy (LumieraThread self) // get the pthread out of the processing loop // need to signal to the thread that it should start quitting // should this be within the section? - LUMIERA_CONDITION_SECTION(threads, &self->signal) + LUMIERA_CONDITION_SECTION (threads, &self->signal) { REQUIRE (self->state == LUMIERA_THREADSTATE_IDLE, "trying to delete a thread in state other than IDLE (%s)", lumiera_threadstate_names[self->state]); self->state = LUMIERA_THREADSTATE_SHUTDOWN; @@ -167,11 +167,11 @@ lumiera_thread_destroy (LumieraThread self) LUMIERA_CONDITION_SIGNAL; } - int error = pthread_join(self->id, NULL); - ENSURE (0 == error, "pthread_join returned %d:%s", error, strerror(error)); + int error = pthread_join (self->id, NULL); + ENSURE (0 == error, "pthread_join returned %d:%s", error, strerror (error)); // condition has to be destroyed after joining with the thread - lumiera_condition_destroy (&self->signal, &NOBUG_FLAG(threads)); + lumiera_condition_destroy (&self->signal, &NOBUG_FLAG (threads)); return self; } From de7debc55731adf575b0c1329d65fe1a4fd59957 Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 17:27:02 -0500 Subject: [PATCH 67/68] fix comilation by using an existing TEST macro --- tests/backend/test-threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/backend/test-threads.c b/tests/backend/test-threads.c index b4373c1ff..1a22d361c 100644 --- a/tests/backend/test-threads.c +++ b/tests/backend/test-threads.c @@ -153,7 +153,7 @@ TEST ("mutex_thread") } -PLANNED ("error_cleared_on_join") +TEST ("error_cleared_on_join") { //when a thread/job is finished, there must be no pending errors } From 11f9d6255390c295df84a2c0dc4f0782f86fbebe Mon Sep 17 00:00:00 2001 From: Michael Ploujnikov Date: Sat, 16 Jan 2010 18:42:07 -0500 Subject: [PATCH 68/68] partially fix a pkg-config problem with scons on Fedora12 (happens when nobug and other libs are installed in non-standard paths) --- admin/scons/LumieraEnvironment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/admin/scons/LumieraEnvironment.py b/admin/scons/LumieraEnvironment.py index 44b76ed07..21b2a6c09 100644 --- a/admin/scons/LumieraEnvironment.py +++ b/admin/scons/LumieraEnvironment.py @@ -74,6 +74,7 @@ class LumieraEnvironment(Environment): return False self.libInfo[libID] = libInfo = LumieraEnvironment() + libInfo["ENV"]["PKG_CONFIG_PATH"] = os.environ.get("PKG_CONFIG_PATH") libInfo.ParseConfig ('pkg-config --cflags --libs '+ libID ) if alias: self.libInfo[alias] = libInfo