We shouldn't need lock protection to increment
splitPointsCnt and set curSplitPoint of masterThread.
Anyhow because this code is very tricky and prone to
races bound the change in a single patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Because TT table is shared tte->move() could change
under our feet, in particular we could validate
tte->move() then the move is changed by another
thread and we call pos.do_move() with a move different
from the original validated one !
This leads to a very rare but reproducible crash once
every about 20K games.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
But only when needed, after a split point. This behaviour
does not apply when useSleepingThreads is false, becuase
in this case threads are not woken up at split points so
must be already running.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This method belongs to Thread, not to ThreadsManager.
Reshuffle stuff in thread.cpp while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Release split point lock before to wake up
master thread. This seems to increase speed
in case "sleeping threads" are used:
After 7792 games with 4 threads at very fast TC (2"+0.05)
Mod vs Orig 1722 - 1627 - 4443 ELO +4 (+- 5.1)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When allocating a slave we set both is_searching
and splitPoint under lock protection.
Unfortunatly the order in which the variables are
set is not defined. This article was very clarifying:
http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/
So when in idle loop we test for is_searching and then
access splitPoint, it could happen that splitPoint is still
not updated leading to a possible crash.
Fix the race lock protecting splitPoint access.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With current code we could raise bestValue above beta,
not what is intended for.
Spotted by Richard Vida.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix an issue where the log file stores an incorrect +0.00
eval after a search has been stopped.
Bug reported by Ajedrecista.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This allows to retire ClearMaskBB[] and use just
one SquareBB[] array to set and clear a bit.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is not clear the advantage and we don't want
to risk of introducing regressions on this
very critical parameter. So revert to old limit.
After 16003 games
Mod vs Orig 2496 - 2421 - 11086 ELO +1 (+-3)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Apart from some renaming the biggest change
is the retire of split_point_finished()
replaced by slavesMask flags. As a side
effect we now take also split point lock
when allocation available threads.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
pass references (Windows style) instead of
pointers (Posix style) as function arguments.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case of a PvNode could happen that alpha == beta - 1,
for instance in case the same previous node was visited
with same beta during a non-pv search, the node failed low
and stored beta-1 in TT. Then the node is searched again
in PV mode, TT value beta-1 is retrieved and updates alpha
that now happens to be beta-1.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And increase LMR limit. Tests show no change ELO wise,
but we prefer to take the risk to commit anyhow becuase
is a 'prune reducing' patch.
After 10749 games
Mod vs Orig: 1670 - 1676 - 7403 ELO 0 (+-3.7)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix an incorrect warning: 'bm may be used uninitialized'
with the old but still commonly used gcc 4.4
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And directly pass RootMoves instead of SearchMoves
to main thread. A class declaration is better suited
in a header and slims a bit the fatty search.cpp
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We just need to verify if a legal move is among the
SearchMoves, so we don't need a vector for this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It could lead to terrible mistakes otherwise, as
it happened during a game on playchess when on
this position (after white's f4):
2q4r/4b1k1/p3rpp1/3np2p/PpNpNP1P/1P1P2PQ/2P1R3/4R1K1 b - - 0 1
SF moves immediately e5xf4 instead of the correct f5.
In general during engine matches it is impossible the
opponent leaves a piece hanging or anyhow starts a
clear losing sequence. So avoid to fall in subtle traps.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Further increase safety against time losses. After this
change (tested on LittleBlitzer and cutechess) I had no
more time losses at 2" and 1"+0.02 TC both on Windows
and Linux on more than 10000 games.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We try hard not to lose on time even under extreme
time pressure. We achieve this through 3 different but
coordinated steps:
1) Increase max frequency of timer events
2) Quickly return after a stop signal
3) Take in account timer resolution
With these SF played under LittleBlitzer at 1"+0.02 and 3"+0
without losing on time even one game.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The aim is to have shorter names without losing
readibility but, if possible, increasing it.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Retire open(), close() and name() from public visibility
and greately simplify the code. It is amazing how much
can be squeezed out of an already mature code !
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Greatly improves the usage. User defined conversions
are a novelity for SF, another amazing C++ facility
at work !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The condition for a mate score was wrong:
abs(v) < VALUE_MATE - PLY_MAX * ONE_PLY
instead of
abs(v) < VALUE_MATE_IN_PLY_MAX
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
During the search we score a mate as "plies to mate
from the root" to compare in an homogeneous way the
values returned by different sub-trees. However we
store in TT a mate score as "plies to mate from the
current position" the let the TT value remain valid
across the game.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is simple but somewhat tricky code that deserves
a bit of documentation. A bit of renaming while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add the check that alpha < beta - 1 if and only if PvNode is true.
The current code would not flag PvNode and alpha == beta - 1. In
other words, the || is not an exclusive OR!.
Also sync assert conditions of search() and qsearch()
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Make a better use of C++ operators overloading to
streamline the APIs.
Also sync polyglot.ini file while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Follow the suggested Qt style:
http://doc.qt.nokia.com/qq/qq13-apis.html
It seems to me simpler and easier to read.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I am not able to reproduce the speed regression anymore,
and also we were using cout even before speed regression
so probably the reason is not there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We don't use killers to order evasions, so it
seems natural do not consider an evasion cut-off
move as a possible killer. Test shows almost no
change, as it should be becuase this is a really
tiny change, but neverthless seems the correct
thing to do.
After 11893 games
Mod vs Orig 1773 - 1696 - 8424 ELO +2 (+-3.4)
Idea from Critter.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Take advantage of argument-dependent lookup (ADL) to
avoid specifying std:: qualifier in some STL functions.
When a function argument refers to a namespace (in this
case std) then the compiler will search the unqualified
function in that namespace too.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Seems sensibly faster: On a
./stockfish bench > /dev/null
We have +2% on mingw and even +5% on MSVC !
Also removed the nice but complex enum set960 machinery,
use directly the underlying move_to_uci() function.
Speed regression reported by Heinz van Saanen.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Do not return the book move if is not among the
RootMoves,in particular if we have been asked to
search on a move subset with "searchmoves" then
return book move only if it is among this subset.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
