Fixes so that DDS_AB_STATS will work when it's set. by tameware · Pull Request #195 · dds-bridge/dds

tameware · 2026-06-17T13:45:26Z

I've tested this locally with bazel build //library/tests:dtest --cxxopt=-DDDS_AB_STATS. A dtest run then produces ABstats*.txt files, as it ought to. The build fails without this PR.

We should either merge this PR or else remove the DDS_AB_STATS code entirely.

Another possible enhancement would be to mirror the DDS_SCHEDULER pattern, so that we can use --define=ab_stats=true instead of --cxxopt.

Including debug.h from thread_data.hpp exposed a global named counter that conflicts with pybind11 on MSVC when warnings are treated as errors. Co-authored-by: Cursor <cursoragent@cursor.com>

zzcgumn · 2026-06-17T16:07:03Z

I think we should prefer --define=ab_stats=true over --cxxopt=-DDDS_AB_STATS. Explicit is better than implicit and the --define pattern is immediately obvious but the --cxxopt= forces me parse through the right hand side to understand what is happening.

zzcgumn · 2026-06-17T16:08:24Z

I will give this a more careful review once I have a bit more time. We are out of Copilot cycles for the month of June.

tameware · 2026-06-18T02:20:44Z

There's no hurry!

In any case, I suggest the --define=ab_stats refactor can go in a separate PR. It's orthogonal to this change.

Copilot

Pull request overview

This PR fixes build/runtime behavior when compiling DDS with DDS_AB_STATS enabled by making the AB-stats code linkable via Bazel targets and by reworking how debug/stat output files are named/opened/closed via SolverContext/ThreadData.

Changes:

Adds a dedicated ab_stats Bazel target and wires it into dds/system builds.
Introduces a File helper for lazy debug/stat file creation and integrates it into ThreadData.
Moves debug file lifecycle to SolverContext (open/init on bind, close on destruction) and removes unused debug counter declarations.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
library/src/utility/debug.h	Removes unused performance counter declarations.
library/src/system/thread_data.hpp	Includes debug/file headers and conditionally enables debug file support.
library/src/system/file.hpp	Adds `dds::File` interface for lazy debug/stat logging output streams.
library/src/system/file.cpp	Implements `File` open/close/reset behavior.
library/src/system/BUILD.bazel	Adds `ab_stats` dependency to system libraries.
library/src/solver_context/solver_context.hpp	Refactors constructor to bind thread data via helper.
library/src/solver_context/solver_context.cpp	Implements debug-file suffixing and ties debug file lifecycle to `SolverContext`.
library/src/init.cpp	Removes legacy debug-file close loop (now handled by `SolverContext`).
library/src/BUILD.bazel	Adds `ab_stats` library target and updates source glob exclusions/deps.

Only owning SolverContext instances should open or close per-thread debug files; helper contexts that wrap existing ThreadData must not reassign names mid-solve or close streams still in use by the primary context. Co-authored-by: Cursor <cursoragent@cursor.com>

Track whether debug files were opened on ThreadData and defer close until ~SolverContext() sees use_count() == 1, so shared/helper contexts do not close streams while another context is still writing. Co-authored-by: Cursor <cursoragent@cursor.com>

Reset() previously cleared the filename and file_open_ flag without closing an already-open ofstream, which could leak handles and leave stale streams. Co-authored-by: Cursor <cursoragent@cursor.com>

SetName() previously updated fname_ while leaving an open ofstream on the old path, so later writes could go to the wrong file. Co-authored-by: Cursor <cursoragent@cursor.com>

GetStream() previously set file_open_ even when open() failed, leaving the object stuck with a closed but supposedly-open stream; skip open on empty names and rely on is_open() for lazy open and close. Co-authored-by: Cursor <cursoragent@cursor.com>

Per-context debug files are closed in ~SolverContext(); the empty CloseDebugFiles() stub and library unload calls are no longer needed. Co-authored-by: Cursor <cursoragent@cursor.com>

Drop the global using alias from file.hpp so File stays in namespace dds and callers reference it explicitly. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

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

Document that SolverContext passes "<serial>.txt" rather than a thread-id suffix, so debug output filenames are easier to trace. Co-authored-by: Cursor <cursoragent@cursor.com>

SearchContext kept a second shared_ptr, so use_count() was never 1 during destruction and close_debug_files() never ran; release that reference first. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

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

Add direct headers for std::min and std::snprintf instead of relying on transitive includes from other headers. Co-authored-by: Cursor <cursoragent@cursor.com>

Document that wrapper contexts skip init but may close debug files when they are the last shared_ptr holder of the ThreadData. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

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

        "//library/src/trans_table",
        "//library/src/moves:moves",
+        "//library/src:ab_stats",
        "//library/src/system",
        "//library/src/solver_context:solver_context",


Exclude ab_stats.cpp from testable_dds_sources and link //library/src:ab_stats explicitly, matching :dds and preventing duplicate ABstats symbols. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware and others added 2 commits June 17, 2026 09:08

Fixes so that DDS_AB_STATS will work when it's set.

fcf913c

Remove unused global counter from debug.h to fix Windows builds.

49bc926

Including debug.h from thread_data.hpp exposed a global named counter that conflicts with pybind11 on MSVC when warnings are treated as errors. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware marked this pull request as ready for review June 17, 2026 14:10

zzcgumn reviewed Jun 18, 2026

View reviewed changes

Comment thread library/src/system/file.hpp Outdated

zzcgumn reviewed Jun 18, 2026

View reviewed changes

Comment thread library/src/init.cpp Outdated

tameware requested a review from Copilot June 19, 2026 02:36

Copilot started reviewing on behalf of tameware June 19, 2026 02:37 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread library/src/solver_context/solver_context.cpp

Comment thread library/src/solver_context/solver_context.cpp

Comment thread library/src/system/file.cpp Outdated

Comment thread library/src/system/file.cpp Outdated

Comment thread library/src/system/file.cpp Outdated

tameware and others added 7 commits June 18, 2026 23:09

Close open streams in File::Reset() before clearing state.

5619656

Reset() previously cleared the filename and file_open_ flag without closing an already-open ofstream, which could leak handles and leave stale streams. Co-authored-by: Cursor <cursoragent@cursor.com>

Close open streams in File::SetName() before renaming.

ac15d62

SetName() previously updated fname_ while leaving an open ofstream on the old path, so later writes could go to the wrong file. Co-authored-by: Cursor <cursoragent@cursor.com>

Remove no-op CloseDebugFiles() global teardown hook.

ce6f04f

Per-context debug files are closed in ~SolverContext(); the empty CloseDebugFiles() stub and library unload calls are no longer needed. Co-authored-by: Cursor <cursoragent@cursor.com>

Qualify dds::File and remove header using declaration.

02ccc23

Drop the global using alias from file.hpp so File stays in namespace dds and callers reference it explicitly. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware requested a review from Copilot June 19, 2026 04:10

Copilot started reviewing on behalf of tameware June 19, 2026 04:10 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread library/src/system/thread_data.hpp Outdated

Comment thread library/src/solver_context/solver_context.cpp Outdated

tameware and others added 2 commits June 19, 2026 00:20

Fix init_debug_files suffix comment to match actual naming.

1e50677

Document that SolverContext passes "<serial>.txt" rather than a thread-id suffix, so debug output filenames are easier to trace. Co-authored-by: Cursor <cursoragent@cursor.com>

Fix debug file close when SolverContext is the last ThreadData owner.

d0a7c69

SearchContext kept a second shared_ptr, so use_count() was never 1 during destruction and close_debug_files() never ran; release that reference first. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware marked this pull request as draft June 19, 2026 04:24

tameware requested a review from Copilot June 19, 2026 04:24

tameware self-assigned this Jun 19, 2026

Copilot started reviewing on behalf of tameware June 19, 2026 04:24 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread library/src/solver_context/solver_context.cpp

Comment thread library/src/solver_context/solver_context.hpp Outdated

tameware and others added 2 commits June 19, 2026 00:31

Include algorithm and cstdio in solver_context.cpp.

f16271a

Add direct headers for std::min and std::snprintf instead of relying on transitive includes from other headers. Co-authored-by: Cursor <cursoragent@cursor.com>

Fix SolverContext wrapper comment on debug file lifecycle.

0e07849

Document that wrapper contexts skip init but may close debug files when they are the last shared_ptr holder of the ThreadData. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware requested a review from Copilot June 19, 2026 04:33

Copilot started reviewing on behalf of tameware June 19, 2026 04:33 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread library/src/BUILD.bazel

Comment on lines 58 to 62

"//library/src/trans_table",

"//library/src/moves:moves",

"//library/src:ab_stats",

"//library/src/system",

"//library/src/solver_context:solver_context",

Avoid compiling ab_stats.cpp twice in testable_dds.

5df3ca9

Exclude ab_stats.cpp from testable_dds_sources and link //library/src:ab_stats explicitly, matching :dds and preventing duplicate ABstats symbols. Co-authored-by: Cursor <cursoragent@cursor.com>

tameware marked this pull request as ready for review June 19, 2026 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes so that DDS_AB_STATS will work when it's set.#195

Fixes so that DDS_AB_STATS will work when it's set.#195
tameware wants to merge 14 commits into
dds-bridge:developfrom
tameware:AB-STATS-FIX

tameware commented Jun 17, 2026 •

edited

Loading

Uh oh!

zzcgumn commented Jun 17, 2026

Uh oh!

zzcgumn commented Jun 17, 2026

Uh oh!

tameware commented Jun 18, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

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

Uh oh!

Uh oh!

zzcgumn commented Jun 17, 2026

Uh oh!

zzcgumn commented Jun 17, 2026

Uh oh!

tameware commented Jun 18, 2026

Uh oh!

Uh oh!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tameware commented Jun 17, 2026 •

edited

Loading