From 94dc83794fb87c325346b1688292fd31511cdb6b Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 24 Jul 2026 15:04:46 +0200 Subject: [PATCH 01/14] Wire dtest calc through CalcAllTablesPBNx as one parallel job. Add unbounded CalcAllTablesx/PBNx that expand all dealxstrain boards into a single parallel_all_boards_n dispatch (heap-backed), and point loop_calc at CalcAllTablesPBNx so 1000-deal batches are no longer 25x40 chunks. Legacy CalcAllTablesN stays capped at MAXNOOFTABLES. Co-authored-by: Cursor --- library/src/api/dll.h | 37 ++++ library/src/calc_tables.cpp | 184 ++++++++++++++++++ library/src/calc_tables.hpp | 3 + library/src/system/parallel_boards.cpp | 7 + library/src/system/parallel_boards.hpp | 4 + library/tests/loop.cpp | 62 +++--- library/tests/loop.hpp | 2 +- library/tests/system/BUILD.bazel | 11 ++ .../tests/system/calc_all_tables_x_test.cpp | 159 +++++++++++++++ 9 files changed, 439 insertions(+), 30 deletions(-) create mode 100644 library/tests/system/calc_all_tables_x_test.cpp diff --git a/library/src/api/dll.h b/library/src/api/dll.h index e1c3aa9c..28bd2aa8 100644 --- a/library/src/api/dll.h +++ b/library/src/api/dll.h @@ -646,6 +646,43 @@ EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesPBNN( struct AllParResults * presp, int maxThreads) -> int; +/** + * @brief Unbounded CalcAllTables: any number of deals, one parallel board job. + * + * Legacy CalcAllTablesN remains capped at MAXNOOFTABLES. This entry point + * expands all deal×strain boards and solves them in a single + * parallel_all_boards_n dispatch (heap-backed), matching the ddss large-batch + * shape while preserving the fixed-size ABI of the legacy structs. + * + * @param numDeals Number of deals (may exceed MAXNOOFTABLES) + * @param deals Flat array of numDeals deals + * @param mode Par mode (-1 = no par); par requires all strains and non-null par + * @param trumpFilter Per-strain filter (0 = include) + * @param results Output array of numDeals tables + * @param par Optional par output (numDeals); required when mode requests par + * @param maxThreads Worker cap; <= 0 means auto + */ +EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesx( + int numDeals, + struct DdTableDeal const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + struct DdTableResults * results, + struct ParResults * par, + int maxThreads) -> int; + +/** + * @brief PBN variant of CalcAllTablesx. + */ +EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesPBNx( + int numDeals, + struct DdTableDealPBN const * deals, + int mode, + int const trumpFilter[DDS_STRAINS], + struct DdTableResults * results, + struct ParResults * par, + int maxThreads) -> int; + /** * @brief Solve multiple bridge deals in PBN format. * diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 72c6cf78..7f32a469 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 @@ -394,6 +395,189 @@ int STDCALL CalcAllTablesPBN( } +auto calc_all_tables_chunk_deals(const int included_strains) -> int +{ + if (included_strains <= 0) + return 0; + return MAXNOOFBOARDS / included_strains; +} + + +namespace +{ + +// Solve one strain board (all four declarers) into scores[DDS_HANDS]. +auto calc_single_deal_scores( + SolverContext& ctx, + Deal deal, + const int target, + const int solutions, + const int mode, + int scores[DDS_HANDS]) -> int +{ + FutureTricks fut{}; + deal.first = 0; + int res = solve_board(ctx, deal, target, solutions, mode, &fut); + if (res != RETURN_NO_FAULT) + return res; + scores[0] = fut.score[0]; + + for (int k = 1; k < DDS_HANDS; k++) + { + const int hint = (k == 2 ? fut.score[0] : 13 - fut.score[0]); + deal.first = k; + res = solve_same_board(ctx, deal, &fut, hint); + if (res != RETURN_NO_FAULT) + return res; + scores[k] = fut.score[0]; + } + return RETURN_NO_FAULT; +} + +} // namespace + + +int STDCALL CalcAllTablesx( + int numDeals, + DdTableDeal const * deals, + int mode, + int const trumpFilter[5], + DdTableResults * results, + ParResults * par, + int maxThreads) +{ + if (numDeals < 0) + return RETURN_TOO_MANY_TABLES; + if (numDeals == 0) + return RETURN_NO_FAULT; + if (deals == nullptr || results == nullptr) + return RETURN_TOO_MANY_TABLES; + + int included = 0; + for (int k = 0; k < DDS_STRAINS; k++) + { + if (!trumpFilter[k]) + included++; + } + if (included == 0) + return RETURN_NO_SUIT; + + const bool want_par = (mode > -1) && (mode < 4) && (included == DDS_STRAINS); + if (want_par && par == nullptr) + return RETURN_TOO_MANY_TABLES; + + // Expand every deal×included-strain into one board list and solve in a + // single parallel_all_boards_n job (heap-backed). This is the ddss-style + // large-batch shape; legacy CalcAllTablesN remains capped at MAXNOOFTABLES. + const int nboards = numDeals * included; + std::vector boards(static_cast(nboards)); + std::vector> scores(static_cast(nboards)); + + int ind = 0; + for (int m = 0; m < numDeals; m++) + { + for (int tr = DDS_STRAINS - 1; tr >= 0; tr--) + { + if (trumpFilter[tr]) + continue; + + Deal& dl = boards[static_cast(ind)]; + for (int h = 0; h < DDS_HANDS; h++) + for (int s = 0; s < DDS_SUITS; s++) + dl.remainCards[h][s] = deals[m].cards[h][s]; + dl.trump = tr; + dl.first = 0; + for (int k = 0; k <= 2; k++) + { + dl.currentTrickRank[k] = 0; + dl.currentTrickSuit[k] = 0; + } + ind++; + } + } + + // Hardest-first dispatch across the full batch (same idea as calc_all_boards_n). + std::vector order(static_cast(nboards)); + std::iota(order.begin(), order.end(), 0); + std::vector fanout(static_cast(nboards)); + for (int i = 0; i < nboards; i++) + fanout[static_cast(i)] = + dds::internal::deal_fanout(boards[static_cast(i)]); + std::stable_sort(order.begin(), order.end(), + [&](const int a, const int b) { + return fanout[static_cast(a)] > fanout[static_cast(b)]; + }); + + const int nthreads = resolve_worker_count(maxThreads, nboards); + 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( + dds::internal::worker_solver_context(), + boards[static_cast(bno)], + -1, 1, 1, + scores[static_cast(bno)].data()); + }, + &order); + if (err != RETURN_NO_FAULT) + return err; + + for (int m = 0; m < numDeals; m++) + { + for (int strainIndex = 0; strainIndex < included; strainIndex++) + { + const int index = m * included + strainIndex; + const int strain = boards[static_cast(index)].trump; + for (int first = 0; first < DDS_HANDS; first++) + { + results[m].res_table[strain][rho[first]] = + 13 - scores[static_cast(index)][static_cast(first)]; + } + } + } + + if (want_par) + { + for (int k = 0; k < numDeals; k++) + { + const int res = Par(&results[k], &par[k], mode); + if (res != RETURN_NO_FAULT) + return res; + } + } + + return RETURN_NO_FAULT; +} + + +int STDCALL CalcAllTablesPBNx( + int numDeals, + DdTableDealPBN const * deals, + int mode, + int const trumpFilter[5], + DdTableResults * results, + ParResults * par, + int maxThreads) +{ + if (numDeals < 0) + return RETURN_TOO_MANY_TABLES; + if (numDeals == 0) + return RETURN_NO_FAULT; + if (deals == nullptr || results == nullptr) + return RETURN_TOO_MANY_TABLES; + + std::vector binary(static_cast(numDeals)); + for (int i = 0; i < numDeals; ++i) + { + if (convert_from_pbn(deals[i].cards, binary[static_cast(i)].cards) != 1) + return RETURN_PBN_FAULT; + } + + return CalcAllTablesx( + numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads); +} + + int STDCALL CalcDDtablePBNN( DdTableDealPBN tableDealPBN, DdTableResults * tablep, diff --git a/library/src/calc_tables.hpp b/library/src/calc_tables.hpp index 70a23592..99b839ea 100644 --- a/library/src/calc_tables.hpp +++ b/library/src/calc_tables.hpp @@ -54,3 +54,6 @@ auto detect_calc_duplicates( const Boards& bds, std::vector& uniques, std::vector& crossrefs) -> void; + +/// Legacy board-budget deal chunk size (MAXNOOFBOARDS / included strains). +auto calc_all_tables_chunk_deals(int included_strains) -> int; diff --git a/library/src/system/parallel_boards.cpp b/library/src/system/parallel_boards.cpp index ce69cb36..df2beac7 100644 --- a/library/src/system/parallel_boards.cpp +++ b/library/src/system/parallel_boards.cpp @@ -25,6 +25,7 @@ namespace { std::atomic g_threads_created{0}; +std::atomic g_last_job_board_count{0}; class BoardWorkerPool @@ -274,6 +275,10 @@ auto parallel_boards_worker_threads_created() -> std::uint64_t return g_threads_created.load(std::memory_order_relaxed); } +auto parallel_boards_last_job_board_count() -> int +{ + return g_last_job_board_count.load(std::memory_order_relaxed); +} void shutdown_parallel_boards_pool() { @@ -302,6 +307,8 @@ auto parallel_all_boards_n( return RETURN_NO_FAULT; } + g_last_job_board_count.store(count, std::memory_order_relaxed); + // Map a dispatch slot to the board number to process. With an order, hand out // boards in that sequence (e.g. hardest first); otherwise in index order. The // order is only honored when it is a valid permutation of [0, count); a diff --git a/library/src/system/parallel_boards.hpp b/library/src/system/parallel_boards.hpp index 551c7af9..a08bdd99 100644 --- a/library/src/system/parallel_boards.hpp +++ b/library/src/system/parallel_boards.hpp @@ -64,6 +64,10 @@ namespace dds::internal // Cumulative number of OS worker threads created by the board pool (test seam). auto parallel_boards_worker_threads_created() -> std::uint64_t; +// Board count of the most recent parallel_all_boards_n call (test seam). +// Used to assert unbounded calc submits one job covering the full batch. +auto parallel_boards_last_job_board_count() -> int; + // Join and destroy the process-local board worker pool. Safe to call with no // pool, and again after a later parallel_all_boards_n recreates it. void shutdown_parallel_boards_pool(); diff --git a/library/tests/loop.cpp b/library/tests/loop.cpp index 7dbeb6a8..b782cc7d 100644 --- a/library/tests/loop.cpp +++ b/library/tests/loop.cpp @@ -113,47 +113,51 @@ bool loop_calc( const int number, const int stepsize) { + (void)dealsp; + (void)resp; + (void)parp; + (void)stepsize; + #ifdef BATCHTIMES - cout << setw(8) << left << "Hand no." << + cout << setw(8) << left << "Hand no." << setw(25) << right << "Time" << "\n"; #endif + // One CalcAllTablesPBNx call for the whole file: expands to number×strains + // boards and solves them in a single parallel job (ddss large-batch shape). int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; const int strain_count = DDS_STRAINS; - for (int i = 0; i < number; i += stepsize) - { - int count = (i + stepsize > number ? number - i : stepsize); - dealsp->no_of_tables = count; - for (int j = 0; j < count; j++) - strcpy(dealsp->deals[j].cards, deal_list[i+j].remainCards); + std::vector deals(static_cast(number)); + std::vector results(static_cast(number)); + for (int i = 0; i < number; i++) + std::strcpy(deals[static_cast(i)].cards, deal_list[i].remainCards); - timer.start(count); - const int workload = count * strain_count; - const int threads = dtest_effective_threads(options.num_threads_, workload); - const int ret = CalcAllTablesPBNN(dealsp, -1, filter, resp, parp, threads); - if (ret != RETURN_NO_FAULT) - { - cout << "loop_calc: i " << i << ", return " << ret << "\n"; - exit(0); - } - timer.end(); + timer.start(number); + const int workload = number * strain_count; + const int threads = dtest_effective_threads(options.num_threads_, workload); + const int ret = CalcAllTablesPBNx( + number, deals.data(), -1, filter, results.data(), nullptr, threads); + timer.end(); + if (ret != RETURN_NO_FAULT) + { + cout << "loop_calc: CalcAllTablesPBNx return " << ret << "\n"; + exit(0); + } #ifdef BATCHTIMES - timer.print_running(i+count, number); + timer.print_running(number, number); #endif - for (int j = 0; j < count; j++) - { - if (compare_TABLE(resp->results[j], table_list[i + j])) - continue; + for (int j = 0; j < number; j++) + { + if (compare_TABLE(results[static_cast(j)], table_list[j])) + continue; - cout << "loop_calc: i " << i << ", j " << j << ": " << - "Difference\n\n"; - print_TABLE(resp->results[j] ); - cout << "\n"; - print_TABLE(table_list[i + j]) ; - cout << "\n"; - } + cout << "loop_calc: j " << j << ": Difference\n\n"; + print_TABLE(results[static_cast(j)]); + cout << "\n"; + print_TABLE(table_list[j]); + cout << "\n"; } #ifdef BATCHTIMES diff --git a/library/tests/loop.hpp b/library/tests/loop.hpp index f65708ad..4702f09b 100644 --- a/library/tests/loop.hpp +++ b/library/tests/loop.hpp @@ -32,7 +32,7 @@ void loop_solve( const int number, const int stepsize); -/// Calculate loop: execute calc_dd_table for multiple deals. +/// Calculate loop: CalcAllTablesPBNx for the full deal list in one parallel job. bool loop_calc( DdTableDealsPBN * dealsp, DdTablesRes * resp, diff --git a/library/tests/system/BUILD.bazel b/library/tests/system/BUILD.bazel index aa1f52a3..09ec0079 100644 --- a/library/tests/system/BUILD.bazel +++ b/library/tests/system/BUILD.bazel @@ -85,6 +85,17 @@ cc_test( ], ) +cc_test( + name = "calc_all_tables_x_test", + size = "medium", + srcs = ["calc_all_tables_x_test.cpp"], + deps = [ + "//library/src:testable_dds", + "//library/src/api:api_definitions", + "@googletest//:gtest_main", + ], +) + cc_test( name = "worker_context_reuse_test", size = "small", diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp new file mode 100644 index 00000000..c83954e4 --- /dev/null +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -0,0 +1,159 @@ +/// @file calc_all_tables_x_test.cpp +/// @brief Tests for unbounded CalcAllTablesx / CalcAllTablesPBNx. +/// +/// The unbounded APIs must accept more than MAXNOOFTABLES deals and run all +/// deal×strain boards as a single parallel job (ddss-style), not as repeated +/// MAXNOOFTABLES-sized chunks. + +#include +#include +#include + +#include +#include +#include +#include + +namespace +{ + +DdTableDeal make_known_deal() +{ + DdTableDeal deal{}; + deal.cards[0][0] = 0x1800 | 0x0040; + deal.cards[0][1] = 0x2000 | 0x0060 | 0x0004; + deal.cards[0][2] = 0x0800 | 0x0100 | 0x0020; + deal.cards[0][3] = 0x0400 | 0x0200 | 0x0100; + deal.cards[1][0] = 0x0100 | 0x0080 | 0x0008; + deal.cards[1][1] = 0x0800 | 0x0200 | 0x0080; + deal.cards[1][2] = 0x4000 | 0x0400 | 0x0080 | 0x0040 | 0x0010; + deal.cards[1][3] = 0x1000 | 0x0010; + deal.cards[2][0] = 0x2000 | 0x0020; + deal.cards[2][1] = 0x0400 | 0x0100 | 0x0008; + deal.cards[2][2] = 0x2000 | 0x1000 | 0x0200; + deal.cards[2][3] = 0x4000 | 0x0080 | 0x0040 | 0x0020 | 0x0004; + deal.cards[3][0] = 0x4000 | 0x0400 | 0x0200 | 0x0010 | 0x0004; + deal.cards[3][1] = 0x4000 | 0x1000 | 0x0010; + deal.cards[3][2] = 0x0008 | 0x0004; + deal.cards[3][3] = 0x2000 | 0x0800 | 0x0008; + return deal; +} + +void expect_tables_equal(const DdTableResults& a, const DdTableResults& b) +{ + for (int strain = 0; strain < DDS_STRAINS; strain++) + for (int hand = 0; hand < DDS_HANDS; hand++) + EXPECT_EQ(a.res_table[strain][hand], b.res_table[strain][hand]) + << "Mismatch at strain=" << strain << " hand=" << hand; +} + +} // namespace + +TEST(CalcAllTablesX, ChunkDealsMatchesBoardBudget) +{ + EXPECT_EQ(calc_all_tables_chunk_deals(DDS_STRAINS), MAXNOOFBOARDS / DDS_STRAINS); + EXPECT_EQ(calc_all_tables_chunk_deals(0), 0); +} + +TEST(CalcAllTablesX, LegacyRejectsMoreThanMaxTables) +{ + InitializeStaticMemory(); + DdTableDeals deals{}; + deals.no_of_tables = MAXNOOFTABLES + 1; + const DdTableDeal known = make_known_deal(); + for (int i = 0; i < deals.no_of_tables; i++) + deals.deals[i] = known; + + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + DdTablesRes results{}; + AllParResults par{}; + EXPECT_EQ( + CalcAllTablesN(&deals, -1, filter, &results, &par, /*maxThreads=*/1), + RETURN_TOO_MANY_TABLES); +} + +TEST(CalcAllTablesX, AcceptsMoreThanMaxTablesAndMatchesLegacy) +{ + InitializeStaticMemory(); + const DdTableDeal known = make_known_deal(); + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + + DdTableDeals legacy{}; + legacy.no_of_tables = 2; + legacy.deals[0] = known; + legacy.deals[1] = known; + DdTablesRes legacy_results{}; + AllParResults par{}; + ASSERT_EQ( + CalcAllTablesN(&legacy, -1, filter, &legacy_results, &par, 1), + RETURN_NO_FAULT); + + constexpr int kNum = MAXNOOFTABLES + 1; + std::vector deals(static_cast(kNum), known); + std::vector results(static_cast(kNum)); + ASSERT_EQ( + CalcAllTablesx( + kNum, deals.data(), -1, filter, results.data(), nullptr, 1), + RETURN_NO_FAULT); + + expect_tables_equal(legacy_results.results[0], results[0]); + expect_tables_equal(legacy_results.results[0], results[static_cast(kNum - 1)]); +} + +// The performance point of PBNx: a batch larger than MAXNOOFTABLES must be +// one parallel_all_boards_n job covering every deal×strain board, not N +// chunked jobs of MAXNOOFBOARDS each. +TEST(CalcAllTablesX, LargeBatchIsSingleParallelJob) +{ + InitializeStaticMemory(); + const DdTableDeal known = make_known_deal(); + constexpr int kNum = MAXNOOFTABLES + 1; + const int expected_boards = kNum * DDS_STRAINS; + + std::vector deals(static_cast(kNum), known); + std::vector results(static_cast(kNum)); + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + + (void)dds::internal::parallel_boards_last_job_board_count(); + ASSERT_EQ( + CalcAllTablesx( + kNum, deals.data(), -1, filter, results.data(), nullptr, + /*maxThreads=*/2), + RETURN_NO_FAULT); + + EXPECT_EQ( + dds::internal::parallel_boards_last_job_board_count(), + expected_boards) + << "unbounded calc must dispatch all boards in one parallel job"; +} + +TEST(CalcAllTablesX, PbnVariantMatchesBinary) +{ + InitializeStaticMemory(); + const DdTableDeal known = make_known_deal(); + constexpr int kNum = 3; + std::vector binary(static_cast(kNum), known); + std::vector binary_results(static_cast(kNum)); + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + ASSERT_EQ( + CalcAllTablesx( + kNum, binary.data(), -1, filter, binary_results.data(), nullptr, 1), + RETURN_NO_FAULT); + + // PBN for the known deal (examples/hands.cpp hand 0). + const char* pbn = + "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3"; + std::vector pbn_deals(static_cast(kNum)); + for (int i = 0; i < kNum; i++) + std::strncpy(pbn_deals[static_cast(i)].cards, pbn, sizeof(pbn_deals[0].cards)); + std::vector pbn_results(static_cast(kNum)); + ASSERT_EQ( + CalcAllTablesPBNx( + kNum, pbn_deals.data(), -1, filter, pbn_results.data(), nullptr, 1), + RETURN_NO_FAULT); + + for (int i = 0; i < kNum; i++) + expect_tables_equal( + binary_results[static_cast(i)], + pbn_results[static_cast(i)]); +} From 6ef4f9b698dbc8914d8d67d266739c4a2246853e Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 26 Jul 2026 22:54:02 +0200 Subject: [PATCH 02/14] Rename CalcAllTablesx/PBNx to CalcAllTablesX/PBNX. Keep the unbounded calc entry points consistent with local API casing and regenerate the JNI export lists so the shared library still exports them. Co-authored-by: Cursor --- jni/exported_symbols.lds | 2 ++ jni/version_script.lds | 2 ++ library/src/api/dll.h | 6 +++--- library/src/calc_tables.cpp | 6 +++--- library/tests/loop.cpp | 6 +++--- library/tests/loop.hpp | 2 +- library/tests/system/calc_all_tables_x_test.cpp | 10 +++++----- 7 files changed, 19 insertions(+), 15 deletions(-) diff --git a/jni/exported_symbols.lds b/jni/exported_symbols.lds index 1e4e0980..66d30184 100644 --- a/jni/exported_symbols.lds +++ b/jni/exported_symbols.lds @@ -6,6 +6,8 @@ _CalcAllTables _CalcAllTablesN _CalcAllTablesPBN _CalcAllTablesPBNN +_CalcAllTablesPBNX +_CalcAllTablesX _CalcDDtable _CalcDDtableN _CalcDDtablePBN diff --git a/jni/version_script.lds b/jni/version_script.lds index d02f1be6..ab0e82b2 100644 --- a/jni/version_script.lds +++ b/jni/version_script.lds @@ -8,6 +8,8 @@ CalcAllTablesN; CalcAllTablesPBN; CalcAllTablesPBNN; + CalcAllTablesPBNX; + CalcAllTablesX; CalcDDtable; CalcDDtableN; CalcDDtablePBN; diff --git a/library/src/api/dll.h b/library/src/api/dll.h index 28bd2aa8..f6073a35 100644 --- a/library/src/api/dll.h +++ b/library/src/api/dll.h @@ -662,7 +662,7 @@ EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesPBNN( * @param par Optional par output (numDeals); required when mode requests par * @param maxThreads Worker cap; <= 0 means auto */ -EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesx( +EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesX( int numDeals, struct DdTableDeal const * deals, int mode, @@ -672,9 +672,9 @@ EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesx( int maxThreads) -> int; /** - * @brief PBN variant of CalcAllTablesx. + * @brief PBN variant of CalcAllTablesX. */ -EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesPBNx( +EXTERN_C DLLEXPORT auto STDCALL CalcAllTablesPBNX( int numDeals, struct DdTableDealPBN const * deals, int mode, diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 7f32a469..dc4f53bc 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -437,7 +437,7 @@ auto calc_single_deal_scores( } // namespace -int STDCALL CalcAllTablesx( +int STDCALL CalcAllTablesX( int numDeals, DdTableDeal const * deals, int mode, @@ -550,7 +550,7 @@ int STDCALL CalcAllTablesx( } -int STDCALL CalcAllTablesPBNx( +int STDCALL CalcAllTablesPBNX( int numDeals, DdTableDealPBN const * deals, int mode, @@ -573,7 +573,7 @@ int STDCALL CalcAllTablesPBNx( return RETURN_PBN_FAULT; } - return CalcAllTablesx( + return CalcAllTablesX( numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads); } diff --git a/library/tests/loop.cpp b/library/tests/loop.cpp index b782cc7d..e39bb8fe 100644 --- a/library/tests/loop.cpp +++ b/library/tests/loop.cpp @@ -123,7 +123,7 @@ bool loop_calc( setw(25) << right << "Time" << "\n"; #endif - // One CalcAllTablesPBNx call for the whole file: expands to number×strains + // One CalcAllTablesPBNX call for the whole file: expands to number×strains // boards and solves them in a single parallel job (ddss large-batch shape). int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; const int strain_count = DDS_STRAINS; @@ -135,12 +135,12 @@ bool loop_calc( timer.start(number); const int workload = number * strain_count; const int threads = dtest_effective_threads(options.num_threads_, workload); - const int ret = CalcAllTablesPBNx( + const int ret = CalcAllTablesPBNX( number, deals.data(), -1, filter, results.data(), nullptr, threads); timer.end(); if (ret != RETURN_NO_FAULT) { - cout << "loop_calc: CalcAllTablesPBNx return " << ret << "\n"; + cout << "loop_calc: CalcAllTablesPBNX return " << ret << "\n"; exit(0); } diff --git a/library/tests/loop.hpp b/library/tests/loop.hpp index 4702f09b..aa01a6b4 100644 --- a/library/tests/loop.hpp +++ b/library/tests/loop.hpp @@ -32,7 +32,7 @@ void loop_solve( const int number, const int stepsize); -/// Calculate loop: CalcAllTablesPBNx for the full deal list in one parallel job. +/// Calculate loop: CalcAllTablesPBNX for the full deal list in one parallel job. bool loop_calc( DdTableDealsPBN * dealsp, DdTablesRes * resp, diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index c83954e4..d48e0624 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -1,5 +1,5 @@ /// @file calc_all_tables_x_test.cpp -/// @brief Tests for unbounded CalcAllTablesx / CalcAllTablesPBNx. +/// @brief Tests for unbounded CalcAllTablesX / CalcAllTablesPBNX. /// /// The unbounded APIs must accept more than MAXNOOFTABLES deals and run all /// deal×strain boards as a single parallel job (ddss-style), not as repeated @@ -92,7 +92,7 @@ TEST(CalcAllTablesX, AcceptsMoreThanMaxTablesAndMatchesLegacy) std::vector deals(static_cast(kNum), known); std::vector results(static_cast(kNum)); ASSERT_EQ( - CalcAllTablesx( + CalcAllTablesX( kNum, deals.data(), -1, filter, results.data(), nullptr, 1), RETURN_NO_FAULT); @@ -116,7 +116,7 @@ TEST(CalcAllTablesX, LargeBatchIsSingleParallelJob) (void)dds::internal::parallel_boards_last_job_board_count(); ASSERT_EQ( - CalcAllTablesx( + CalcAllTablesX( kNum, deals.data(), -1, filter, results.data(), nullptr, /*maxThreads=*/2), RETURN_NO_FAULT); @@ -136,7 +136,7 @@ TEST(CalcAllTablesX, PbnVariantMatchesBinary) std::vector binary_results(static_cast(kNum)); int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; ASSERT_EQ( - CalcAllTablesx( + CalcAllTablesX( kNum, binary.data(), -1, filter, binary_results.data(), nullptr, 1), RETURN_NO_FAULT); @@ -148,7 +148,7 @@ TEST(CalcAllTablesX, PbnVariantMatchesBinary) std::strncpy(pbn_deals[static_cast(i)].cards, pbn, sizeof(pbn_deals[0].cards)); std::vector pbn_results(static_cast(kNum)); ASSERT_EQ( - CalcAllTablesPBNx( + CalcAllTablesPBNX( kNum, pbn_deals.data(), -1, filter, pbn_results.data(), nullptr, 1), RETURN_NO_FAULT); From b4185fbb3b8eefe4aef239166c14432a11ff1fbd Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 26 Jul 2026 23:38:26 +0200 Subject: [PATCH 03/14] Reject null CalcAllTablesX inputs with RETURN_UNKNOWN_FAULT. Null deals, results, or trumpFilter are not size errors; match other C ABI pointer checks and cover both binary and PBN entry points in tests. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 8 ++-- .../tests/system/calc_all_tables_x_test.cpp | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index dc4f53bc..1757cf86 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -450,8 +450,8 @@ int STDCALL CalcAllTablesX( return RETURN_TOO_MANY_TABLES; if (numDeals == 0) return RETURN_NO_FAULT; - if (deals == nullptr || results == nullptr) - return RETURN_TOO_MANY_TABLES; + if (deals == nullptr || results == nullptr || trumpFilter == nullptr) + return RETURN_UNKNOWN_FAULT; int included = 0; for (int k = 0; k < DDS_STRAINS; k++) @@ -563,8 +563,8 @@ int STDCALL CalcAllTablesPBNX( return RETURN_TOO_MANY_TABLES; if (numDeals == 0) return RETURN_NO_FAULT; - if (deals == nullptr || results == nullptr) - return RETURN_TOO_MANY_TABLES; + if (deals == nullptr || results == nullptr || trumpFilter == nullptr) + return RETURN_UNKNOWN_FAULT; std::vector binary(static_cast(numDeals)); for (int i = 0; i < numDeals; ++i) diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index d48e0624..a0b19781 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -55,6 +55,47 @@ TEST(CalcAllTablesX, ChunkDealsMatchesBoardBudget) EXPECT_EQ(calc_all_tables_chunk_deals(0), 0); } +TEST(CalcAllTablesX, NullPointersReturnUnknownFault) +{ + InitializeStaticMemory(); + const DdTableDeal known = make_known_deal(); + DdTableDeal deal = known; + DdTableResults result{}; + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + + EXPECT_EQ( + CalcAllTablesX(1, nullptr, -1, filter, &result, nullptr, 1), + RETURN_UNKNOWN_FAULT); + EXPECT_EQ( + CalcAllTablesX(1, &deal, -1, filter, nullptr, nullptr, 1), + RETURN_UNKNOWN_FAULT); + EXPECT_EQ( + CalcAllTablesX(1, &deal, -1, nullptr, &result, nullptr, 1), + RETURN_UNKNOWN_FAULT); +} + +TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) +{ + InitializeStaticMemory(); + DdTableDealPBN deal{}; + std::strncpy( + deal.cards, + "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3", + sizeof(deal.cards)); + DdTableResults result{}; + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + + EXPECT_EQ( + CalcAllTablesPBNX(1, nullptr, -1, filter, &result, nullptr, 1), + RETURN_UNKNOWN_FAULT); + EXPECT_EQ( + CalcAllTablesPBNX(1, &deal, -1, filter, nullptr, nullptr, 1), + RETURN_UNKNOWN_FAULT); + EXPECT_EQ( + CalcAllTablesPBNX(1, &deal, -1, nullptr, &result, nullptr, 1), + RETURN_UNKNOWN_FAULT); +} + TEST(CalcAllTablesX, LegacyRejectsMoreThanMaxTables) { InitializeStaticMemory(); From 4fe7fbb8d91c87b3cc35b2730ff09d435402e7af Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 26 Jul 2026 23:42:28 +0200 Subject: [PATCH 04/14] Return RETURN_UNKNOWN_FAULT when CalcAllTablesX needs par but gets null. Missing par is an invalid argument, not a table-count error; cover it in the null-pointer tests. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 2 +- library/tests/system/calc_all_tables_x_test.cpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 1757cf86..77d909c1 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -464,7 +464,7 @@ int STDCALL CalcAllTablesX( const bool want_par = (mode > -1) && (mode < 4) && (included == DDS_STRAINS); if (want_par && par == nullptr) - return RETURN_TOO_MANY_TABLES; + return RETURN_UNKNOWN_FAULT; // Expand every deal×included-strain into one board list and solve in a // single parallel_all_boards_n job (heap-backed). This is the ddss-style diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index a0b19781..064b113b 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -72,6 +72,10 @@ TEST(CalcAllTablesX, NullPointersReturnUnknownFault) EXPECT_EQ( CalcAllTablesX(1, &deal, -1, nullptr, &result, nullptr, 1), RETURN_UNKNOWN_FAULT); + // mode in [0, 3] with all strains included requests par output. + EXPECT_EQ( + CalcAllTablesX(1, &deal, /*mode=*/0, filter, &result, nullptr, 1), + RETURN_UNKNOWN_FAULT); } TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) From c2c96665322eabdccf8ed61666421205deeb5b31 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 26 Jul 2026 23:43:25 +0200 Subject: [PATCH 05/14] Avoid OOB writes when testing legacy CalcAllTablesN overflow. Fill only the in-bounds deals slots while setting no_of_tables past MAXNOOFTABLES so the rejection path does not rely on undefined behavior. Co-authored-by: Cursor --- library/tests/system/calc_all_tables_x_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index 064b113b..ada2956c 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -106,7 +106,8 @@ TEST(CalcAllTablesX, LegacyRejectsMoreThanMaxTables) DdTableDeals deals{}; deals.no_of_tables = MAXNOOFTABLES + 1; const DdTableDeal known = make_known_deal(); - for (int i = 0; i < deals.no_of_tables; i++) + // Fill only the in-bounds slots; CalcAllTablesN must reject on count alone. + for (int i = 0; i < MAXNOOFTABLES; i++) deals.deals[i] = known; int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; From 28895aedbfe369ba6ef76719b479b044b0693f96 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 00:07:27 +0200 Subject: [PATCH 06/14] Keep CalcAllTablesX large-batch tests to one strain for TSAN. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Still exercise >MAXNOOFTABLES deals and single-job dispatch, but avoid the 5× board count that timed out macOS ThreadSanitizer CI at 300s. Co-authored-by: Cursor --- library/tests/system/calc_all_tables_x_test.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index ada2956c..f9438bc2 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -122,7 +122,8 @@ TEST(CalcAllTablesX, AcceptsMoreThanMaxTablesAndMatchesLegacy) { InitializeStaticMemory(); const DdTableDeal known = make_known_deal(); - int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + // One strain keeps the >MAXNOOFTABLES path cheap enough for TSAN CI. + int filter[DDS_STRAINS] = {1, 1, 1, 1, 0}; DdTableDeals legacy{}; legacy.no_of_tables = 2; @@ -146,19 +147,21 @@ TEST(CalcAllTablesX, AcceptsMoreThanMaxTablesAndMatchesLegacy) expect_tables_equal(legacy_results.results[0], results[static_cast(kNum - 1)]); } -// The performance point of PBNx: a batch larger than MAXNOOFTABLES must be -// one parallel_all_boards_n job covering every deal×strain board, not N +// The performance point of the X APIs: a batch larger than MAXNOOFTABLES must +// be one parallel_all_boards_n job covering every deal×strain board, not N // chunked jobs of MAXNOOFBOARDS each. TEST(CalcAllTablesX, LargeBatchIsSingleParallelJob) { InitializeStaticMemory(); const DdTableDeal known = make_known_deal(); constexpr int kNum = MAXNOOFTABLES + 1; - const int expected_boards = kNum * DDS_STRAINS; + // One strain: still >MAXNOOFTABLES boards, without a 5× TSAN timeout. + constexpr int kIncludedStrains = 1; + int filter[DDS_STRAINS] = {1, 1, 1, 1, 0}; + const int expected_boards = kNum * kIncludedStrains; std::vector deals(static_cast(kNum), known); std::vector results(static_cast(kNum)); - int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; (void)dds::internal::parallel_boards_last_job_board_count(); ASSERT_EQ( From 84dcb701f1b3df1e4f022751a696f9ac1b198954 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 00:14:41 +0200 Subject: [PATCH 07/14] Harden loop_calc PBN deal copying against empty or oversized input. Bail out when number is non-positive so the unsigned size cast cannot underflow, and copy remainCards with bounded strncpy. Co-authored-by: Cursor --- library/tests/loop.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/tests/loop.cpp b/library/tests/loop.cpp index e39bb8fe..d3ef7a82 100644 --- a/library/tests/loop.cpp +++ b/library/tests/loop.cpp @@ -127,10 +127,18 @@ bool loop_calc( // boards and solves them in a single parallel job (ddss large-batch shape). int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; const int strain_count = DDS_STRAINS; + if (number <= 0) + return true; std::vector deals(static_cast(number)); std::vector results(static_cast(number)); for (int i = 0; i < number; i++) - std::strcpy(deals[static_cast(i)].cards, deal_list[i].remainCards); + { + std::strncpy( + deals[static_cast(i)].cards, + deal_list[i].remainCards, + sizeof(deals[0].cards)); + deals[static_cast(i)].cards[sizeof(deals[0].cards) - 1] = '\0'; + } timer.start(number); const int workload = number * strain_count; From 652bb60b327ed09c7401d9bfcade59ec91963c3a Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 04:32:57 +0200 Subject: [PATCH 08/14] Drop unused legacy batch buffers from loop_calc signature. It now owns flat X-API vectors, so dealsp/resp/parp/stepsize only obscured the real CalcAllTablesPBNX path. Co-authored-by: Cursor --- library/tests/loop.cpp | 11 +---------- library/tests/loop.hpp | 11 ++++++----- library/tests/testcommon.cpp | 6 +----- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/library/tests/loop.cpp b/library/tests/loop.cpp index d3ef7a82..1e6ef1eb 100644 --- a/library/tests/loop.cpp +++ b/library/tests/loop.cpp @@ -105,19 +105,10 @@ void loop_solve( bool loop_calc( - DdTableDealsPBN * dealsp, - DdTablesRes * resp, - AllParResults * parp, DealPBN * deal_list, DdTableResults * table_list, - const int number, - const int stepsize) + const int number) { - (void)dealsp; - (void)resp; - (void)parp; - (void)stepsize; - #ifdef BATCHTIMES cout << setw(8) << left << "Hand no." << setw(25) << right << "Time" << "\n"; diff --git a/library/tests/loop.hpp b/library/tests/loop.hpp index aa01a6b4..a40f3e19 100644 --- a/library/tests/loop.hpp +++ b/library/tests/loop.hpp @@ -33,14 +33,15 @@ void loop_solve( const int stepsize); /// Calculate loop: CalcAllTablesPBNX for the full deal list in one parallel job. +/// Allocates its own flat deal/result buffers (unbounded X API); no legacy +/// DdTableDealsPBN / DdTablesRes batch structs. +/// @param deal_list Input deals in PBN format +/// @param table_list Expected DD table results +/// @param number Number of deals in the test set bool loop_calc( - DdTableDealsPBN * dealsp, - DdTablesRes * resp, - AllParResults * parp, DealPBN * deal_list, DdTableResults * table_list, - const int number, - const int stepsize); + const int number); /// PAR loop: calculate PAR scores for multiple deals. bool loop_par( diff --git a/library/tests/testcommon.cpp b/library/tests/testcommon.cpp index 7d03b242..f28bbb3b 100644 --- a/library/tests/testcommon.cpp +++ b/library/tests/testcommon.cpp @@ -109,9 +109,6 @@ int real_main([[maybe_unused]] int argc, [[maybe_unused]] char * argv[]) BoardsPBN bop; SolvedBoards solvedbdp; - DdTableDealsPBN dealsp; - DdTablesRes resp; - AllParResults parp; PlayTracesPBN playsp; SolvedPlays solvedplp; @@ -121,8 +118,7 @@ int real_main([[maybe_unused]] int argc, [[maybe_unused]] char * argv[]) } else if (options.solver_ == Solver::DTEST_SOLVER_CALC) { - loop_calc(&dealsp, &resp, &parp, deal_list, table_list, - number, stepsize); + loop_calc(deal_list, table_list, number); } else if (options.solver_ == Solver::DTEST_SOLVER_PLAY) { From b6a9c3f8e2912cc63042dfc5e4e0949b6d4f5253 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 04:34:52 +0200 Subject: [PATCH 09/14] Remove unused calc_all_tables_chunk_deals from the library API. It was only referenced by a vestigial test; CalcAllTablesX does not chunk by MAXNOOFBOARDS, so keep that helper out of the production header. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 8 -------- library/src/calc_tables.hpp | 3 --- library/tests/system/calc_all_tables_x_test.cpp | 7 ------- 3 files changed, 18 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 77d909c1..d720cf29 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -395,14 +395,6 @@ int STDCALL CalcAllTablesPBN( } -auto calc_all_tables_chunk_deals(const int included_strains) -> int -{ - if (included_strains <= 0) - return 0; - return MAXNOOFBOARDS / included_strains; -} - - namespace { diff --git a/library/src/calc_tables.hpp b/library/src/calc_tables.hpp index 99b839ea..70a23592 100644 --- a/library/src/calc_tables.hpp +++ b/library/src/calc_tables.hpp @@ -54,6 +54,3 @@ auto detect_calc_duplicates( const Boards& bds, std::vector& uniques, std::vector& crossrefs) -> void; - -/// Legacy board-budget deal chunk size (MAXNOOFBOARDS / included strains). -auto calc_all_tables_chunk_deals(int included_strains) -> int; diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index f9438bc2..8533d854 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -11,7 +11,6 @@ #include #include -#include #include namespace @@ -49,12 +48,6 @@ void expect_tables_equal(const DdTableResults& a, const DdTableResults& b) } // namespace -TEST(CalcAllTablesX, ChunkDealsMatchesBoardBudget) -{ - EXPECT_EQ(calc_all_tables_chunk_deals(DDS_STRAINS), MAXNOOFBOARDS / DDS_STRAINS); - EXPECT_EQ(calc_all_tables_chunk_deals(0), 0); -} - TEST(CalcAllTablesX, NullPointersReturnUnknownFault) { InitializeStaticMemory(); From ccaf3466cdd78008c3c86dc525338c30322d2fcc Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 17:45:59 +0200 Subject: [PATCH 10/14] Catch exceptions at CalcAllTablesX/PBNX C ABI boundaries. Heap allocations can throw; map any throw to RETURN_UNKNOWN_FAULT so exceptions never unwind into foreign C callers. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 216 ++++++++++-------- .../tests/system/calc_all_tables_x_test.cpp | 18 ++ 2 files changed, 135 insertions(+), 99 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index d720cf29..2cb58ee7 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -438,107 +438,117 @@ int STDCALL CalcAllTablesX( ParResults * par, int maxThreads) { - if (numDeals < 0) - return RETURN_TOO_MANY_TABLES; - if (numDeals == 0) - return RETURN_NO_FAULT; - if (deals == nullptr || results == nullptr || trumpFilter == nullptr) - return RETURN_UNKNOWN_FAULT; - - int included = 0; - for (int k = 0; k < DDS_STRAINS; k++) - { - if (!trumpFilter[k]) - included++; - } - if (included == 0) - return RETURN_NO_SUIT; - - const bool want_par = (mode > -1) && (mode < 4) && (included == DDS_STRAINS); - if (want_par && par == nullptr) - return RETURN_UNKNOWN_FAULT; - - // Expand every deal×included-strain into one board list and solve in a - // single parallel_all_boards_n job (heap-backed). This is the ddss-style - // large-batch shape; legacy CalcAllTablesN remains capped at MAXNOOFTABLES. - const int nboards = numDeals * included; - std::vector boards(static_cast(nboards)); - std::vector> scores(static_cast(nboards)); - - int ind = 0; - for (int m = 0; m < numDeals; m++) + // C ABI: exceptions must not unwind into a foreign caller (UB). Heap + // allocations below (and parallel_all_boards_n) may throw; map any throw + // to RETURN_UNKNOWN_FAULT. + try { - for (int tr = DDS_STRAINS - 1; tr >= 0; tr--) + if (numDeals < 0) + return RETURN_TOO_MANY_TABLES; + if (numDeals == 0) + return RETURN_NO_FAULT; + if (deals == nullptr || results == nullptr || trumpFilter == nullptr) + return RETURN_UNKNOWN_FAULT; + + int included = 0; + for (int k = 0; k < DDS_STRAINS; k++) { - if (trumpFilter[tr]) - continue; - - Deal& dl = boards[static_cast(ind)]; - for (int h = 0; h < DDS_HANDS; h++) - for (int s = 0; s < DDS_SUITS; s++) - dl.remainCards[h][s] = deals[m].cards[h][s]; - dl.trump = tr; - dl.first = 0; - for (int k = 0; k <= 2; k++) + if (!trumpFilter[k]) + included++; + } + if (included == 0) + return RETURN_NO_SUIT; + + const bool want_par = (mode > -1) && (mode < 4) && (included == DDS_STRAINS); + if (want_par && par == nullptr) + return RETURN_UNKNOWN_FAULT; + + // Expand every deal×included-strain into one board list and solve in a + // single parallel_all_boards_n job (heap-backed). This is the ddss-style + // large-batch shape; legacy CalcAllTablesN remains capped at MAXNOOFTABLES. + const int nboards = numDeals * included; + std::vector boards(static_cast(nboards)); + std::vector> scores(static_cast(nboards)); + + int ind = 0; + for (int m = 0; m < numDeals; m++) + { + for (int tr = DDS_STRAINS - 1; tr >= 0; tr--) { - dl.currentTrickRank[k] = 0; - dl.currentTrickSuit[k] = 0; + if (trumpFilter[tr]) + continue; + + Deal& dl = boards[static_cast(ind)]; + for (int h = 0; h < DDS_HANDS; h++) + for (int s = 0; s < DDS_SUITS; s++) + dl.remainCards[h][s] = deals[m].cards[h][s]; + dl.trump = tr; + dl.first = 0; + for (int k = 0; k <= 2; k++) + { + dl.currentTrickRank[k] = 0; + dl.currentTrickSuit[k] = 0; + } + ind++; } - ind++; } - } - // Hardest-first dispatch across the full batch (same idea as calc_all_boards_n). - std::vector order(static_cast(nboards)); - std::iota(order.begin(), order.end(), 0); - std::vector fanout(static_cast(nboards)); - for (int i = 0; i < nboards; i++) - fanout[static_cast(i)] = - dds::internal::deal_fanout(boards[static_cast(i)]); - std::stable_sort(order.begin(), order.end(), - [&](const int a, const int b) { - return fanout[static_cast(a)] > fanout[static_cast(b)]; - }); - - const int nthreads = resolve_worker_count(maxThreads, nboards); - 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( - dds::internal::worker_solver_context(), - boards[static_cast(bno)], - -1, 1, 1, - scores[static_cast(bno)].data()); - }, - &order); - if (err != RETURN_NO_FAULT) - return err; + // Hardest-first dispatch across the full batch (same idea as calc_all_boards_n). + std::vector order(static_cast(nboards)); + std::iota(order.begin(), order.end(), 0); + std::vector fanout(static_cast(nboards)); + for (int i = 0; i < nboards; i++) + fanout[static_cast(i)] = + dds::internal::deal_fanout(boards[static_cast(i)]); + std::stable_sort(order.begin(), order.end(), + [&](const int a, const int b) { + return fanout[static_cast(a)] > fanout[static_cast(b)]; + }); + + const int nthreads = resolve_worker_count(maxThreads, nboards); + 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( + dds::internal::worker_solver_context(), + boards[static_cast(bno)], + -1, 1, 1, + scores[static_cast(bno)].data()); + }, + &order); + if (err != RETURN_NO_FAULT) + return err; - for (int m = 0; m < numDeals; m++) - { - for (int strainIndex = 0; strainIndex < included; strainIndex++) + for (int m = 0; m < numDeals; m++) { - const int index = m * included + strainIndex; - const int strain = boards[static_cast(index)].trump; - for (int first = 0; first < DDS_HANDS; first++) + for (int strainIndex = 0; strainIndex < included; strainIndex++) { - results[m].res_table[strain][rho[first]] = - 13 - scores[static_cast(index)][static_cast(first)]; + const int index = m * included + strainIndex; + const int strain = boards[static_cast(index)].trump; + for (int first = 0; first < DDS_HANDS; first++) + { + results[m].res_table[strain][rho[first]] = + 13 - scores[static_cast(index)][static_cast(first)]; + } } } - } - if (want_par) - { - for (int k = 0; k < numDeals; k++) + if (want_par) { - const int res = Par(&results[k], &par[k], mode); - if (res != RETURN_NO_FAULT) - return res; + for (int k = 0; k < numDeals; k++) + { + const int res = Par(&results[k], &par[k], mode); + if (res != RETURN_NO_FAULT) + return res; + } } - } - return RETURN_NO_FAULT; + return RETURN_NO_FAULT; + } + catch (...) + { + return RETURN_UNKNOWN_FAULT; + } } @@ -551,22 +561,30 @@ int STDCALL CalcAllTablesPBNX( ParResults * par, int maxThreads) { - if (numDeals < 0) - return RETURN_TOO_MANY_TABLES; - if (numDeals == 0) - return RETURN_NO_FAULT; - if (deals == nullptr || results == nullptr || trumpFilter == nullptr) - return RETURN_UNKNOWN_FAULT; + // C ABI: same catch-all contract as CalcAllTablesX / dds_c_api.cpp. + try + { + if (numDeals < 0) + return RETURN_TOO_MANY_TABLES; + if (numDeals == 0) + return RETURN_NO_FAULT; + if (deals == nullptr || results == nullptr || trumpFilter == nullptr) + return RETURN_UNKNOWN_FAULT; + + std::vector binary(static_cast(numDeals)); + for (int i = 0; i < numDeals; ++i) + { + if (convert_from_pbn(deals[i].cards, binary[static_cast(i)].cards) != 1) + return RETURN_PBN_FAULT; + } - std::vector binary(static_cast(numDeals)); - for (int i = 0; i < numDeals; ++i) + return CalcAllTablesX( + numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads); + } + catch (...) { - if (convert_from_pbn(deals[i].cards, binary[static_cast(i)].cards) != 1) - return RETURN_PBN_FAULT; + return RETURN_UNKNOWN_FAULT; } - - return CalcAllTablesX( - numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads); } diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index 8533d854..5014c306 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -73,6 +73,9 @@ TEST(CalcAllTablesX, NullPointersReturnUnknownFault) TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) { + // Also documents the C ABI contract: heap allocation failure (or any throw) + // inside CalcAllTablesPBNX must surface as RETURN_UNKNOWN_FAULT, never as an + // exception crossing the C boundary. InitializeStaticMemory(); DdTableDealPBN deal{}; std::strncpy( @@ -93,6 +96,21 @@ TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) RETURN_UNKNOWN_FAULT); } +TEST(CalcAllTablesPBNX, InvalidPbnReturnsPbnFault) +{ + InitializeStaticMemory(); + DdTableDealPBN deal{}; + // Must not start with N/E/S/W (case-insensitive) or convert_from_pbn may + // attempt a partial parse and fail later with a different code. + std::strncpy(deal.cards, "ZZZ:not-a-deal", sizeof(deal.cards)); + DdTableResults result{}; + int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; + + EXPECT_EQ( + CalcAllTablesPBNX(1, &deal, -1, filter, &result, nullptr, 1), + RETURN_PBN_FAULT); +} + TEST(CalcAllTablesX, LegacyRejectsMoreThanMaxTables) { InitializeStaticMemory(); From 11e6fa82ab13d593b537b0f1821b3e7b561f98a5 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 17:49:16 +0200 Subject: [PATCH 11/14] Record last job board count even for empty parallel_all_boards_n calls. Update the test seam before the count <= 0 early return so a prior job cannot leave a stale value. Co-authored-by: Cursor --- library/src/system/parallel_boards.cpp | 6 ++++-- library/src/system/parallel_boards.hpp | 5 +++-- library/tests/system/parallel_boards_test.cpp | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/library/src/system/parallel_boards.cpp b/library/src/system/parallel_boards.cpp index df2beac7..7bb468e8 100644 --- a/library/src/system/parallel_boards.cpp +++ b/library/src/system/parallel_boards.cpp @@ -302,13 +302,15 @@ auto parallel_all_boards_n( const std::function& process_board, const std::vector* order) -> int { + // Always record the requested count, including early-return paths, so the + // test seam cannot report a stale prior job. + g_last_job_board_count.store(count, std::memory_order_relaxed); + if (count <= 0) { return RETURN_NO_FAULT; } - g_last_job_board_count.store(count, std::memory_order_relaxed); - // Map a dispatch slot to the board number to process. With an order, hand out // boards in that sequence (e.g. hardest first); otherwise in index order. The // order is only honored when it is a valid permutation of [0, count); a diff --git a/library/src/system/parallel_boards.hpp b/library/src/system/parallel_boards.hpp index a08bdd99..faa651a7 100644 --- a/library/src/system/parallel_boards.hpp +++ b/library/src/system/parallel_boards.hpp @@ -64,8 +64,9 @@ namespace dds::internal // Cumulative number of OS worker threads created by the board pool (test seam). auto parallel_boards_worker_threads_created() -> std::uint64_t; -// Board count of the most recent parallel_all_boards_n call (test seam). -// Used to assert unbounded calc submits one job covering the full batch. +// Board count of the most recent parallel_all_boards_n call (test seam), +// including calls that return early for count <= 0. Used to assert unbounded +// calc submits one job covering the full batch. auto parallel_boards_last_job_board_count() -> int; // Join and destroy the process-local board worker pool. Safe to call with no diff --git a/library/tests/system/parallel_boards_test.cpp b/library/tests/system/parallel_boards_test.cpp index 8254cd08..6c88a5a8 100644 --- a/library/tests/system/parallel_boards_test.cpp +++ b/library/tests/system/parallel_boards_test.cpp @@ -432,6 +432,21 @@ TEST(ParallelAllBoards, ReusesWorkerThreadsAcrossConsecutiveCalls) EXPECT_EQ(created_after_reuse, created_after_warm); } +TEST(ParallelAllBoards, LastJobBoardCountReflectsZeroBoardCall) +{ + // The test seam must record the most recent call's board count, including + // early-return paths (count <= 0), so a prior job cannot leave a stale value. + const auto noop = [](const int, const int) { return RETURN_NO_FAULT; }; + + ASSERT_EQ(parallel_all_boards_n(4, 1, noop), RETURN_NO_FAULT); + ASSERT_EQ(dds::internal::parallel_boards_last_job_board_count(), 4); + + ASSERT_EQ(parallel_all_boards_n(0, 1, noop), RETURN_NO_FAULT); + EXPECT_EQ(dds::internal::parallel_boards_last_job_board_count(), 0); + + ASSERT_EQ(parallel_all_boards_n(-3, 1, noop), RETURN_NO_FAULT); + EXPECT_EQ(dds::internal::parallel_boards_last_job_board_count(), -3); +} TEST(ParallelAllBoards, ShutdownJoinsPoolAndAllowsRecreation) { From 4388f8c84a3d15f7235477f761784a598e987bea Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 18:50:58 +0200 Subject: [PATCH 12/14] NUL-terminate PBN card buffers after strncpy in calc tests. strncpy can leave fixed-size DdTableDealPBN::cards without a terminator; copy_pbn_cards always writes one. Co-authored-by: Cursor --- .../tests/system/calc_all_tables_x_test.cpp | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index 5014c306..290657fd 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -46,6 +46,13 @@ void expect_tables_equal(const DdTableResults& a, const DdTableResults& b) << "Mismatch at strain=" << strain << " hand=" << hand; } +// strncpy does not guarantee NUL-termination when src length >= dest size. +void copy_pbn_cards(char* dest, const std::size_t dest_size, const char* src) +{ + std::strncpy(dest, src, dest_size - 1); + dest[dest_size - 1] = '\0'; +} + } // namespace TEST(CalcAllTablesX, NullPointersReturnUnknownFault) @@ -78,10 +85,10 @@ TEST(CalcAllTablesPBNX, NullPointersReturnUnknownFault) // exception crossing the C boundary. InitializeStaticMemory(); DdTableDealPBN deal{}; - std::strncpy( + copy_pbn_cards( deal.cards, - "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3", - sizeof(deal.cards)); + sizeof(deal.cards), + "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3"); DdTableResults result{}; int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; @@ -102,7 +109,7 @@ TEST(CalcAllTablesPBNX, InvalidPbnReturnsPbnFault) DdTableDealPBN deal{}; // Must not start with N/E/S/W (case-insensitive) or convert_from_pbn may // attempt a partial parse and fail later with a different code. - std::strncpy(deal.cards, "ZZZ:not-a-deal", sizeof(deal.cards)); + copy_pbn_cards(deal.cards, sizeof(deal.cards), "ZZZ:not-a-deal"); DdTableResults result{}; int filter[DDS_STRAINS] = {0, 0, 0, 0, 0}; @@ -205,7 +212,12 @@ TEST(CalcAllTablesX, PbnVariantMatchesBinary) "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3"; std::vector pbn_deals(static_cast(kNum)); for (int i = 0; i < kNum; i++) - std::strncpy(pbn_deals[static_cast(i)].cards, pbn, sizeof(pbn_deals[0].cards)); + { + copy_pbn_cards( + pbn_deals[static_cast(i)].cards, + sizeof(pbn_deals[0].cards), + pbn); + } std::vector pbn_results(static_cast(kNum)); ASSERT_EQ( CalcAllTablesPBNX( From 706ef9bdfdf9625d6d422493252cd415d128ff42 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 18:52:50 +0200 Subject: [PATCH 13/14] Skip fanout sort in CalcAllTablesX for single-worker runs. Hardest-first dispatch only helps with multiple workers; avoid O(n log n) overhead on large single-thread batches, matching calc_all_boards_n. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 33 ++++++++++------- library/src/system/deal_fanout.cpp | 14 ++++++++ library/src/system/deal_fanout.hpp | 3 ++ .../tests/system/calc_all_tables_x_test.cpp | 36 +++++++++++++++++++ 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 2cb58ee7..3c6a5db8 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -493,19 +493,26 @@ int STDCALL CalcAllTablesX( } } - // Hardest-first dispatch across the full batch (same idea as calc_all_boards_n). - std::vector order(static_cast(nboards)); - std::iota(order.begin(), order.end(), 0); - std::vector fanout(static_cast(nboards)); - for (int i = 0; i < nboards; i++) - fanout[static_cast(i)] = - dds::internal::deal_fanout(boards[static_cast(i)]); - std::stable_sort(order.begin(), order.end(), - [&](const int a, const int b) { - return fanout[static_cast(a)] > fanout[static_cast(b)]; - }); - const int nthreads = resolve_worker_count(maxThreads, nboards); + + // Hardest-first dispatch only helps with multiple workers (same idea as + // calc_all_boards_n). Skip fanout+sort on the single-thread path so large + // single-worker batches avoid O(n log n) overhead. + std::vector order; + if (nthreads > 1) + { + order.resize(static_cast(nboards)); + std::iota(order.begin(), order.end(), 0); + std::vector fanout(static_cast(nboards)); + for (int i = 0; i < nboards; i++) + fanout[static_cast(i)] = + dds::internal::deal_fanout(boards[static_cast(i)]); + std::stable_sort(order.begin(), order.end(), + [&](const int a, const int b) { + return fanout[static_cast(a)] > fanout[static_cast(b)]; + }); + } + const int err = parallel_all_boards_n(nboards, nthreads, [&](const int worker_id, const int bno) -> int { (void)worker_id; @@ -515,7 +522,7 @@ int STDCALL CalcAllTablesX( -1, 1, 1, scores[static_cast(bno)].data()); }, - &order); + order.empty() ? nullptr : &order); if (err != RETURN_NO_FAULT) return err; diff --git a/library/src/system/deal_fanout.cpp b/library/src/system/deal_fanout.cpp index 68b143a1..79ede289 100644 --- a/library/src/system/deal_fanout.cpp +++ b/library/src/system/deal_fanout.cpp @@ -9,6 +9,8 @@ #include "deal_fanout.hpp" +#include + #include namespace dds @@ -16,8 +18,20 @@ namespace dds namespace internal { +namespace +{ +std::atomic g_deal_fanout_call_count{0}; +} // namespace + +auto deal_fanout_call_count() -> int +{ + return g_deal_fanout_call_count.load(std::memory_order_relaxed); +} + auto deal_fanout(const Deal& dl) -> int { + g_deal_fanout_call_count.fetch_add(1, std::memory_order_relaxed); + // The fanout for a given suit and a given player is the number // of bit groups, so KT982 has 3 groups. In a given suit the // maximum number over all four players is 13. diff --git a/library/src/system/deal_fanout.hpp b/library/src/system/deal_fanout.hpp index 1ade73ea..48fc5956 100644 --- a/library/src/system/deal_fanout.hpp +++ b/library/src/system/deal_fanout.hpp @@ -15,5 +15,8 @@ namespace internal */ auto deal_fanout(const Deal& dl) -> int; +/** Test seam: how many times deal_fanout() has been called process-wide. */ +auto deal_fanout_call_count() -> int; + } // namespace internal } // namespace dds diff --git a/library/tests/system/calc_all_tables_x_test.cpp b/library/tests/system/calc_all_tables_x_test.cpp index 290657fd..8c9e4b36 100644 --- a/library/tests/system/calc_all_tables_x_test.cpp +++ b/library/tests/system/calc_all_tables_x_test.cpp @@ -11,6 +11,7 @@ #include #include +#include #include namespace @@ -165,6 +166,41 @@ TEST(CalcAllTablesX, AcceptsMoreThanMaxTablesAndMatchesLegacy) expect_tables_equal(legacy_results.results[0], results[static_cast(kNum - 1)]); } +// Single-worker path must not pay for hardest-first fanout+sort (mirrors +// calc_all_boards_n). Multi-worker still estimates every board once. +TEST(CalcAllTablesX, SingleWorkerSkipsFanoutSort) +{ + InitializeStaticMemory(); + const DdTableDeal known = make_known_deal(); + constexpr int kNum = MAXNOOFTABLES + 1; + constexpr int kIncludedStrains = 1; + int filter[DDS_STRAINS] = {1, 1, 1, 1, 0}; + const int expected_boards = kNum * kIncludedStrains; + + std::vector deals(static_cast(kNum), known); + std::vector results(static_cast(kNum)); + + const int before_single = dds::internal::deal_fanout_call_count(); + ASSERT_EQ( + CalcAllTablesX( + kNum, deals.data(), -1, filter, results.data(), nullptr, + /*maxThreads=*/1), + RETURN_NO_FAULT); + EXPECT_EQ(dds::internal::deal_fanout_call_count(), before_single) + << "single-worker CalcAllTablesX must skip deal_fanout/sort"; + + const int before_multi = dds::internal::deal_fanout_call_count(); + ASSERT_EQ( + CalcAllTablesX( + kNum, deals.data(), -1, filter, results.data(), nullptr, + /*maxThreads=*/2), + RETURN_NO_FAULT); + EXPECT_EQ( + dds::internal::deal_fanout_call_count() - before_multi, + expected_boards) + << "multi-worker CalcAllTablesX must fanout-sort every board once"; +} + // The performance point of the X APIs: a batch larger than MAXNOOFTABLES must // be one parallel_all_boards_n job covering every deal×strain board, not N // chunked jobs of MAXNOOFBOARDS each. From cacbd7a9678629c21d8fed1c50ea9212ab440339 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Mon, 27 Jul 2026 19:01:48 +0200 Subject: [PATCH 14/14] Use DDS_STRAINS for trumpFilter params in calc_tables. Match dll.h declarations and avoid a magic 5 that can drift from the strain count constant. Co-authored-by: Cursor --- library/src/calc_tables.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/src/calc_tables.cpp b/library/src/calc_tables.cpp index 3c6a5db8..90116387 100644 --- a/library/src/calc_tables.cpp +++ b/library/src/calc_tables.cpp @@ -250,7 +250,7 @@ int STDCALL CalcDDtable( int STDCALL CalcAllTablesN( DdTableDeals const * dealsp, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTablesRes * resp, AllParResults * presp, int maxThreads) @@ -356,7 +356,7 @@ int STDCALL CalcAllTablesN( int STDCALL CalcAllTables( DdTableDeals const * dealsp, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTablesRes * resp, AllParResults * presp) { @@ -367,7 +367,7 @@ int STDCALL CalcAllTables( int STDCALL CalcAllTablesPBNN( DdTableDealsPBN const * dealsp, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTablesRes * resp, AllParResults * presp, int maxThreads) @@ -387,7 +387,7 @@ int STDCALL CalcAllTablesPBNN( int STDCALL CalcAllTablesPBN( DdTableDealsPBN const * dealsp, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTablesRes * resp, AllParResults * presp) { @@ -433,7 +433,7 @@ int STDCALL CalcAllTablesX( int numDeals, DdTableDeal const * deals, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTableResults * results, ParResults * par, int maxThreads) @@ -563,7 +563,7 @@ int STDCALL CalcAllTablesPBNX( int numDeals, DdTableDealPBN const * deals, int mode, - int const trumpFilter[5], + int const trumpFilter[DDS_STRAINS], DdTableResults * results, ParResults * par, int maxThreads)