This is the first nice effect of previous patch !
Because thread_should_stop() should be declared 'const' we
need to remove the setting of 'stop' flag to true that
turns out to be a bug because thread_should_stop() is called
outside from lock protection while 'stop' flag is a volatile
shared variable so cannot be changed when not in lock.
Note that this bugs fires ONLY when we use more then 2 threads,
so commonly only in a QUAD or OCTAL machine.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Main aim of this patch is to consolidate all the thread related stuff
behind a single class interface so to avoid messing with global flags
and having thread code scattered among non-thread related stuff.
Another advantage is that now access to thread's variables is
more controlled, in particular we can differentiate between
read and write accesses by the mean of different interfaces, it
is so simpler to understand how a function is related to threads.
Lastly this rewrite is the base for future code consolidations and
semplifications that are easier now that we have only one thread's
access point.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Ensure threads are sleeping when leaving init_threads() and
the newly introduced put_threads_to_sleep().
Also ensure threads are not sleeping when leaving
wake_sleeping_threads().
As a side effect we now leave think() with all the threads
(but the main one) guaranteed to sleep. So when we enter
again in think(), after the opponent next move, we know
threads must be sleeping.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Wait inside wake_sleeping_threads() for the threads to be
effectively and reliably woken up.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Will be used by future patches. Also:
- Renamed Idle in AllThreadsShouldSleep
- Explicitly inited AllThreadsShouldExit and AllThreadsShouldSleep
in init_thread() instead of use an anonymous global initialization.
- Rewritten idle_loop() while condition to avoid a 'break' statement
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is an hidden bug waiting to fire. The main problem is
that ss[ply] is overwritten by search() and qsearch() called
from IID and razoring, so that we cannot hold a pointer to a
local EvalInfo variable.
For instance if we go razoring then we overwrite the pointer
with the address of a variable local to qsearch(), when we return
from qsearch() variable goes out of scope and now ss[ply].evalInfo
holds a stale pointer !
Because we are not looking for troubles we go through the
safe route and we remove it entirely.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In search routines we use information from previous ply
and init killers two plies ahead.
So for me it seems correct to copy 4 searchstack items
in split:
ply - 1, ply, ply + 1, ply + 2
Because
a) we do not split at root (ply == 0)
b) ply < PLY_MAX and SearchStack size is PLY_MAX_PLUS_2
there should be no risk of underflows or overflows
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It was only used to control StopOnPonderHit variable.
Now use FailLow variable instead.
Patch has a minor effect on time management when ponder is on.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
noProblemFound condition is never true.
This was verified by running 800 games 1+0 match in 1 CPU computer.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of piece-from-to, in this way it is similar
to what we already do for history.
Almost no change, but seems a bit simpler in this way.
After 995 games at 1+0
Mod vs Orig +207 =596 -192 +5 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems more standard conformant. Also added a bit of
description directly from Tord.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Futility captures alone does not seem an improvment.
Perhaps is a combination of stand pat + futility that is winning,
so revert for now and continue testing starting from a standard
base until we find the correct receipe.
After 999 games at 1+0
Mod vs Orig +231 0506 -201 +10 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Not a biggie but is a reduced pruned patch that doesn't
seems to hurt, so it is welcomed ;-)
After 999 games at 1+0
Mod vs Orig +207 =601 -191 +6 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
At ply == OnePly (common case) we avoid some useless
floating point computation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Joona new aspiration window. Main idea is to always
research aspiration fail highs/low at the same
ply and use much smaller aspiration window than previously.
Testing result is very positive.
1CPU:
953-1149
4CPU:
545 - 656
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Avoid to take the lock two times in a tight sequence, the first
in get_next_move() and the second to update sp->moves.
Do all with one lock and so retire the now useless locked version
of get_next_move().
Also fix some theorical race due to comparison sp->bestValue < sp->beta
is done out of lock protection. Finally fix another (harmless but time
waster) race that coudl occur because thread_should_stop() is also
called outside of lock protection.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In debug run with 2 threads it happens to be following
assert after some minutes:
assert(value > -VALUE_INFINITE && value < VALUE_INFINITE);
in search(), line 1615.
I am not able to understand why, anyhow reverted for the moment.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This avoid us to forget some very needed tests now that
futility has changed in a whole big chunk we need to fine
tuning every splitted change.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This will be useful to use gains table in move
ordering along with history table.
No functional change and big code remove.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
