diff --git a/changelog.in b/changelog.in index 5a4e863f65..d08794e5fe 100644 --- a/changelog.in +++ b/changelog.in @@ -68,7 +68,7 @@ [RELEASE] Version: 6.0.0 -Date: 2018-02-07 +Date: 2018-02-´23 [DESCRIPTION] This major release fixes many bugs, adds quite some new functionality, and changes how cloning works (most likely the @@ -81,6 +81,21 @@ by an array of variables), a new much faster implementation of extensional constraints using tuple sets, and support for tracing search engines. +[ENTRY] +Module: search +What: bug +Rank: minor +[DESCRIPTION] +The statistics for parallel search engines was wrong. + +[ENTRY] +Module: search +What: bug +Rank: major +[DESCRIPTION] +Fixed several race conditions during the termination of parallel +search engines. + [ENTRY] Module: support What: change diff --git a/gecode/kernel/memory/manager.cpp b/gecode/kernel/memory/manager.cpp index f725bc0849..6c3305cc8f 100644 --- a/gecode/kernel/memory/manager.cpp +++ b/gecode/kernel/memory/manager.cpp @@ -39,7 +39,10 @@ namespace Gecode { namespace Kernel { - Support::Mutex SharedMemory::m; + Support::Mutex& SharedMemory::m(void) { + static Support::Mutex _m; + return _m; + } void MemoryManager::alloc_refill(SharedMemory& sm, size_t sz) { diff --git a/gecode/kernel/memory/manager.hpp b/gecode/kernel/memory/manager.hpp index 0ab427b411..ca0cfe281a 100755 --- a/gecode/kernel/memory/manager.hpp +++ b/gecode/kernel/memory/manager.hpp @@ -71,7 +71,7 @@ namespace Gecode { namespace Kernel { HeapChunk* hc; } heap; /// A mutex for access - GECODE_KERNEL_EXPORT static Support::Mutex m; + GECODE_KERNEL_EXPORT static Support::Mutex& m(void); public: /// Initialize SharedMemory(void); @@ -200,7 +200,7 @@ namespace Gecode { namespace Kernel { forceinline HeapChunk* SharedMemory::alloc(size_t s, size_t l) { - m.acquire(); + m().acquire(); while ((heap.hc != NULL) && (heap.hc->size < l)) { heap.n_hc--; HeapChunk* hc = heap.hc; @@ -217,19 +217,19 @@ namespace Gecode { namespace Kernel { hc = heap.hc; heap.hc = static_cast(hc->next); } - m.release(); + m().release(); return hc; } forceinline void SharedMemory::free(HeapChunk* hc) { - m.acquire(); + m().acquire(); if (heap.n_hc == MemoryConfig::n_hc_cache) { Gecode::heap.rfree(hc); } else { heap.n_hc++; hc->next = heap.hc; heap.hc = hc; } - m.release(); + m().release(); } diff --git a/gecode/kernel/memory/region.hpp b/gecode/kernel/memory/region.hpp index 358f5fad33..84ce6fec97 100755 --- a/gecode/kernel/memory/region.hpp +++ b/gecode/kernel/memory/region.hpp @@ -92,7 +92,7 @@ namespace Gecode { ~Pool(void); }; /// Just use a single static pool for heap chunks - GECODE_KERNEL_EXPORT Pool& pool(); + GECODE_KERNEL_EXPORT static Pool& pool(); /// Heap information data structure class HeapInfo { public: diff --git a/gecode/search/par/bab.hpp b/gecode/search/par/bab.hpp index d499342169..963608a0dd 100644 --- a/gecode/search/par/bab.hpp +++ b/gecode/search/par/bab.hpp @@ -165,7 +165,9 @@ namespace Gecode { namespace Search { namespace Par { mark = 0; if (best != NULL) cur->constrain(*best); + Statistics t = *this; Search::Worker::reset(r_d); + (*this) += t; m.release(); return; } @@ -240,8 +242,7 @@ namespace Gecode { namespace Search { namespace Par { engine().ack_terminate(); // Wait until termination can proceed engine().wait_terminate(); - // Terminate thread - engine().terminated(); + // Thread will be terminated by returning from run return; case C_RESET: // Acknowledge reset request diff --git a/gecode/search/par/dfs.hpp b/gecode/search/par/dfs.hpp index 338534dd96..5bdc651d07 100644 --- a/gecode/search/par/dfs.hpp +++ b/gecode/search/par/dfs.hpp @@ -136,7 +136,9 @@ namespace Gecode { namespace Search { namespace Par { path.ngdl(0); d = 0; cur = s; + Statistics t = *this; Search::Worker::reset(r_d); + (*this) += t; m.release(); return; } @@ -188,8 +190,7 @@ namespace Gecode { namespace Search { namespace Par { engine().ack_terminate(); // Wait until termination can proceed engine().wait_terminate(); - // Terminate thread - engine().terminated(); + // Thread will be terminated by returning from run return; case C_RESET: // Acknowledge reset request diff --git a/gecode/search/par/engine.hh b/gecode/search/par/engine.hh index 9bc1fa8a58..74ca6c44af 100644 --- a/gecode/search/par/engine.hh +++ b/gecode/search/par/engine.hh @@ -47,7 +47,7 @@ namespace Gecode { namespace Search { namespace Par { /// %Parallel depth-first search engine template - class Engine : public Search::Engine { + class Engine : public Search::Engine, public Support::Terminator { protected: /// %Parallel depth-first search worker class Worker : public Search::Worker, public Support::Runnable { @@ -80,6 +80,8 @@ namespace Gecode { namespace Search { namespace Par { NoGoods& nogoods(void); /// Destructor virtual ~Worker(void); + /// Terminator (engine) + virtual Support::Terminator* terminator(void) const; }; /// Search options Options _opt; @@ -133,7 +135,7 @@ namespace Gecode { namespace Search { namespace Par { /// For worker to acknowledge termination command void ack_terminate(void); /// For worker to register termination - void terminated(void); + virtual void terminated(void); /// For worker to wait until termination is legal void wait_terminate(void); /// For engine to peform thread termination diff --git a/gecode/search/par/engine.hpp b/gecode/search/par/engine.hpp index 09c72c7342..db629cc446 100644 --- a/gecode/search/par/engine.hpp +++ b/gecode/search/par/engine.hpp @@ -231,8 +231,6 @@ namespace Gecode { namespace Search { namespace Par { // Now all threads are terminated! } - - /* * Engine: reset control */ @@ -351,6 +349,12 @@ namespace Gecode { namespace Search { namespace Par { return NULL; } + template + Support::Terminator* + Engine::Worker::terminator(void) const { + return &_engine; + } + /* * Termination and deletion */ diff --git a/gecode/support/thread.hpp b/gecode/support/thread.hpp index ed5b3054fe..087393239e 100755 --- a/gecode/support/thread.hpp +++ b/gecode/support/thread.hpp @@ -104,13 +104,17 @@ namespace Gecode { namespace Support { */ class Mutex { private: -#ifdef GECODE_THREADS_WINDOWS +#if defined(GECODE_THREADS_WINDOWS) /// Use a simple but more efficient critical section on Windows CRITICAL_SECTION w_cs; -#endif -#ifdef GECODE_THREADS_PTHREADS +#elif defined(GECODE_THREADS_OSX_UNFAIR) + /// Use unfair lock on macOS + os_unfair_lock lck; +#elif defined(GECODE_THREADS_PTHREADS) /// The Pthread mutex pthread_mutex_t p_m; +#else +#error No suitable mutex implementation found #endif public: /// Initialize mutex @@ -134,15 +138,11 @@ namespace Gecode { namespace Support { void operator=(const Mutex&) {} }; -#if defined(GECODE_THREADS_WINDOWS) || !defined(GECODE_THREADS_PTHREADS) +#ifndef GECODE_THREADS_PTHREADS_SPINLOCK typedef Mutex FastMutex; -#endif - -#ifdef GECODE_THREADS_PTHREADS - -#if defined(GECODE_THREADS_OSX) || defined(GECODE_THREADS_OSX_UNFAIR) || defined(GECODE_THREADS_PTHREADS_SPINLOCK) +#else /** * \brief A fast mutex for mutual exclausion among several threads @@ -160,16 +160,8 @@ namespace Gecode { namespace Support { */ class FastMutex { private: -#ifdef GECODE_THREADS_OSX - /// The OSX spin lock - OSSpinLock lck; -#elif defined(GECODE_THREADS_OSX_UNFAIR) - /// The OSX spin lock - os_unfair_lock lck; -#else /// The Pthread spinlock pthread_spinlock_t p_s; -#endif public: /// Initialize mutex FastMutex(void); @@ -192,12 +184,6 @@ namespace Gecode { namespace Support { void operator=(const FastMutex&) {} }; -#else - - typedef Mutex FastMutex; - -#endif - #endif /** @@ -259,6 +245,20 @@ namespace Gecode { namespace Support { void operator=(const Event&) {} }; + /** + * \brief An interface for objects that can be called after a + * thread has terminated (after running the thread's destructor) + * + * \ingroup FuncSupportThread + */ + class Terminator { + public: + /// Destructor + virtual ~Terminator() {} + /// The function that is called when the thread has terminated + virtual void terminated(void) = 0; + }; + /** * \brief An interface for objects that can be run by a thread * @@ -275,6 +275,8 @@ namespace Gecode { namespace Support { void todelete(bool d); /// Return whether to be deleted upon termination bool todelete(void) const; + /// Return terminator object + virtual Terminator* terminator(void) const { return NULL; } /// The function that is executed when the thread starts virtual void run(void) = 0; /// Destructor diff --git a/gecode/support/thread/pthreads.hpp b/gecode/support/thread/pthreads.hpp index 81264e7270..da085f8fac 100755 --- a/gecode/support/thread/pthreads.hpp +++ b/gecode/support/thread/pthreads.hpp @@ -43,6 +43,30 @@ namespace Gecode { namespace Support { +#ifdef GECODE_THREADS_OSX_UNFAIR + + /* + * Mutex + */ + forceinline + Mutex::Mutex(void) : lck(OS_UNFAIR_LOCK_INIT) {} + forceinline void + Mutex::acquire(void) { + os_unfair_lock_lock(&lck); + } + forceinline bool + Mutex::tryacquire(void) { + return os_unfair_lock_trylock(&lck); + } + forceinline void + Mutex::release(void) { + os_unfair_lock_unlock(&lck); + } + forceinline + Mutex::~Mutex(void) {} + +#else + /* * Mutex */ @@ -73,55 +97,8 @@ namespace Gecode { namespace Support { std::terminate(); } } - -#ifdef GECODE_THREADS_OSX - - /* - * FastMutex - */ - forceinline - FastMutex::FastMutex(void) : lck(OS_SPINLOCK_INIT) {} - forceinline void - FastMutex::acquire(void) { - OSSpinLockLock(&lck); - } - forceinline bool - FastMutex::tryacquire(void) { - return OSSpinLockTry(&lck); - } - forceinline void - FastMutex::release(void) { - OSSpinLockUnlock(&lck); - } - forceinline - FastMutex::~FastMutex(void) {} - -#endif - -#ifdef GECODE_THREADS_OSX_UNFAIR - - /* - * FastMutex - */ - forceinline - FastMutex::FastMutex(void) {} - forceinline void - FastMutex::acquire(void) { - os_unfair_lock_lock(&lck); - } - forceinline bool - FastMutex::tryacquire(void) { - return os_unfair_lock_trylock(&lck); - } - forceinline void - FastMutex::release(void) { - os_unfair_lock_unlock(&lck); - } - forceinline - FastMutex::~FastMutex(void) {} - #endif - + #ifdef GECODE_THREADS_PTHREADS_SPINLOCK /* diff --git a/gecode/support/thread/thread.cpp b/gecode/support/thread/thread.cpp index a0cfde1dab..7f0a92f716 100755 --- a/gecode/support/thread/thread.cpp +++ b/gecode/support/thread/thread.cpp @@ -61,8 +61,12 @@ namespace Gecode { namespace Support { m.release(); assert(e != NULL); e->run(); - if (e->todelete()) + if (e->todelete()) { + Terminator* t = e->terminator(); delete e; + if (t) + t->terminated(); + } } // Put into idle stack Thread::m()->acquire(); diff --git a/gecode/support/thread/thread.hpp b/gecode/support/thread/thread.hpp index a66ba2b9d4..caf3d636ea 100755 --- a/gecode/support/thread/thread.hpp +++ b/gecode/support/thread/thread.hpp @@ -60,7 +60,6 @@ namespace Gecode { namespace Support { return heap.ralloc(s); } - /* * Mutexes * @@ -75,7 +74,7 @@ namespace Gecode { namespace Support { Gecode::heap.rfree(p); } -#if defined(GECODE_THREADS_OSX) || defined(GECODE_THREADS_OSX_UNFAIR) || defined(GECODE_THREADS_PTHREADS_SPINLOCK) +#if ! ( defined(GECODE_THREADS_WINDOWS) || defined(GECODE_THREADS_OSX_UNFAIR) || !defined(GECODE_THREADS_PTHREADS_SPINLOCK) ) /* * Fast mutexes