Bigger batches

[tameware]

Export new CalcAllTablesX / CalcAllTablesPBNX APIs that remove the batch size limitation, similar to @BSalita's ddss. Speeds up large Calc batches substantially. Extends the ABI but does not mutate it, so should (still) be a drop-in replacement for 2.9. I've not tested that.

We ought to be able improve performance for Solve batches - that can wait for a future PR.

See these Issues:
#139
#221
#260
#269

Benchmarks versus v2.9, ddss, and the 3.0 develop head, averaged over 10 runs:

Summary (avg user ms)
==============================================================================
solver file             dtest_2.9   dtest_ddss      develop bigger-batch
------ ------------- ------------ ------------ ------------ ------------
solve  list1000.txt          1.43         1.39         1.49         1.50
solve  list100.txt           1.80         1.81         1.91         1.92
solve  list10.txt            4.04         4.25         4.21         4.22
solve  list1.txt            11.58        12.07        11.00        11.00
calc   list1000.txt          9.67         7.86         9.61         7.79
calc   list100.txt           9.45         7.18         9.23         7.13
calc   list10.txt           13.79        13.87        13.28        13.52
calc   list1.txt            37.21        38.36        37.10        37.10
------ ------------- ------------ ------------ ------------ ------------
TOTAL  solve                17.63        17.30        18.37        18.45
TOTAL  calc                109.13        88.84       108.11        87.72

[Copilot]

Pull request overview

This PR introduces new “unbounded” CalcAllTables APIs that accept more than MAXNOOFTABLES deals by expanding deal×strain work onto heap-backed board lists and dispatching the entire batch as a single parallel_all_boards_n job, improving throughput for large batches.

Changes:

• Add new exported C APIs CalcAllTablesX and CalcAllTablesPBNX and wire them into JNI symbol export lists.
• Implement large-batch “single parallel job” execution in CalcAllTablesX, and add a test seam to observe the last dispatched board-count.
• Add new system tests for the unbounded APIs and update the test harness calc loop to use CalcAllTablesPBNX for full-file batches.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
library/tests/system/calc_all_tables_x_test.cpp	New system tests covering unbounded CalcAllTables* behavior and single-job dispatch shape.
library/tests/system/BUILD.bazel	Registers the new system test target.
library/tests/loop.hpp	Updates loop calc comment to match the new unbounded calc path.
library/tests/loop.cpp	Switches calc loop to a single CalcAllTablesPBNX call over the whole deal list.
library/src/system/parallel_boards.hpp	Adds test seam accessor for last job’s board count.
library/src/system/parallel_boards.cpp	Stores last job’s board count for tests to assert single-job dispatch.
library/src/calc_tables.hpp	Declares a helper exposing the legacy chunk-deal sizing.
library/src/calc_tables.cpp	Implements CalcAllTablesX/CalcAllTablesPBNX and the chunk-deals helper.
library/src/api/dll.h	Exposes the new unbounded APIs in the public C header with documentation.
jni/version_script.lds	Exports new symbols for the JNI shared library.
jni/exported_symbols.lds	Exports new symbols for the JNI shared library (underscore-prefixed list).

Comments suppressed due to low confidence (1)

library/src/calc_tables.cpp:567

• CalcAllTablesPBNX should also validate trumpFilter for null and use a non-size error code for null pointer arguments. As written, trumpFilter == nullptr will be dereferenced later in CalcAllTablesX.

  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;

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

library/tests/loop.hpp:43

• The comment says this calculate loop is implemented as a single CalcAllTablesPBNX call, but the function signature still exposes legacy batch structs (DdTableDealsPBN*, DdTablesRes*, AllParResults*) that are no longer populated. This mismatch makes it hard to tell what callers can rely on.

/// Calculate loop: CalcAllTablesPBNX for the full deal list in one parallel job.
bool loop_calc(
    DdTableDealsPBN * dealsp,
    DdTablesRes * resp,
    AllParResults * parp,
    DealPBN * deal_list,
    DdTableResults * table_list,
    const int number,
    const int stepsize);

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

library/src/calc_tables.cpp:466

• CalcAllTablesX is a C ABI entry point but it performs unbounded heap allocations (std::vector/std::stable_sort) that can throw (e.g. std::bad_alloc). Letting exceptions escape an extern "C" frame is undefined behavior; the pure-C shim in library/src/api/dds_c_api.cpp explicitly wraps entry points in catch-all handlers for this reason. Consider adding a catch-all around the body and returning RETURN_UNKNOWN_FAULT on exception.

  // 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<Deal> boards(static_cast<unsigned>(nboards));
  std::vector<std::array<int, DDS_HANDS>> scores(static_cast<unsigned>(nboards));

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

library/tests/system/calc_all_tables_x_test.cpp:107

• std::strncpy() may leave deal.cards unterminated. Add an explicit terminator so the invalid-PBN test can’t accidentally depend on buffer contents past the copied bytes.

  // 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};

library/tests/system/calc_all_tables_x_test.cpp:208

• This for loop uses std::strncpy() into a fixed-size C buffer without guaranteeing NUL-termination. If the PBN string ever grows to 80 bytes, convert_from_pbn() could read past the buffer. Add explicit termination (and braces since there will be multiple statements).

  for (int i = 0; i < kNum; i++)
    std::strncpy(pbn_deals[static_cast<unsigned>(i)].cards, pbn, sizeof(pbn_deals[0].cards));

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

library/src/calc_tables.cpp:567

• CalcAllTablesPBNX definition also hard-codes trumpFilter[5] even though the public header uses DDS_STRAINS. Keeping the definition consistent avoids a stale dimension if DDS_STRAINS ever changes.

int STDCALL CalcAllTablesPBNX(
  int numDeals,
  DdTableDealPBN const * deals,
  int mode,
  int const trumpFilter[5],
  DdTableResults * results,

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.