From: Marco Costalba Date: Mon, 23 Jan 2012 14:20:53 +0000 (+0100) Subject: Simplify locking usage X-Git-Url: https://git.sesse.net/?p=stockfish;a=commitdiff_plain;h=3d937e1e901c59c04cd5d602c05f72892222ded8 Simplify locking usage pass references (Windows style) instead of pointers (Posix style) as function arguments. No functional change. Signed-off-by: Marco Costalba --- diff --git a/src/lock.h b/src/lock.h index 7f74fcf7..44aeabd4 100644 --- a/src/lock.h +++ b/src/lock.h @@ -26,16 +26,19 @@ typedef pthread_mutex_t Lock; typedef pthread_cond_t WaitCondition; +typedef pthread_t ThreadHandle; -# define lock_init(x) pthread_mutex_init(x, NULL) -# define lock_grab(x) pthread_mutex_lock(x) -# define lock_release(x) pthread_mutex_unlock(x) -# define lock_destroy(x) pthread_mutex_destroy(x) -# define cond_destroy(x) pthread_cond_destroy(x) -# define cond_init(x) pthread_cond_init(x, NULL) -# define cond_signal(x) pthread_cond_signal(x) -# define cond_wait(x,y) pthread_cond_wait(x,y) -# define cond_timedwait(x,y,z) pthread_cond_timedwait(x,y,z) +# define lock_init(x) pthread_mutex_init(&(x), NULL) +# define lock_grab(x) pthread_mutex_lock(&(x)) +# define lock_release(x) pthread_mutex_unlock(&(x)) +# define lock_destroy(x) pthread_mutex_destroy(&(x)) +# define cond_destroy(x) pthread_cond_destroy(&(x)) +# define cond_init(x) pthread_cond_init(&(x), NULL) +# define cond_signal(x) pthread_cond_signal(&(x)) +# define cond_wait(x,y) pthread_cond_wait(&(x),&(y)) +# define cond_timedwait(x,y,z) pthread_cond_timedwait(&(x),&(y),z) +# define thread_create(x,f,id) !pthread_create(&(x),NULL,f,&(id)) +# define thread_join(x) pthread_join(x, NULL) #else @@ -50,16 +53,19 @@ typedef pthread_cond_t WaitCondition; // but apart from this they have the same speed performance of SRW locks. typedef CRITICAL_SECTION Lock; typedef HANDLE WaitCondition; +typedef HANDLE ThreadHandle; -# define lock_init(x) InitializeCriticalSection(x) -# define lock_grab(x) EnterCriticalSection(x) -# define lock_release(x) LeaveCriticalSection(x) -# define lock_destroy(x) DeleteCriticalSection(x) -# define cond_init(x) { *x = CreateEvent(0, FALSE, FALSE, 0); } -# define cond_destroy(x) CloseHandle(*x) -# define cond_signal(x) SetEvent(*x) -# define cond_wait(x,y) { lock_release(y); WaitForSingleObject(*x, INFINITE); lock_grab(y); } -# define cond_timedwait(x,y,z) { lock_release(y); WaitForSingleObject(*x,z); lock_grab(y); } +# define lock_init(x) InitializeCriticalSection(&(x)) +# define lock_grab(x) EnterCriticalSection(&(x)) +# define lock_release(x) LeaveCriticalSection(&(x)) +# define lock_destroy(x) DeleteCriticalSection(&(x)) +# define cond_init(x) { x = CreateEvent(0, FALSE, FALSE, 0); } +# define cond_destroy(x) CloseHandle(x) +# define cond_signal(x) SetEvent(x) +# define cond_wait(x,y) { lock_release(y); WaitForSingleObject(x, INFINITE); lock_grab(y); } +# define cond_timedwait(x,y,z) { lock_release(y); WaitForSingleObject(x,z); lock_grab(y); } +# define thread_create(x,f,id) (x = CreateThread(NULL,0,f,&(id),0,NULL), x != NULL) +# define thread_join(x) { WaitForSingleObject(x, INFINITE); CloseHandle(x); } #endif diff --git a/src/misc.cpp b/src/misc.cpp index d436ee92..ae8b6ffb 100644 --- a/src/misc.cpp +++ b/src/misc.cpp @@ -153,7 +153,7 @@ int cpu_count() { /// timed_wait() waits for msec milliseconds. It is mainly an helper to wrap /// conversion from milliseconds to struct timespec, as used by pthreads. -void timed_wait(WaitCondition* sleepCond, Lock* sleepLock, int msec) { +void timed_wait(WaitCondition& sleepCond, Lock& sleepLock, int msec) { #if defined(_MSC_VER) int tm = msec; diff --git a/src/misc.h b/src/misc.h index d6fb062f..0569e468 100644 --- a/src/misc.h +++ b/src/misc.h @@ -29,7 +29,7 @@ extern const std::string engine_info(bool to_uci = false); extern int system_time(); extern int cpu_count(); -extern void timed_wait(WaitCondition*, Lock*, int); +extern void timed_wait(WaitCondition&, Lock&, int); extern void prefetch(char* addr); extern void dbg_hit_on(bool b); diff --git a/src/search.cpp b/src/search.cpp index eb9f63f1..2720edb3 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -822,7 +822,7 @@ split_point_start: // At split points actual search starts from here && tte->depth() >= depth - 3 * ONE_PLY; if (SpNode) { - lock_grab(&(sp->lock)); + lock_grab(sp->lock); bestValue = sp->bestValue; moveCount = sp->moveCount; @@ -854,7 +854,7 @@ split_point_start: // At split points actual search starts from here if (SpNode) { moveCount = ++sp->moveCount; - lock_release(&(sp->lock)); + lock_release(sp->lock); } else moveCount++; @@ -925,7 +925,7 @@ split_point_start: // At split points actual search starts from here && (!threatMove || !connected_threat(pos, move, threatMove))) { if (SpNode) - lock_grab(&(sp->lock)); + lock_grab(sp->lock); continue; } @@ -940,7 +940,7 @@ split_point_start: // At split points actual search starts from here if (futilityValue < beta) { if (SpNode) - lock_grab(&(sp->lock)); + lock_grab(sp->lock); continue; } @@ -950,7 +950,7 @@ split_point_start: // At split points actual search starts from here && pos.see_sign(move) < 0) { if (SpNode) - lock_grab(&(sp->lock)); + lock_grab(sp->lock); continue; } @@ -1015,7 +1015,7 @@ split_point_start: // At split points actual search starts from here // Step 18. Check for new best move if (SpNode) { - lock_grab(&(sp->lock)); + lock_grab(sp->lock); bestValue = sp->bestValue; alpha = sp->alpha; } @@ -1134,7 +1134,7 @@ split_point_start: // At split points actual search starts from here // Here we have the lock still grabbed sp->is_slave[pos.thread()] = false; sp->nodes += pos.nodes_searched(); - lock_release(&(sp->lock)); + lock_release(sp->lock); } assert(bestValue > -VALUE_INFINITE && bestValue < VALUE_INFINITE); @@ -1858,12 +1858,12 @@ void Thread::idle_loop(SplitPoint* sp) { } // Grab the lock to avoid races with Thread::wake_up() - lock_grab(&sleepLock); + lock_grab(sleepLock); // If we are master and all slaves have finished don't go to sleep if (sp && Threads.split_point_finished(sp)) { - lock_release(&sleepLock); + lock_release(sleepLock); break; } @@ -1872,9 +1872,9 @@ void Thread::idle_loop(SplitPoint* sp) { // in the meanwhile, allocated us and sent the wake_up() call before we // had the chance to grab the lock. if (do_sleep || !is_searching) - cond_wait(&sleepCond, &sleepLock); + cond_wait(sleepCond, sleepLock); - lock_release(&sleepLock); + lock_release(sleepLock); } // If this thread has been assigned work, launch a search @@ -1917,8 +1917,8 @@ void Thread::idle_loop(SplitPoint* sp) { { // Because sp->is_slave[] is reset under lock protection, // be sure sp->lock has been released before to return. - lock_grab(&(sp->lock)); - lock_release(&(sp->lock)); + lock_grab(sp->lock); + lock_release(sp->lock); return; } } diff --git a/src/thread.cpp b/src/thread.cpp index c7d9d410..2c40ef6f 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -64,9 +64,9 @@ namespace { extern "C" { void Thread::wake_up() { - lock_grab(&sleepLock); - cond_signal(&sleepCond); - lock_release(&sleepLock); + lock_grab(sleepLock); + cond_signal(sleepCond); + lock_release(sleepLock); } @@ -153,17 +153,17 @@ void ThreadsManager::set_size(int cnt) { void ThreadsManager::init() { // Initialize sleep condition and lock used by thread manager - cond_init(&sleepCond); - lock_init(&threadsLock); + cond_init(sleepCond); + lock_init(threadsLock); // Initialize thread's sleep conditions and split point locks for (int i = 0; i <= MAX_THREADS; i++) { - lock_init(&threads[i].sleepLock); - cond_init(&threads[i].sleepCond); + lock_init(threads[i].sleepLock); + cond_init(threads[i].sleepCond); for (int j = 0; j < MAX_ACTIVE_SPLIT_POINTS; j++) - lock_init(&(threads[i].splitPoints[j].lock)); + lock_init(threads[i].splitPoints[j].lock); } // Allocate main thread tables to call evaluate() also when not searching @@ -177,12 +177,7 @@ void ThreadsManager::init() { threads[i].do_sleep = (i != 0); // Avoid a race with start_thinking() threads[i].threadID = i; -#if defined(_MSC_VER) - threads[i].handle = CreateThread(NULL, 0, start_routine, &threads[i], 0, NULL); - bool ok = (threads[i].handle != NULL); -#else - bool ok = !pthread_create(&threads[i].handle, NULL, start_routine, &threads[i]); -#endif + bool ok = thread_create(threads[i].handle, start_routine, threads[i]); if (!ok) { @@ -202,24 +197,18 @@ void ThreadsManager::exit() { threads[i].do_terminate = true; // Search must be already finished threads[i].wake_up(); - // Wait for thread termination -#if defined(_MSC_VER) - WaitForSingleObject(threads[i].handle, INFINITE); - CloseHandle(threads[i].handle); -#else - pthread_join(threads[i].handle, NULL); -#endif + thread_join(threads[i].handle); // Wait for thread termination // Now we can safely destroy associated locks and wait conditions - lock_destroy(&threads[i].sleepLock); - cond_destroy(&threads[i].sleepCond); + lock_destroy(threads[i].sleepLock); + cond_destroy(threads[i].sleepCond); for (int j = 0; j < MAX_ACTIVE_SPLIT_POINTS; j++) - lock_destroy(&(threads[i].splitPoints[j].lock)); + lock_destroy(threads[i].splitPoints[j].lock); } - lock_destroy(&threadsLock); - cond_destroy(&sleepCond); + lock_destroy(threadsLock); + cond_destroy(sleepCond); } @@ -310,7 +299,7 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, // Try to allocate available threads and ask them to start searching setting // is_searching flag. This must be done under lock protection to avoid concurrent // allocation of the same slave by another master. - lock_grab(&threadsLock); + lock_grab(threadsLock); for (i = 0; !Fake && i < activeThreads && workersCnt < maxThreadsPerSplitPoint; i++) if (threads[i].is_available_to(master)) @@ -326,7 +315,7 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, threads[i].wake_up(); } - lock_release(&threadsLock); + lock_release(threadsLock); // We failed to allocate even one slave, return if (!Fake && workersCnt == 1) @@ -349,12 +338,12 @@ Value ThreadsManager::split(Position& pos, Stack* ss, Value alpha, Value beta, // We have returned from the idle loop, which means that all threads are // finished. Note that changing state and decreasing activeSplitPoints is done // under lock protection to avoid a race with Thread::is_available_to(). - lock_grab(&threadsLock); + lock_grab(threadsLock); masterThread.is_searching = true; masterThread.activeSplitPoints--; - lock_release(&threadsLock); + lock_release(threadsLock); masterThread.splitPoint = sp->parent; pos.set_nodes_searched(pos.nodes_searched() + sp->nodes); @@ -375,9 +364,9 @@ void Thread::timer_loop() { while (!do_terminate) { - lock_grab(&sleepLock); - timed_wait(&sleepCond, &sleepLock, maxPly ? maxPly : INT_MAX); - lock_release(&sleepLock); + lock_grab(sleepLock); + timed_wait(sleepCond, sleepLock, maxPly ? maxPly : INT_MAX); + lock_release(sleepLock); check_time(); } } @@ -390,10 +379,10 @@ void ThreadsManager::set_timer(int msec) { Thread& timer = threads[MAX_THREADS]; - lock_grab(&timer.sleepLock); + lock_grab(timer.sleepLock); timer.maxPly = msec; - cond_signal(&timer.sleepCond); // Wake up and restart the timer - lock_release(&timer.sleepLock); + cond_signal(timer.sleepCond); // Wake up and restart the timer + lock_release(timer.sleepLock); } @@ -404,20 +393,20 @@ void Thread::main_loop() { while (true) { - lock_grab(&sleepLock); + lock_grab(sleepLock); do_sleep = true; // Always return to sleep after a search is_searching = false; while (do_sleep && !do_terminate) { - cond_signal(&Threads.sleepCond); // Wake up UI thread if needed - cond_wait(&sleepCond, &sleepLock); + cond_signal(Threads.sleepCond); // Wake up UI thread if needed + cond_wait(sleepCond, sleepLock); } is_searching = true; - lock_release(&sleepLock); + lock_release(sleepLock); if (do_terminate) return; @@ -436,11 +425,11 @@ void ThreadsManager::start_thinking(const Position& pos, const LimitsType& limit const std::set& searchMoves, bool async) { Thread& main = threads[0]; - lock_grab(&main.sleepLock); + lock_grab(main.sleepLock); // Wait main thread has finished before to launch a new search while (!main.do_sleep) - cond_wait(&sleepCond, &main.sleepLock); + cond_wait(sleepCond, main.sleepLock); // Copy input arguments to initialize the search RootPosition.copy(pos, 0); @@ -458,13 +447,13 @@ void ThreadsManager::start_thinking(const Position& pos, const LimitsType& limit Signals.stop = Signals.failedLowAtRoot = false; main.do_sleep = false; - cond_signal(&main.sleepCond); // Wake up main thread and start searching + cond_signal(main.sleepCond); // Wake up main thread and start searching if (!async) while (!main.do_sleep) - cond_wait(&sleepCond, &main.sleepLock); + cond_wait(sleepCond, main.sleepLock); - lock_release(&main.sleepLock); + lock_release(main.sleepLock); } @@ -478,14 +467,14 @@ void ThreadsManager::stop_thinking() { Search::Signals.stop = true; - lock_grab(&main.sleepLock); + lock_grab(main.sleepLock); - cond_signal(&main.sleepCond); // In case is waiting for stop or ponderhit + cond_signal(main.sleepCond); // In case is waiting for stop or ponderhit while (!main.do_sleep) - cond_wait(&sleepCond, &main.sleepLock); + cond_wait(sleepCond, main.sleepLock); - lock_release(&main.sleepLock); + lock_release(main.sleepLock); } @@ -502,10 +491,10 @@ void ThreadsManager::wait_for_stop_or_ponderhit() { Thread& main = threads[0]; - lock_grab(&main.sleepLock); + lock_grab(main.sleepLock); while (!Signals.stop) - cond_wait(&main.sleepCond, &main.sleepLock); + cond_wait(main.sleepCond, main.sleepLock); - lock_release(&main.sleepLock); + lock_release(main.sleepLock); } diff --git a/src/thread.h b/src/thread.h index c762f130..80363fa7 100644 --- a/src/thread.h +++ b/src/thread.h @@ -80,17 +80,12 @@ struct Thread { int maxPly; Lock sleepLock; WaitCondition sleepCond; + ThreadHandle handle; SplitPoint* volatile splitPoint; volatile int activeSplitPoints; volatile bool is_searching; volatile bool do_sleep; volatile bool do_terminate; - -#if defined(_MSC_VER) - HANDLE handle; -#else - pthread_t handle; -#endif };