Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Master #7

Closed
wants to merge 9 commits into from
Closed

Master #7

wants to merge 9 commits into from

Conversation

lantonov
Copy link

No description provided.

@lantonov lantonov closed this Jun 21, 2014
@lantonov lantonov deleted the master branch June 21, 2014 14:34
@LouisZulli LouisZulli mentioned this pull request Sep 5, 2015
Closed
@ajithcj ajithcj mentioned this pull request Sep 20, 2015
Closed
@vondele vondele mentioned this pull request Oct 16, 2015
Closed
@ChameleonRed ChameleonRed mentioned this pull request Apr 3, 2016
Closed
vondele referenced this pull request in vondele/Stockfish Jul 26, 2017
@vondele
Avoid a race between uci thread and search threads.
9844c00 
Fixes a race when ucinewgame is issued 'after' a search, as seen for a thread-sanitized binary issuing :

```
go depth 5
[wait for bestmove ..]
ucinewgame
```
leading to

```
WARNING: ThreadSanitizer: data race (pid=5148)
  Write of size 4 at 0x7fbf5dcf6af8 by main thread:
    #0 __gnu_cxx::__enable_if<!std::__is_scalar<Move>::__value, void>::__type std::__fill_a<Move*, Move>(Move*, Move*, Move const&) <null> (stockfish+0x000000462884)
    #1 void std::fill<Move*, Move>(Move*, Move*, Move const&) /usr/include/c++/5/bits/stl_algobase.h:747 (stockfish+0x0000004618cc)
    #2 StatBoards<16, 64, Move>::fill(Move const&) src/movepick.h:36 (stockfish+0x00000046084f)
    #3 Search::clear() src/search.cpp:194 (stockfish+0x000000451732)
    #4 newgame src/uci.cpp:145 (stockfish+0x0000004738a0)
    #5 UCI::loop(int, char**) src/uci.cpp:200 (stockfish+0x000000473d7d)
    #6 main src/main.cpp:47 (stockfish+0x000000433322)

  Previous write of size 4 at 0x7fbf5dcf6af8 by thread T1:
    #0 update_stats src/search.cpp:1417 (stockfish+0x000000453ecb)
    #1 search<(<unnamed>::NodeType)0u> src/search.cpp:1114 (stockfish+0x00000045c4c6)
    #2 search<(<unnamed>::NodeType)1u> src/search.cpp:997 (stockfish+0x000000457658)
    #3 search<(<unnamed>::NodeType)1u> src/search.cpp:1019 (stockfish+0x000000457a50)
    #4 search<(<unnamed>::NodeType)1u> src/search.cpp:1019 (stockfish+0x000000457a50)
    #5 Thread::search() src/search.cpp:402 (stockfish+0x000000452e28)
    #6 MainThread::search() src/search.cpp:264 (stockfish+0x000000451c32)
    #7 Thread::idle_loop() src/thread.cpp:114 (stockfish+0x000000468c90)

```

The solution adopted is to split the uci commands in those that can be issued during search (quit, stop, ponderhit, isready, uci) and those that should not run during search (ucinewgame, setoption, .. ).  Waiting for search to finish was previously done only for a 'go'.

This fixes the above race.

There are two side effects, one possibly negative, depending on how a gui deals with uci (or a user at the cli).
- crashes are avoided (e.g. resizing hash during search).
- as earlier commands can block on the search to be completed, 'stop' can be non-effective. This behavior is already present on master :

```
go depth 20
go depth 5
stop
```

will wait for depth 20 to be completed before stopping depth 5. This behavior now shows for other commands as well. As such some gui testing of this patch would be useful.

No functional change.
vondele referenced this pull request in vondele/Stockfish Jul 27, 2017
@vondele
avoid a race between uci and search threads (II)
d974307 
limits.ponder was used as a signal between uci and search threads, but is not an atomic variable,
leading to the following race as flagged by a sanitized binary.

Expect input:
```
 spawn  ./stockfish
 send "uci\n"
 expect "uciok"
 send "setoption name Ponder value true\n"
 send "go wtime 4000 btime 4000\n"
 expect "bestmove"
 send "position startpos e2e4 d7d5\n"
 send "go wtime 4000 btime 4000 ponder\n"
 sleep 0.01
 send "ponderhit\n"
 expect "bestmove"
 send "quit\n"
 expect eof
```

Race:
```
WARNING: ThreadSanitizer: data race (pid=7191)
  Read of size 4 at 0x0000005c2260 by thread T1:
    #0 MainThread::check_time() src/search.cpp:1488 (stockfish+0x000000454471)
    #1 search<(<unnamed>::NodeType)0u> src/search.cpp:566 (stockfish+0x0000004594e0)
    #2 search<(<unnamed>::NodeType)0u> src/search.cpp:997 (stockfish+0x00000045bfb6)
    #3 search<(<unnamed>::NodeType)0u> src/search.cpp:1006 (stockfish+0x00000045c1a3)
    #4 search<(<unnamed>::NodeType)1u> src/search.cpp:997 (stockfish+0x000000457658)
    #5 Thread::search() src/search.cpp:402 (stockfish+0x000000452e28)
    #6 MainThread::search() src/search.cpp:264 (stockfish+0x000000451c32)
    #7 Thread::idle_loop() src/thread.cpp:114 (stockfish+0x000000468c90)

  Previous write of size 4 at 0x0000005c2260 by main thread:
    #0 UCI::loop(int, char**) src/uci.cpp:193 (stockfish+0x000000473c9b)
    #1 main src/main.cpp:47 (stockfish+0x000000433322)

  Location is global 'Search::Limits' of size 88 at 0x0000005c2220 (stockfish+0x0000005c2260)
```

The fix is to add an atomic bool to the threads structure to signal the ponder status, letting Search::Limits to reflect just what was passed to 'go'.

No functional change.
vondele referenced this pull request in vondele/Stockfish Aug 2, 2017
@vondele
avoid a race between uci and search threads (II)
c7cd9e6 
limits.ponder was used as a signal between uci and search threads, but is not an atomic variable,
leading to the following race as flagged by a sanitized binary.

Expect input:
```
 spawn  ./stockfish
 send "uci\n"
 expect "uciok"
 send "setoption name Ponder value true\n"
 send "go wtime 4000 btime 4000\n"
 expect "bestmove"
 send "position startpos e2e4 d7d5\n"
 send "go wtime 4000 btime 4000 ponder\n"
 sleep 0.01
 send "ponderhit\n"
 expect "bestmove"
 send "quit\n"
 expect eof
```

Race:
```
WARNING: ThreadSanitizer: data race (pid=7191)
  Read of size 4 at 0x0000005c2260 by thread T1:
    #0 MainThread::check_time() src/search.cpp:1488 (stockfish+0x000000454471)
    #1 search<(<unnamed>::NodeType)0u> src/search.cpp:566 (stockfish+0x0000004594e0)
    #2 search<(<unnamed>::NodeType)0u> src/search.cpp:997 (stockfish+0x00000045bfb6)
    #3 search<(<unnamed>::NodeType)0u> src/search.cpp:1006 (stockfish+0x00000045c1a3)
    #4 search<(<unnamed>::NodeType)1u> src/search.cpp:997 (stockfish+0x000000457658)
    #5 Thread::search() src/search.cpp:402 (stockfish+0x000000452e28)
    #6 MainThread::search() src/search.cpp:264 (stockfish+0x000000451c32)
    #7 Thread::idle_loop() src/thread.cpp:114 (stockfish+0x000000468c90)

  Previous write of size 4 at 0x0000005c2260 by main thread:
    #0 UCI::loop(int, char**) src/uci.cpp:193 (stockfish+0x000000473c9b)
    #1 main src/main.cpp:47 (stockfish+0x000000433322)

  Location is global 'Search::Limits' of size 88 at 0x0000005c2220 (stockfish+0x0000005c2260)
```

The fix is to add an atomic bool to the threads structure to signal the ponder status, letting Search::Limits to reflect just what was passed to 'go'.

No functional change.
@MisesEnForce MisesEnForce mentioned this pull request Aug 11, 2020
Closed
@gvreuls gvreuls mentioned this pull request Jun 3, 2021
Closed
dubslow referenced this pull request in dubslow/Stockfish May 19, 2022
@dubslow
SE scale depth: vaguely-log without the least-depth bounds
2a8ba82 
bench 6530892
Viren6 referenced this pull request in Viren6/Stockfish Feb 7, 2024
@Viren6
dbg results
75032f8 
bench 1024 1 26 default depth nnue

Hit #5: Total 670979 Hits 290935 Hit Rate (%) 43.3598
Hit #6: Total 526362 Hits 200666 Hit Rate (%) 38.1232
Hit #7: Total 409913 Hits 155327 Hit Rate (%) 37.8927
Hit #8: Total 307992 Hits 107444 Hit Rate (%) 34.8853
Hit official-stockfish#9: Total 238620 Hits 82639 Hit Rate (%) 34.6321
Hit official-stockfish#10: Total 159371 Hits 52820 Hit Rate (%) 33.1428
Hit official-stockfish#11: Total 109867 Hits 35774 Hit Rate (%) 32.5612
Hit official-stockfish#12: Total 79281 Hits 24117 Hit Rate (%) 30.4196
Hit official-stockfish#13: Total 60737 Hits 18658 Hit Rate (%) 30.7193
Hit official-stockfish#14: Total 44693 Hits 13464 Hit Rate (%) 30.1255
Hit official-stockfish#15: Total 35225 Hits 11088 Hit Rate (%) 31.4776
Hit official-stockfish#16: Total 50823 Hits 16387 Hit Rate (%) 32.2433
Hit official-stockfish#17: Total 19747 Hits 6494 Hit Rate (%) 32.886
Hit official-stockfish#18: Total 15972 Hits 5266 Hit Rate (%) 32.9702
Hit official-stockfish#19: Total 12658 Hits 4173 Hit Rate (%) 32.9673
Hit official-stockfish#20: Total 8633 Hits 2871 Hit Rate (%) 33.2561
Hit official-stockfish#21: Total 6368 Hits 2236 Hit Rate (%) 35.1131
Hit official-stockfish#22: Total 4569 Hits 1670 Hit Rate (%) 36.5507
Hit official-stockfish#23: Total 3136 Hits 1302 Hit Rate (%) 41.5179
Hit official-stockfish#24: Total 1992 Hits 910 Hit Rate (%) 45.6827
Hit official-stockfish#25: Total 1398 Hits 680 Hit Rate (%) 48.6409
Hit official-stockfish#26: Total 883 Hits 514 Hit Rate (%) 58.2106
Hit official-stockfish#27: Total 569 Hits 348 Hit Rate (%) 61.1599
Hit official-stockfish#28: Total 331 Hits 223 Hit Rate (%) 67.3716
Hit official-stockfish#29: Total 205 Hits 141 Hit Rate (%) 68.7805
Hit official-stockfish#30: Total 103 Hits 71 Hit Rate (%) 68.932
Hit official-stockfish#31: Total 57 Hits 34 Hit Rate (%) 59.6491
Viren6 referenced this pull request in Viren6/Stockfish Feb 7, 2024
@Viren6
dbg results
8808908 
bench 1024 1 26 default depth nnue

Hit #5: Total 673841 Hits 286569 Hit Rate (%) 42.5277
Hit #6: Total 563135 Hits 213101 Hit Rate (%) 37.8419
Hit #7: Total 454918 Hits 175263 Hit Rate (%) 38.5263
Hit #8: Total 349255 Hits 123193 Hit Rate (%) 35.2731
Hit official-stockfish#9: Total 271301 Hits 96581 Hit Rate (%) 35.5992
Hit official-stockfish#10: Total 184972 Hits 64415 Hit Rate (%) 34.8242
Hit official-stockfish#11: Total 127732 Hits 43448 Hit Rate (%) 34.015
Hit official-stockfish#12: Total 93881 Hits 30636 Hit Rate (%) 32.6328
Hit official-stockfish#13: Total 73504 Hits 24436 Hit Rate (%) 33.2444
Hit official-stockfish#14: Total 54409 Hits 17552 Hit Rate (%) 32.2594
Hit official-stockfish#15: Total 43262 Hits 13924 Hit Rate (%) 32.1853
Hit official-stockfish#16: Total 63010 Hits 20206 Hit Rate (%) 32.0679
Hit official-stockfish#17: Total 23513 Hits 7507 Hit Rate (%) 31.927
Hit official-stockfish#18: Total 19250 Hits 6361 Hit Rate (%) 33.0442
Hit official-stockfish#19: Total 15187 Hits 4775 Hit Rate (%) 31.4414
Hit official-stockfish#20: Total 10790 Hits 3435 Hit Rate (%) 31.835
Hit official-stockfish#21: Total 7924 Hits 2486 Hit Rate (%) 31.373
Hit official-stockfish#22: Total 5587 Hits 1941 Hit Rate (%) 34.7414
Hit official-stockfish#23: Total 3822 Hits 1314 Hit Rate (%) 34.3799
Hit official-stockfish#24: Total 2559 Hits 987 Hit Rate (%) 38.5698
Hit official-stockfish#25: Total 1568 Hits 623 Hit Rate (%) 39.7321
Hit official-stockfish#26: Total 1042 Hits 491 Hit Rate (%) 47.1209
Hit official-stockfish#27: Total 651 Hits 370 Hit Rate (%) 56.8356
Hit official-stockfish#28: Total 455 Hits 257 Hit Rate (%) 56.4835
Hit official-stockfish#29: Total 235 Hits 135 Hit Rate (%) 57.4468
Hit official-stockfish#30: Total 131 Hits 86 Hit Rate (%) 65.6489
Hit official-stockfish#31: Total 41 Hits 35 Hit Rate (%) 85.3659
@peregrineshahin peregrineshahin mentioned this pull request Jun 29, 2024
Closed
AYESDIE added a commit to AYESDIE/Stockfish that referenced this pull request Feb 8, 2025
@AYESDIE
official-stockfish#7
21dab02
AYESDIE added a commit to AYESDIE/Stockfish that referenced this pull request Feb 9, 2025
@AYESDIE
official-stockfish#7
1ddabe4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@lantonov