Rename SetMaxThreads to InitialiseStaticMemory; add max_threads override by zzcgumn · Pull Request #204 · dds-bridge/dds

zzcgumn · 2026-06-21T10:40:42Z

Summary

Rename SetMaxThreads to InitialiseStaticMemory: the function initialises
static library state (TT memory pools, scheduler/thread-manager, lookup
tables) and no longer controls thread count, since internal batch threading
was removed. SetMaxThreads is kept as a thin deprecated alias so the
exported C symbol, the .NET P/Invoke, and the Python binding keep working.
Collapse the duplicated max(1, min(hardware_concurrency, n)) worker-count
formula into one resolve_worker_count(max_threads, count) helper, and plumb
an optional max_threads (0 = auto) through solve_all_boards_n and the
legacy parallel calc_all_boards_n.
Add public *N C variants — SolveAllBoardsN, SolveAllBoardsBinN,
CalcDDtableN, CalcDDtablePBNN, CalcAllTablesN, CalcAllTablesPBNN —
that accept maxThreads. The existing functions keep their signatures/ABI
and delegate with 0, so current auto-sizing behaviour is unchanged.
Add tests: resolve_worker_count edge cases, and equivalence checks proving
an explicit maxThreads cap produces results identical to the auto path.

Notes

The C++ calc_dd_table overloads use the sequential context-aware
calc_all_boards_n, where a thread cap would be a no-op, so they were left
unchanged. Only the legacy parallel path (used by the C API) carries the
parameter.
No behaviour change for any existing caller: defaults and delegation preserve
the current hardware_concurrency auto-sizing.

Test plan

bazel build //library/... //examples/...
bazel test //library/tests/... — 31 tests pass (29 existing + 2 new)
CalcDDtableN(..., maxThreads=1) matches CalcDDtable(...)
SolveAllBoardsBinN(..., maxThreads=1) matches SolveAllBoardsBin(...)
InitialiseStaticMemory() and the deprecated SetMaxThreads alias both
leave the library usable for a subsequent solve

🤖 Generated with Claude Code

Closes #168
Closes #188

Copilot

Pull request overview

This PR clarifies DDS initialization by renaming the legacy SetMaxThreads concept to InitialiseStaticMemory, and introduces *N batch C APIs that allow callers to optionally cap worker-thread usage while preserving existing auto-sizing behavior.

Changes:

Add InitialiseStaticMemory() and make SetMaxThreads() a deprecated alias that only performs static initialization.
Introduce resolve_worker_count(max_threads, count) and plumb max_threads through the legacy parallel batch paths (SolveAllBoards*, Calc*Tables*) via new *N C entry points.
Add system tests covering worker-count resolution and equivalence between auto vs explicit maxThreads caps.

Reviewed changes

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

Show a summary per file

File	Description
python/src/bindings.cpp	Updates Python docstring to reflect `SetMaxThreads` deprecation/aliasing.
library/tests/system/worker_count_test.cpp	Adds unit tests for `resolve_worker_count` behavior.
library/tests/system/max_threads_equivalence_test.cpp	Adds equivalence tests for `*N` APIs and init/alias behavior.
library/tests/system/context_tt_facade_test.cpp	Switches tests from `SetMaxThreads` to `InitialiseStaticMemory`.
library/tests/system/context_equivalence_test.cpp	Switches tests from `SetMaxThreads` to `InitialiseStaticMemory`.
library/tests/system/BUILD.bazel	Registers the two new system tests.
library/src/system/parallel_boards.hpp	Declares `resolve_worker_count`.
library/src/system/parallel_boards.cpp	Implements `resolve_worker_count` and uses it in `parallel_all_boards_n`.
library/src/solve_board.cpp	Adds `SolveAllBoardsN`/`SolveAllBoardsBinN` and threads `maxThreads` to batch solve path.
library/src/init.cpp	Adds `InitialiseStaticMemory`; makes `SetMaxThreads` a deprecated alias.
library/src/dds.cpp	Updates library-load init hooks to call `InitialiseStaticMemory`.
library/src/calc_tables.cpp	Threads `max_threads` through legacy parallel batch calc and adds `Calc*N` entry points.
library/src/api/dll.h	Exposes `InitialiseStaticMemory` and new `*N` C API declarations with docs.
examples/migration_example.cpp	Updates example to call `InitialiseStaticMemory` instead of `SetMaxThreads`.
examples/dd_table_for_deal.cpp	Updates example initialization call to `InitialiseStaticMemory`.
dotnet/DDS_Core/Native/DdsNative.cs	Clarifies in comments that `SetMaxThreads` is deprecated/aliased.
dotnet/DDS_Core/DDS.cs	Clarifies in comments that the .NET wrapper calls the deprecated alias symbol.

Collapse the duplicated max(1, min(hardware_concurrency, count)) worker-count formula into a single resolve_worker_count helper in parallel_boards, and rewire parallel_all_boards_n to use it. Pure refactor, no behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The function initialises static library state (TT memory pools, scheduler / thread-manager, lookup tables) and no longer controls thread count. Add InitialiseStaticMemory() as the public name and keep SetMaxThreads as a thin deprecated alias for ABI/back-compat. Repoint internal callers (dds.cpp, examples); update .NET/Python binding comments only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add a trailing max_threads (default 0 = auto) to solve_all_boards_n and the legacy parallel calc_all_boards_n, forwarding it to parallel_all_boards_n and using resolve_worker_count to size the SolverContext pool. Default preserves the current hardware_concurrency auto-sizing, so no behaviour change. Deviation from task: the C++ calc_dd_table overloads use the sequential context-aware calc_all_boards_n, so threading max_threads there would be a no-op; left unchanged. The C API CalcDDtable/CalcAllTables use the legacy parallel overload, which now carries the parameter for Task 04. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add SolveAllBoardsN, SolveAllBoardsBinN, CalcDDtableN, CalcDDtablePBNN, CalcAllTablesN and CalcAllTablesPBNN, which forward an explicit maxThreads (<= 0 = auto) down to the internal batch solvers. The existing functions keep their signatures and now delegate to the *N variants with 0, preserving ABI and current auto-sizing behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add worker_count_test (resolve_worker_count edge cases) and max_threads_equivalence_test (CalcDDtableN / SolveAllBoardsBinN with an explicit cap match the auto path; InitialiseStaticMemory and the deprecated SetMaxThreads alias both leave the library usable). Migrate existing SetMaxThreads(1) call sites to InitialiseStaticMemory(), keeping one deliberate alias usage in the new test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

tameware · 2026-06-21T14:48:37Z

Shall I wait to request another Copilot review until the build is fixed?

zzcgumn · 2026-06-21T17:41:28Z

Ready for another round of Copilot.

Copilot

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…eads.

Copilot

Pull request overview

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

…lise static memory.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

…string.

Copilot

Pull request overview

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

tameware

I am not qualified to review the code for readability and functionality - I'm happy to delegate that to Copilot.

I did pull the branch and run it through my benchmarks. I see that dtest's -n flag - number of threads - still works, so the replacement threading limits are being respected.

Copilot is complaining about something that a previous iteration marked as resolved. I suggest either making Copilot happy or adding a comment noting why it's OK.

The spelling of Initialise is inconsistent with the internal variable spelling that uses the American Initialize. I suggest renaming to InitializeStaticMemory everywhere unless there's a reason not to. That's what most developers will expect.

Copilot

Pull request overview

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

tameware · 2026-06-25T14:47:18Z

LGTM

zzcgumn · 2026-06-25T15:23:30Z

@tameware, would you mind approving the pull request as well?

tameware · 2026-06-25T16:16:21Z

I would if I knew how! I see no "Approve" button.

zzcgumn · 2026-06-25T16:45:33Z

Do you see a submit review button here https://github.com/dds-bridge/dds/pull/204/changes?

Otherwise I need to look into permissions.

zzcgumn requested a review from tameware June 21, 2026 10:40

zzcgumn self-assigned this Jun 21, 2026

zzcgumn mentioned this pull request Jun 21, 2026

Fix DDS_SCHEDULER build consistency and dtest -r reporting. #201

Merged

tameware requested a review from Copilot June 21, 2026 13:54

Copilot started reviewing on behalf of tameware June 21, 2026 13:54 View session

Copilot AI reviewed Jun 21, 2026

View reviewed changes

Comment thread library/src/system/parallel_boards.cpp

Comment thread library/src/system/parallel_boards.hpp

zzcgumn and others added 5 commits June 21, 2026 15:00

zzcgumn force-pushed the fix/pass_instead_of_set_max_thread branch from ed4e1a9 to f36dc17 Compare June 21, 2026 14:02

zzcgumn added 2 commits June 21, 2026 18:16

fix: align argument names and default values.

3acd2e9

fix: updates documenting comment.

31c4cd4

tameware requested a review from Copilot June 21, 2026 21:32

Copilot started reviewing on behalf of tameware June 21, 2026 21:32 View session

Copilot AI reviewed Jun 21, 2026

View reviewed changes

Comment thread library/src/solve_board.cpp Outdated

Comment thread library/src/api/dll.h Outdated

Comment thread python/src/bindings.cpp Outdated

Comment thread dotnet/DDS_Core/DDS.cs Outdated

zzcgumn and others added 2 commits June 21, 2026 22:59

Apply suggestions from code review

441feca

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

exports initialise static memory to python and deprecates set max thr…

d5ef976

…eads.

tameware requested a review from Copilot June 22, 2026 12:56

Copilot started reviewing on behalf of tameware June 22, 2026 12:56 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread library/src/calc_tables.cpp

Comment thread library/src/solve_board.cpp

Comment thread python/src/bindings.cpp

Comment thread python/src/bindings.cpp

zzcgumn added 2 commits June 22, 2026 20:38

fix: removes code duplication and uses nthreads consistently.

9c831a8

fix: releases the global interpreter lock before doing work in initia…

25477c5

…lise static memory.

tameware requested a review from Copilot June 23, 2026 03:24

Copilot started reviewing on behalf of tameware June 23, 2026 03:24 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread python/src/bindings.cpp Outdated

fix: deletes the input guard on the ignored user_threads argument.

582b127

fix: removes test that is now incorrect.

600515d

tameware requested a review from Copilot June 24, 2026 00:01

Copilot started reviewing on behalf of tameware June 24, 2026 00:01 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread python/src/bindings.cpp

Comment thread python/tests/test_analyse.py

fix: remove mention of the guard on user_threads from the python doc …

26e2912

…string.

tameware requested a review from Copilot June 24, 2026 20:11

Copilot started reviewing on behalf of tameware June 24, 2026 20:12 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread python/src/bindings.cpp Outdated

tameware reviewed Jun 24, 2026

View reviewed changes

Comment thread examples/dd_table_for_deal.cpp Outdated

zzcgumn added 2 commits June 25, 2026 13:13

fix: removes incorrect mention of value error.

5397d23

fix: renames InitialiseStaticMemory to use the American misspelling.

a0fed20

tameware requested a review from Copilot June 25, 2026 14:12

Copilot started reviewing on behalf of tameware June 25, 2026 14:13 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

tameware approved these changes Jun 25, 2026

View reviewed changes

zzcgumn merged commit 9c78c46 into develop Jun 25, 2026
5 checks passed

zzcgumn deleted the fix/pass_instead_of_set_max_thread branch June 25, 2026 19:25

Uh oh!

Conversation

zzcgumn commented Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

tameware commented Jun 21, 2026

Uh oh!

zzcgumn commented Jun 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

tameware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tameware commented Jun 25, 2026

Uh oh!

zzcgumn commented Jun 25, 2026

Uh oh!

tameware commented Jun 25, 2026

Uh oh!

zzcgumn commented Jun 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zzcgumn commented Jun 21, 2026 •

edited

Loading