From fa4f05d3ef2dc393da73b0709e6c2c59a09652c5 Mon Sep 17 00:00:00 2001 From: Timothy Herchen Date: Sun, 9 Nov 2025 03:50:15 -0800 Subject: [PATCH] Don't copy around DirtyThreats 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 --- src/nnue/nnue_accumulator.cpp | 9 ++++++--- src/nnue/nnue_accumulator.h | 14 ++++++++++--- src/position.cpp | 24 +++++++++++------------ src/position.h | 37 ++++++++++++++++++++++------------- src/search.cpp | 8 ++++---- 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/nnue/nnue_accumulator.cpp b/src/nnue/nnue_accumulator.cpp index 7f723c61f..47f09afce 100644 --- a/src/nnue/nnue_accumulator.cpp +++ b/src/nnue/nnue_accumulator.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "../bitboard.h" @@ -122,11 +123,13 @@ void AccumulatorStack::reset() noexcept { size = 1; } -void AccumulatorStack::push(const DirtyBoardData& dirtyBoardData) noexcept { +std::pair AccumulatorStack::push() noexcept { assert(size < MaxSize); - psq_accumulators[size].reset(dirtyBoardData.dp); - threat_accumulators[size].reset(dirtyBoardData.dts); + auto& dp = psq_accumulators[size].reset(); + auto& dts = threat_accumulators[size].reset(); + new (&dts) DirtyThreats; size++; + return {dp, dts}; } void AccumulatorStack::pop() noexcept { diff --git a/src/nnue/nnue_accumulator.h b/src/nnue/nnue_accumulator.h index 4c907c7e3..341960b30 100644 --- a/src/nnue/nnue_accumulator.h +++ b/src/nnue/nnue_accumulator.h @@ -25,6 +25,7 @@ #include #include #include +#include #include "../types.h" #include "nnue_architecture.h" @@ -141,6 +142,12 @@ struct AccumulatorState { accumulatorBig.computed.fill(false); accumulatorSmall.computed.fill(false); } + + typename FeatureSet::DiffType& reset() noexcept { + accumulatorBig.computed.fill(false); + accumulatorSmall.computed.fill(false); + return diff; + } }; class AccumulatorStack { @@ -150,9 +157,10 @@ class AccumulatorStack { template [[nodiscard]] const AccumulatorState& latest() const noexcept; - void reset() noexcept; - void push(const DirtyBoardData& dirtyBoardData) noexcept; - void pop() noexcept; + void reset() noexcept; + void push(const DirtyBoardData& dirtyBoardData) noexcept; + std::pair push() noexcept; + void pop() noexcept; template void evaluate(const Position& pos, diff --git a/src/position.cpp b/src/position.cpp index 58edbcc33..347bbcfc8 100644 --- a/src/position.cpp +++ b/src/position.cpp @@ -201,7 +201,7 @@ Position& Position::set(const string& fenStr, bool isChess960, StateInfo* si) { Square sq = SQ_A8; std::istringstream ss(fenStr); - std::memset(this, 0, sizeof(Position)); + std::memset(reinterpret_cast(this), 0, sizeof(Position)); std::memset(si, 0, sizeof(StateInfo)); st = si; @@ -688,10 +688,12 @@ bool Position::gives_check(Move m) const { // moves should be filtered out before this function is called. // If a pointer to the TT table is passed, the entry for the new position // will be prefetched -DirtyBoardData Position::do_move(Move m, - StateInfo& newSt, - bool givesCheck, - const TranspositionTable* tt = nullptr) { +void Position::do_move(Move m, + StateInfo& newSt, + bool givesCheck, + DirtyPiece& dp, + DirtyThreats& dts, + const TranspositionTable* tt = nullptr) { assert(m.is_ok()); assert(&newSt != st); @@ -720,12 +722,10 @@ DirtyBoardData Position::do_move(Move m, bool checkEP = false; - DirtyPiece dp; - dp.pc = pc; - dp.from = from; - dp.to = to; - dp.add_sq = SQ_NONE; - DirtyThreats dts; + dp.pc = pc; + dp.from = from; + dp.to = to; + dp.add_sq = SQ_NONE; dts.us = us; dts.prevKsq = square(us); dts.threatenedSqs = dts.threateningSqs = 0; @@ -970,8 +970,6 @@ DirtyBoardData Position::do_move(Move m, assert(!(bool(captured) || m.type_of() == CASTLING) ^ (dp.remove_sq != SQ_NONE)); assert(dp.from != SQ_NONE); assert(!(dp.add_sq != SQ_NONE) ^ (m.type_of() == PROMOTION || m.type_of() == CASTLING)); - - return {dp, dts}; } diff --git a/src/position.h b/src/position.h index 9afdb17f9..c95697d19 100644 --- a/src/position.h +++ b/src/position.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "bitboard.h" @@ -133,11 +134,16 @@ class Position { Piece captured_piece() const; // Doing and undoing moves - void do_move(Move m, StateInfo& newSt, const TranspositionTable* tt); - DirtyBoardData do_move(Move m, StateInfo& newSt, bool givesCheck, const TranspositionTable* tt); - void undo_move(Move m); - void do_null_move(StateInfo& newSt, const TranspositionTable& tt); - void undo_null_move(); + void do_move(Move m, StateInfo& newSt, const TranspositionTable* tt); + void do_move(Move m, + StateInfo& newSt, + bool givesCheck, + DirtyPiece& dp, + DirtyThreats& dts, + const TranspositionTable* tt); + void undo_move(Move m); + void do_null_move(StateInfo& newSt, const TranspositionTable& tt); + void undo_null_move(); // Static Exchange Evaluation bool see_ge(Move m, int threshold = 0) const; @@ -196,14 +202,16 @@ class Position { std::array byTypeBB; std::array byColorBB; - int pieceCount[PIECE_NB]; - int castlingRightsMask[SQUARE_NB]; - Square castlingRookSquare[CASTLING_RIGHT_NB]; - Bitboard castlingPath[CASTLING_RIGHT_NB]; - StateInfo* st; - int gamePly; - Color sideToMove; - bool chess960; + int pieceCount[PIECE_NB]; + int castlingRightsMask[SQUARE_NB]; + Square castlingRookSquare[CASTLING_RIGHT_NB]; + Bitboard castlingPath[CASTLING_RIGHT_NB]; + StateInfo* st; + int gamePly; + Color sideToMove; + bool chess960; + DirtyPiece scratch_dp; + DirtyThreats scratch_dts; }; std::ostream& operator<<(std::ostream& os, const Position& pos); @@ -389,7 +397,8 @@ inline void Position::swap_piece(Square s, Piece pc, DirtyThreats* const dts) { } inline void Position::do_move(Move m, StateInfo& newSt, const TranspositionTable* tt = nullptr) { - do_move(m, newSt, gives_check(m), tt); + new (&scratch_dts) DirtyThreats; + do_move(m, newSt, gives_check(m), scratch_dp, scratch_dts, tt); } inline StateInfo* Position::state() const { return st; } diff --git a/src/search.cpp b/src/search.cpp index afd862273..43d96bf24 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -531,16 +531,16 @@ void Search::Worker::do_move( bool capture = pos.capture_stage(move); nodes.fetch_add(1, std::memory_order_relaxed); - DirtyBoardData dirtyBoardData = pos.do_move(move, st, givesCheck, &tt); - accumulatorStack.push(dirtyBoardData); + auto [dirtyPiece, dirtyThreats] = accumulatorStack.push(); + pos.do_move(move, st, givesCheck, dirtyPiece, dirtyThreats, &tt); if (ss != nullptr) { ss->currentMove = move; ss->continuationHistory = - &continuationHistory[ss->inCheck][capture][dirtyBoardData.dp.pc][move.to_sq()]; + &continuationHistory[ss->inCheck][capture][dirtyPiece.pc][move.to_sq()]; ss->continuationCorrectionHistory = - &continuationCorrectionHistory[dirtyBoardData.dp.pc][move.to_sq()]; + &continuationCorrectionHistory[dirtyPiece.pc][move.to_sq()]; } }