```
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 92736 W: 24149 L: 23764 D: 44823
Ptnml(0-2): 280, 10056, 25334, 10395, 303
```
The use of `IndexType` in the loops is suboptimal because it requires
truncation to 32 bits, and thereby defeats some optimizations. Using
`size_t` for the loop body works nicely, and a signed index into the
index lists allows the compiler to assume the case `added.size() ==
UINT_MAX`, which would be an infinite loop, to not happen.
Passed STC
https://tests.stockfishchess.org/tests/live_elo/692fd98ab23dfeae38d01d98
closes https://github.com/official-stockfish/Stockfish/pull/6466
No functional change
The error is "C1001: Internal compiler error." in uci.cpp on line 370 of
master. For some reason the compiler can't handle the
std::size(hashfullAges) inside the lambda. Older MSVC versions had no
problem.
Most of the warnings are due to implicit type conversions "conversion
from type A to type B, possible loss of data"
many of which have been present for a while.
closes https://github.com/official-stockfish/Stockfish/pull/6464
No functional change
Remove a couple unused things, mark `noRaysContaining` as
`[[maybe_unused]]` (suggested by Viz) to silence a warning on GCC 10,
update a comment, and replace inline constants
closes https://github.com/official-stockfish/Stockfish/pull/6460
No functional change
[Passed STC](https://tests.stockfishchess.org/tests/live_elo/691fc321acb6dbdf23d07e4b):
```
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 215360 W: 55651 L: 55108 D: 104601
Ptnml(0-2): 520, 22399, 61290, 22960, 511
```
This PR is on top of ces42's work so I'll rebase if that PR changes.
Essentially, it comprises my adjustments to `make_index` that remained
unmerged before threat inputs was finalized.
The unsigned intermediate variables let us skip a bunch of useless sign
extensions (because Square and Piece inherit from `int8_t`, and C/C++
semantics require narrow integers to be promoted to `int` before doing
arithmetic on them). Additionally, by removing the special usage of
`offsets[][64]` and `[65]` the indexing becomes more efficient. (This
usage was a temporary hack from sscg anyway, so I think he'll like that
it's gone.) Finally, the `sf_assume` condition was fixed so that it
actually makes a difference to the compiler.
The speedup is fairly nice locally (combining both ces42's and these changes):
```
Result of 100 runs
==================
base (...kfish.master) = 1691982 +/- 1907
test (./stockfish ) = 1714349 +/- 1789
diff = +22367 +/- 2465
speedup = +0.0132
P(speedup > 0) = 1.0000
```
closes https://github.com/official-stockfish/Stockfish/pull/6445
No functional change
The contents of DirtyThreats gets memcpyed twice for each call to do_move.
(Return value optimization doesn't apply to the do_move function itself because
it constructs a std::pair, so it gets copied; and the calls to reset also
require a copy.) This patch inserts the dirty info in place.
Sometimes the caller of do_move ignores the DirtyThreats info, so we pass in
scratch objects. I found that stack-allocating these scratch objects was bad on
Fishtest, so I begrudgingly put them in the Position struct. Both templating
the do_move function on whether dirty threats are needed and putting a
null-check branch for each use of dirty threats were slowdowns locally. Of
course, nothing prevents a future attempt at cleaning this up.
passed STC
LLR: 2.96 (-2.94,2.94) <0.00,2.00>
Total: 68448 W: 17770 L: 17418 D: 33260
Ptnml(0-2): 198, 7425, 18630, 7769, 202
https://tests.stockfishchess.org/tests/view/6914c01a7ca87818523318ba
closes https://github.com/official-stockfish/Stockfish/pull/6414
No functional change
This enables different Stockfish processes that use the same weights to use the
same memory. The approach establishes equivalence by memory content, and is
compatible with NUMA replication. The benefit of sharing is reduced memory usage
and a speedup thanks to improved (inter-process) caching of the network in the
CPUs cache, and thus reduced bandwidth usage to main memory. Even though this
change doesn't benefit a user running a single process, this helps on fishtest
or e.g. for Lichess, when multiple games run concurrently, or multiple
positions are analyzed in parallel.
This concept was probably first introduced in the Monty engine
(https://github.com/official-monty/Monty/pull/62), after a discussion in
https://github.com/official-stockfish/fishtest/issues/2077 on the issue of
memory pressure. Measurements based on Torch
(https://github.com/user-attachments/files/21386224/verbatim.pdf) further
suggested that large gains were possible. Multiple other engines have
adopted this 'verbatim' format as well.
The implementation here adds the flexibility needed for SF, for example, retains
the ability to bundle compressed networks with the binary, to load nets by uci
option, and to distribute the shared nets to the proper NUMA region. This
flexibility comes with a fair amount of complexity in the implementation, such
as OS specific code, and fallback code.
For most users this should be transparent. However, for example, those running
docker containers should ensure the `--ipc` flag is set correctly, and
`--shm-size` is sufficiently large.
The benefits of this patch significantly depend on hardware, with systems with
many cores and a large (O(150MB), the net size) L3 cache benefitting typically
most. On such systems SF speedups (as measured via nps playing games with
large concurrency but just 1 thread) can be 38%, which results in master vs.
patch Elo which gains about 25 Elo.
```
# PLAYER : RATING ERROR POINTS PLAYED (%)
1 shared_memoryPR : 24.8 1.9 39432.0 73728 53
2 master : 0.0 ---- 34296.0 73728 47
```
In a multithreaded setup, where weights are already shared, that benefit is smaller,
for example on the same HW as above, but with 8t for each side.
```
# PLAYER : RATING ERROR POINTS PLAYED (%)
1 shared_memoryPR : 5.2 3.5 9351.0 18432 51
2 master : 0.0 ---- 9081.0 18432 49
```
On fishtest with a typical hardware mix of our contributors, the following was measured:
STC, 60k games
https://tests.stockfishchess.org/tests/view/69074a49ea4b268f1fac236c
Elo: 4.69 ± 1.4 (95%) LOS: 100.0%
Total: 60000 W: 16085 L: 15275 D: 28640
Ptnml(0-2): 154, 6440, 16053, 7148, 205
nElo: 9.38 ± 2.8 (95%) PairsRatio: 1.12
To verify correctness with a single process on a NUMA architecture,
speedtest was used, confirming near equivalence:
```
master: Average (over 10): 296236186
shared_memory: Average (over 10): 295769332
```
Currently, using large pages for the shared network weights is not always possible,
which can lead to a small slowdown (1-2%), in case a single process is run.
closes https://github.com/official-stockfish/Stockfish/pull/6173
No functional change
Co-authored-by: disservin <disservin.social@gmail.com>
Co-authored-by: Joost VandeVondele <Joost.VandeVondele@gmail.com>
The speedup seems to vary by machine. The indexing function can be changed w/o needing to understand intrinsics.
Result of 100 runs
==================
base (...ish_baseline) = 1719637 +/- 3233
test (./stockfish ) = 1734245 +/- 3534
diff = +14608 +/- 4868
speedup = +0.0085
P(speedup > 0) = 1.0000
closes https://github.com/official-stockfish/Stockfish/pull/6366
No functional change
On x86, GCC generates highly suboptimal code for this loop in its old form,
about 2x as many instructions as necessary. This decreases throughput
especially in an SMT setting. Clang does a better job but this change still has
some improvement. Note that the std::ptrdiff_t type is not optional; using an
unsigned type brings back the bad assembly. (Not sure why, but it seems
reliable on all the GCC versions I tested.)
passed STC:
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 44672 W: 11841 L: 11527 D: 21304
Ptnml(0-2): 165, 4625, 12415, 4993, 138
https://tests.stockfishchess.org/tests/view/68d8111efa806e2e8393b10e
closes https://github.com/official-stockfish/Stockfish/pull/6331
No functional change
when we need to update an accumulator by two moves and the second move
captures the piece moved in the first move, we can skip computing the
middle accumulator and cancel a feature add with a feature remove to
save work.
Passed STC
https://tests.stockfishchess.org/tests/view/67f70b1c31d7cf8afdc45f51
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 72800 W: 18878 L: 18529 D: 35393
Ptnml(0-2): 160, 7711, 20374, 7930, 225
closes https://github.com/official-stockfish/Stockfish/pull/5988
No functional change
Since @xu-shawn's refactor the acc pointer has become a bit unnecessary,
we can achieve the same behavior by using the dimension size to get the
right accumulator from the `AccumulatorState`. I think the acc pointer
has become a bit of a burden required to be passed through multiple
different functions together with the necessary templating required.
Passed Non-Regression STC:
https://tests.stockfishchess.org/tests/view/67ed600f31d7cf8afdc45183
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 279744 W: 72037 L: 72082 D: 135625
Ptnml(0-2): 673, 29918, 78767, 29809, 705
closes https://github.com/official-stockfish/Stockfish/pull/5942
No functional change
Helps gcc silence the warning about
```
warning: 'added' may be used uninitialized [-Wmaybe-uninitialized]
const IndexType offsetA0 = TransformedFeatureDimensions * added[0];
```
closes https://github.com/official-stockfish/Stockfish/pull/5950
No functional change
