From a243ca6602c714098089e231995c77ccdfabbe37 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Tue, 15 Sep 2026 11:06:23 -0400 Subject: [PATCH 1/3] Enable dtest -r per-deal timings for -s calc. Calc expands each deal into strain boards; record those solve times and sum them so the existing report path works for DD-table runs. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 59 +++++++++++++++++++-- library/src/calc_tables.hpp | 30 +++++++++++ library/tests/args.cpp | 6 ++- library/tests/loop.cpp | 22 ++++++-- library/tests/loop.hpp | 6 ++- library/tests/loop_failure_test.cpp | 21 ++++++++ library/tests/report_board_timings.cpp | 32 +++++++++++ library/tests/report_board_timings.hpp | 12 +++++ library/tests/report_board_timings_test.cpp | 51 ++++++++++++++++++ library/tests/testcommon.cpp | 17 +++--- 10 files changed, 237 insertions(+), 19 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 67baaa02f..96ee5a3be 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -9,6 +9,7 @@ #include "calc_tables.hpp" #include +#include #include #include #include @@ -578,6 +579,21 @@ int STDCALL CalcAllTablesX( DdTableResults * results, ParResults * par, int maxThreads) +{ + return calc_all_tables_x( + numDeals, deals, mode, trumpFilter, results, par, maxThreads, nullptr); +} + + +auto calc_all_tables_x( + int numDeals, + DdTableDeal const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + DdTableResults * results, + ParResults * par, + int maxThreads, + std::vector * strain_times_us) -> int { // C ABI: exceptions must not unwind into a foreign caller (UB). Heap // allocations below (and parallel_all_boards_n) may throw; map any throw @@ -616,6 +632,9 @@ int STDCALL CalcAllTablesX( const int nboards = numDeals * included; std::vector boards(static_cast(nboards)); std::vector> scores(static_cast(nboards)); + std::vector local_strain_times; + if (strain_times_us != nullptr) + local_strain_times.assign(static_cast(nboards), 0); int ind = 0; for (int m = 0; m < numDeals; m++) @@ -663,16 +682,34 @@ int STDCALL CalcAllTablesX( const int err = parallel_all_boards_n(nboards, nthreads, [&](const int worker_id, const int bno) -> int { (void)worker_id; - return calc_single_deal_scores( + if (strain_times_us == nullptr) + { + return calc_single_deal_scores( + dds::internal::worker_solver_context(), + boards[static_cast(bno)], + -1, 1, 1, + scores[static_cast(bno)].data()); + } + + const auto t0 = std::chrono::steady_clock::now(); + const int res = calc_single_deal_scores( dds::internal::worker_solver_context(), boards[static_cast(bno)], -1, 1, 1, scores[static_cast(bno)].data()); + const auto dur = std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + local_strain_times[static_cast(bno)] = + saturate_board_time_us(dur); + return res; }, order.empty() ? nullptr : &order); if (err != RETURN_NO_FAULT) return err; + if (strain_times_us != nullptr) + *strain_times_us = std::move(local_strain_times); + for (int m = 0; m < numDeals; m++) { const int tricks = remaining_tricks_from_holdings(deals[m].cards); @@ -717,6 +754,21 @@ int STDCALL CalcAllTablesPBNX( DdTableResults * results, ParResults * par, int maxThreads) +{ + return calc_all_tables_pbn_x( + numDeals, deals, mode, trumpFilter, results, par, maxThreads, nullptr); +} + + +auto calc_all_tables_pbn_x( + int numDeals, + DdTableDealPBN const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + DdTableResults * results, + ParResults * par, + int maxThreads, + std::vector * strain_times_us) -> int { // C ABI: same catch-all contract as CalcAllTablesX / dds_c_api.cpp. try @@ -742,8 +794,9 @@ int STDCALL CalcAllTablesPBNX( return RETURN_PBN_FAULT; } - return CalcAllTablesX( - numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads); + return calc_all_tables_x( + numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads, + strain_times_us); } catch (...) { diff --git a/library/src/calc_tables.hpp b/library/src/calc_tables.hpp index 939791f61..fb632b2b3 100644 --- a/library/src/calc_tables.hpp +++ b/library/src/calc_tables.hpp @@ -62,3 +62,33 @@ auto remaining_tricks_from_holdings( auto declarer_tricks_from_leader_score( int remaining_tricks, int leader_side_score) -> int; + +/** + * @brief Unbounded CalcAllTables with optional per-strain-board timings. + * + * Same behavior as CalcAllTablesX. When @p strain_times_us is non-null, it is + * resized to `numDeals * included_strains` and filled with microseconds spent + * in each strain-board solve (batch-local board index order). + */ +auto calc_all_tables_x( + int numDeals, + DdTableDeal const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + DdTableResults * results, + ParResults * par, + int maxThreads, + std::vector * strain_times_us = nullptr) -> int; + +/** + * @brief PBN variant of calc_all_tables_x. + */ +auto calc_all_tables_pbn_x( + int numDeals, + DdTableDealPBN const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + DdTableResults * results, + ParResults * par, + int maxThreads, + std::vector * strain_times_us = nullptr) -> int; diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 9d0f6399b..99060f79b 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -111,8 +111,10 @@ void usage( " memory via SolverConfig instead of this option.)\n" << "\n" << "-r, --report Print per-deal timings in ms (two decimals) for every\n" << - " hand in the input (solve mode), longest first, plus\n" << - " a min/max/mean/median/stddev summary.\n" << + " hand in the input (solve and calc modes), longest\n" << + " first, plus a min/max/mean/median/stddev summary.\n" << + " For calc, each deal time is the sum of its strain-\n" << + " board solve times.\n" << "\n" << endl; } diff --git a/library/tests/loop.cpp b/library/tests/loop.cpp index 0f2f23bad..049cf7e33 100644 --- a/library/tests/loop.cpp +++ b/library/tests/loop.cpp @@ -23,6 +23,7 @@ #include "cst.hpp" #include "dtest_parallel.hpp" #include "report_board_timings.hpp" +#include #include #include "system/scheduler.hpp" @@ -135,7 +136,8 @@ auto loop_calc( DealPBN * deal_list, DdTableResults * table_list, const int number, - const int stepsize) -> bool + const int stepsize, + std::vector>* board_times) -> bool { // dtest harness progress only: call CalcAllTablesPBNX repeatedly with // `stepsize` deals (typically MAXNOOFBOARDS). Each call still expands to @@ -170,8 +172,16 @@ auto loop_calc( timer.start(count); const int workload = count * strain_count; const int threads = dtest_effective_threads(options.num_threads_, workload); - const int ret = CalcAllTablesPBNX( - count, deals.data(), -1, filter, results.data(), nullptr, threads); + std::vector strain_times; + const int ret = calc_all_tables_pbn_x( + count, + deals.data(), + -1, + filter, + results.data(), + nullptr, + threads, + board_times != nullptr ? &strain_times : nullptr); if (ret != RETURN_NO_FAULT) { timer.end(); @@ -182,6 +192,12 @@ auto loop_calc( } timer.end(); + if (board_times != nullptr) + { + append_calc_batch_deal_times( + *board_times, strain_times, strain_count, i); + } + #ifdef BATCHTIMES timer.print_running(i + count, number); #endif diff --git a/library/tests/loop.hpp b/library/tests/loop.hpp index 127a0005c..241c00309 100644 --- a/library/tests/loop.hpp +++ b/library/tests/loop.hpp @@ -48,12 +48,16 @@ auto loop_solve( /// @param table_list Expected DD table results /// @param number Number of deals in the test set /// @param stepsize Deals per CalcAllTablesPBNX batch (typically `MAXNOOFBOARDS`) +/// @param board_times When non-null, appends per-deal timings for every batch +/// with file-relative deal indices (for `dtest -r`); each deal's time +/// is the sum of its strain-board solve times /// @return false on DDS API fault or first expected-result mismatch auto loop_calc( DealPBN * deal_list, DdTableResults * table_list, const int number, - const int stepsize) -> bool; + const int stepsize, + std::vector>* board_times = nullptr) -> bool; /// PAR loop: calculate PAR scores for multiple deals. /// @return false on DDS API fault or first expected-result mismatch diff --git a/library/tests/loop_failure_test.cpp b/library/tests/loop_failure_test.cpp index e70234480..d7b7f3012 100644 --- a/library/tests/loop_failure_test.cpp +++ b/library/tests/loop_failure_test.cpp @@ -343,6 +343,27 @@ TEST_F(LoopFailureTest, CalcMismatchInLaterBatchUsesAbsoluteIndex) EXPECT_EQ(out.find("loop_calc: j 0:"), std::string::npos); } +TEST_F(LoopFailureTest, CalcReportCollectsPerDealTimesAcrossBatches) +{ + // dtest -r for -s calc must publish one (file_index, time_us) per deal, + // remapping batch-local strain aggregates across stepsize chunks. + auto hands = load_hands( + "loop_calc_report_times.txt", two_deal_body(kDealBody, kDealBody)); + ASSERT_EQ(hands.number, 2); + + std::vector> board_times; + testing::internal::CaptureStdout(); + ASSERT_TRUE( + loop_calc(hands.deal_list, hands.table_list, 2, 1, &board_times)); + (void)testing::internal::GetCapturedStdout(); + + ASSERT_EQ(board_times.size(), 2u); + EXPECT_EQ(board_times[0].first, 0); + EXPECT_EQ(board_times[1].first, 1); + EXPECT_GT(board_times[0].second, 0); + EXPECT_GT(board_times[1].second, 0); +} + TEST_F(LoopFailureTest, PlayStopsOnFirstExpectedMismatch) { auto wrong = std::string(kDealBody); diff --git a/library/tests/report_board_timings.cpp b/library/tests/report_board_timings.cpp index 79e981327..be93b705c 100644 --- a/library/tests/report_board_timings.cpp +++ b/library/tests/report_board_timings.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -156,3 +157,34 @@ void append_batch_board_times( for (const auto& p : batch_times) accumulated.emplace_back(p.first + file_offset, p.second); } + +void append_calc_batch_deal_times( + std::vector>& accumulated, + const std::vector& strain_times_us, + int strains_per_deal, + int file_offset) +{ + if (strains_per_deal <= 0 || strain_times_us.empty()) + return; + + const int deal_count = + static_cast(strain_times_us.size() / static_cast(strains_per_deal)); + accumulated.reserve(accumulated.size() + static_cast(deal_count)); + + constexpr auto kMax = static_cast(std::numeric_limits::max()); + for (int d = 0; d < deal_count; ++d) + { + long long sum_us = 0; + const int base = d * strains_per_deal; + for (int s = 0; s < strains_per_deal; ++s) + { + sum_us += strain_times_us[static_cast(base + s)]; + if (sum_us >= kMax) + { + sum_us = kMax; + break; + } + } + accumulated.emplace_back(d + file_offset, static_cast(sum_us)); + } +} diff --git a/library/tests/report_board_timings.hpp b/library/tests/report_board_timings.hpp index 7ad853905..ac9847dee 100644 --- a/library/tests/report_board_timings.hpp +++ b/library/tests/report_board_timings.hpp @@ -31,3 +31,15 @@ void append_batch_board_times( std::vector>& accumulated, const std::vector>& batch_times, int file_offset); + +/// Append one calc-batch's per-strain timings as per-deal file indices. +/// +/// Calc expands each deal into @p strains_per_deal boards. @p strain_times_us +/// is a flat batch-local vector of those board times (microseconds). Each +/// deal's time is the saturated sum of its strain boards, stored as +/// `(deal_index_in_batch + file_offset, time_us)`. +void append_calc_batch_deal_times( + std::vector>& accumulated, + const std::vector& strain_times_us, + int strains_per_deal, + int file_offset); diff --git a/library/tests/report_board_timings_test.cpp b/library/tests/report_board_timings_test.cpp index 3060031db..fd6bed5b1 100644 --- a/library/tests/report_board_timings_test.cpp +++ b/library/tests/report_board_timings_test.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -187,3 +188,53 @@ TEST(AppendBatchBoardTimes, PreReserveKeepsCapacityAcrossBatches) ASSERT_EQ(accumulated.size(), 4u); EXPECT_EQ(accumulated.capacity(), capacity_before); } + +TEST(AppendCalcBatchDealTimes, SumsStrainTimesIntoFileRelativeDeals) +{ + // Arrange: calc expands each deal to strains_per_deal scheduler boards. + // Batch-local strain times for two deals with 5 strains each, then a second + // batch of one deal remapped by file_offset. + std::vector> accumulated; + const std::vector first_batch_strains = { + 10, 20, 30, 40, 50, // deal 0 -> 150 + 1, 2, 3, 4, 5, // deal 1 -> 15 + }; + const std::vector second_batch_strains = { + 100, 0, 0, 0, 7, // deal 2 -> 107 + }; + + // Act + append_calc_batch_deal_times( + accumulated, first_batch_strains, /*strains_per_deal=*/5, /*file_offset=*/0); + append_calc_batch_deal_times( + accumulated, second_batch_strains, /*strains_per_deal=*/5, /*file_offset=*/2); + + // Assert + ASSERT_EQ(accumulated.size(), 3u); + EXPECT_EQ(accumulated[0], (std::pair{0, 150})); + EXPECT_EQ(accumulated[1], (std::pair{1, 15})); + EXPECT_EQ(accumulated[2], (std::pair{2, 107})); +} + +TEST(AppendCalcBatchDealTimes, EmptyStrainTimesIsNoOp) +{ + std::vector> accumulated = {{9, 1}}; + append_calc_batch_deal_times( + accumulated, {}, /*strains_per_deal=*/5, /*file_offset=*/10); + ASSERT_EQ(accumulated.size(), 1u); + EXPECT_EQ(accumulated[0], (std::pair{9, 1})); +} + +TEST(AppendCalcBatchDealTimes, SaturatesSumThatExceedsIntMax) +{ + std::vector> accumulated; + const int almost_max = std::numeric_limits::max() - 5; + const std::vector strains = {almost_max, 10}; + + append_calc_batch_deal_times( + accumulated, strains, /*strains_per_deal=*/2, /*file_offset=*/3); + + ASSERT_EQ(accumulated.size(), 1u); + EXPECT_EQ(accumulated[0].first, 3); + EXPECT_EQ(accumulated[0].second, std::numeric_limits::max()); +} diff --git a/library/tests/testcommon.cpp b/library/tests/testcommon.cpp index 8c8a09a74..7b855c034 100644 --- a/library/tests/testcommon.cpp +++ b/library/tests/testcommon.cpp @@ -136,7 +136,12 @@ int real_main([[maybe_unused]] int argc, [[maybe_unused]] char * argv[]) } else if (options.solver_ == Solver::DTEST_SOLVER_CALC) { - ok = loop_calc(deal_list, table_list, number, stepsize); + ok = loop_calc( + deal_list, + table_list, + number, + stepsize, + options.report_slow_boards_ ? &board_times : nullptr); } else if (options.solver_ == Solver::DTEST_SOLVER_PLAY) { @@ -165,15 +170,7 @@ int real_main([[maybe_unused]] int argc, [[maybe_unused]] char * argv[]) { if (board_times.empty()) { - if (options.solver_ == Solver::DTEST_SOLVER_CALC) - { - cout << "Per-board timing data not available for calc (use -s solve -r)." - << std::endl; - } - else - { - cout << "Per-board timing data not available." << std::endl; - } + cout << "Per-board timing data not available." << std::endl; } else { From 1529ca6cdb403d5d10ab07247b66f712c45eb2e1 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Tue, 15 Sep 2026 11:25:33 -0400 Subject: [PATCH 2/3] Clear reused strain timing output on zero-deal calc calls. A successful numDeals == 0 path must empty an optional strain_times_us vector so callers cannot observe timings from a previous batch. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 8 ++++ library/src/calc_tables.hpp | 4 +- .../tests/system/calc_all_tables_x_test.cpp | 41 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 96ee5a3be..5b394b418 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -603,7 +603,11 @@ auto calc_all_tables_x( if (numDeals < 0) return RETURN_TOO_MANY_TABLES; if (numDeals == 0) + { + if (strain_times_us != nullptr) + strain_times_us->clear(); return RETURN_NO_FAULT; + } if (deals == nullptr || results == nullptr || trumpFilter == nullptr) return RETURN_UNKNOWN_FAULT; @@ -776,7 +780,11 @@ auto calc_all_tables_pbn_x( if (numDeals < 0) return RETURN_TOO_MANY_TABLES; if (numDeals == 0) + { + if (strain_times_us != nullptr) + strain_times_us->clear(); return RETURN_NO_FAULT; + } if (deals == nullptr || results == nullptr || trumpFilter == nullptr) return RETURN_UNKNOWN_FAULT; diff --git a/library/src/calc_tables.hpp b/library/src/calc_tables.hpp index fb632b2b3..1df00e640 100644 --- a/library/src/calc_tables.hpp +++ b/library/src/calc_tables.hpp @@ -68,7 +68,9 @@ auto declarer_tricks_from_leader_score( * * Same behavior as CalcAllTablesX. When @p strain_times_us is non-null, it is * resized to `numDeals * included_strains` and filled with microseconds spent - * in each strain-board solve (batch-local board index order). + * in each strain-board solve (batch-local board index order). A successful + * `numDeals == 0` call clears @p strain_times_us so reused vectors cannot keep + * stale timings. */ auto calc_all_tables_x( int numDeals, diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index 8c9e4b36f..4e7766ce4 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -104,6 +105,46 @@ TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) RETURN_UNKNOWN_FAULT); } +TEST(CalcAllTablesX, ZeroDealsClearsReusedStrainTimes) +{ + InitializeStaticMemory(); + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + std::vector strain_times = {11, 22, 33}; + + ASSERT_EQ( + calc_all_tables_x( + /*numDeals=*/0, + /*deals=*/nullptr, + -1, + filter, + /*results=*/nullptr, + nullptr, + 1, + &strain_times), + RETURN_NO_FAULT); + EXPECT_TRUE(strain_times.empty()); +} + +TEST(CalcAllTablesPBNX, ZeroDealsClearsReusedStrainTimes) +{ + InitializeStaticMemory(); + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + std::vector strain_times = {7, 8}; + + ASSERT_EQ( + calc_all_tables_pbn_x( + /*numDeals=*/0, + /*deals=*/nullptr, + -1, + filter, + /*results=*/nullptr, + nullptr, + 1, + &strain_times), + RETURN_NO_FAULT); + EXPECT_TRUE(strain_times.empty()); +} + TEST(CalcAllTablesPBNX, InvalidPbnReturnsPbnFault) { InitializeStaticMemory(); From f97cb5b16e96c89545b78aa84d7957c22520b540 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Tue, 15 Sep 2026 13:23:42 -0400 Subject: [PATCH 3/3] Include directly where calc_tables uses std::move. Avoid relying on transitive standard-library headers for the optional strain-timing path. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 5b394b418..479cc43b3 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -9,10 +9,11 @@ #include "calc_tables.hpp" #include +#include #include #include -#include #include +#include #include #include