instrument: TT lookup/hit counters (hot-path) + env-gated summary

[wopdevries]

Minimal, low-risk instrumentation to measure the transposition-table hit rate on the search hot path.

Changes:

• Add tt_lookup_count and tt_hit_count counters to ThreadData
• Increment counters in apply_ab_tt_lookup (the canonical AB hot-path TT probe)
• Print env-gated summary at SOLVER_DONE when DDS_PRINT_TT_STATS=1

No behavioral changes — counters are cheap increments, output is gated.

Measurement results on hands/largest.txt (21 hardest hands, single-threaded):
Updated findings after per-solve reset fix:

• Per-board hit rate: 57-75% (varies by hand difficulty)
• Per-board overwrite rate: 2-24% (mostly 2-9%)
• Low overwrite rate confirms: replacement churn is NOT the main cause of misses
• Hard hands have lower hit rates due to genuinely unique positions, not TT churn
• Algorithmic improvements (pruning, move ordering) are the next optimization target

How to test:

bazelisk build //library/tests:dtest
DDS_PRINT_TT_STATS=1 ./bazel-bin/library/tests/dtest -f hands/largest.txt -s solve -n 1

[Copilot]

🟡 Changes recommended

It introduces compilation fragility (missing direct standard includes) and a real stats correctness issue (new PageStats fields aren’t reset on TT teardown).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds lightweight instrumentation to measure transposition-table (TT) effectiveness on the alpha-beta search hot path, and reports an environment-gated summary at solver completion to support TT sizing/tuning work.

Changes:

• Added per-ThreadData TT probe counters (tt_lookup_count, tt_hit_count) and incremented them in apply_ab_tt_lookup.
• Added env-gated (DDS_PRINT_TT_STATS) stderr summary printing on solver completion paths.
• Extended TransTableL::PageStats with insertion/overwrite counters and incremented them on TT entry creation/overwrite.

File summaries

File	Description
library/src/trans_table/trans_table_l.hpp	Extends PageStats with add/overwrite counters.
library/src/trans_table/trans_table_l.cpp	Initializes new PageStats fields; increments add/overwrite counters in create_or_update.
library/src/system/thread_data.hpp	Adds per-thread TT lookup/hit counters to ThreadData.
library/src/solver_if.cpp	Resets counters per solve and prints env-gated TT hit-rate summary at completion.
library/src/ab_search.cpp	Increments TT lookup/hit counters in the canonical TT probe path.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There are a few concrete mismatches and consistency issues (notably the env var gating contract and documentation/formatting inconsistencies) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

library/src/trans_table/trans_table_l.hpp:477

• This inline override uses same-line braces; please reformat it to match the Allman function brace style used throughout the codebase.

    auto get_op_stats(int& adds, int& overwrites, int& harvests) const -> void override {
      adds = page_stats_.num_adds_;
      overwrites = page_stats_.num_overwrites_;
      harvests = page_stats_.num_harvests_;
    }

• Files reviewed: 8/8 changed files
• Comments generated: 6
• Review effort level: Lite

[wopdevries]

Direct recommendation: This is a small, low-risk instrumentation PR that I would approve after addressing a couple of portability / style nits. It adds per-thread TT lookup/hit counters, per-solve reset and summary printing gated by DDS_PRINT_TT_STATS, and minimal trans-table operation counters. I scanned the diffs and ran a focused review below.

What I did and how I proceeded

Loaded the PR review workflow and fetched the PR file diff (all changed files).
Read the changes across: library/src/ab_search.cpp, library/src/solver_if.cpp, thread_data.hpp, trans_table headers/impl, and the trans_table unit test mock.
Focused on correctness, thread-safety assumptions, API/ABI surface (virtuals), test updates, and small portability/formatting issues.

PR summary

Purpose: Add cheap instrumentation to measure transposition-table (TT) lookup/hit counts on the Alpha-Beta hot path and print a per-solve summary when DDS_PRINT_TT_STATS=1.
Main changes:
    Add uint64_t tt_lookup_count and tt_hit_count to ThreadData.
    Increment counters in apply_ab_tt_lookup (library/src/ab_search.cpp).
    Reset per-solve counters and call tt->reset_op_stats() at solve start in solver_if.cpp.
    Print summary (lookups, hits, hit_rate, plus TT add/overwrite/harvest stats) when DDS_PRINT_TT_STATS=1 at solver completion in multiple solve functions.
    Add get_op_stats/reset_op_stats virtuals to TransTable interface and implementations (L and S), and update TransTableL to collect adds/overwrites.
    Update trans_table tests' MockTransTable to implement the new virtuals.
Scope: 8 files changed, +112 additions, -1 deletion. Risk: low (non-behavioral for solver correctness; counters and printing only).

Core changes and rationale

Instrumentation point is apply_ab_tt_lookup, which is the canonical AB hot-path TT probe — good place to measure practical hit rates.
Per-thread counters live in ThreadData (per-context, single-threaded claim). The increments are guarded by if (thrp) and only increment small integer counters: very cheap.
The trans-table adds/overwrite/harvest counters are exposed via new virtuals so the solver can include them in printed summaries.
A small per-solve reset ensures stats are per-board/solve rather than accumulating across many boards — the PR mentions fixing an earlier per-solve reset bug, and I see reset_op_stats() called at start.

Other changes / notable snippets

library/src/ab_search.cpp: increments lookup and hit counters inside apply_ab_tt_lookup right after the TT lookup is done.
library/src/solver_if.cpp:
    Includes added: <cstdlib> and <cstdio>.
    Reset of thrp->tt_* counters and a call to tt->reset_op_stats() at the start of solve functions.
    Printing code gated on getenv("DDS_PRINT_TT_STATS") == "1" and using std::fprintf(stderr, ...) to print counts and rates; uses get_op_stats to print adds/overwrites/harvests and compute overwrite rate.
thread_data.hpp: adds uint64_t counters and includes <cstdint>.
trans_table.*:
    trans_table.hpp: adds virtual get_op_stats and reset_op_stats in the interface.
    trans_table_l.hpp/.cpp: implements counters num_adds_ and num_overwrites_, increments them in create_or_update, and provides get/reset implementations.
    trans_table_s.hpp: returns zeros (not instrumented) in get_op_stats/reset_op_stats.
tests: MockTransTable updated to implement the new virtuals (so tests compile).

Merge readiness and risk assessment

Overall status: Low risk. The changes are instrumentation-only and gated at runtime for printing. The PR updates virtual interface and implementations (minor API addition that the repo already adjusts where needed).
Code quality: Clear and small changes; tests updated to compile.
Test coverage: The unit test mock was updated; I don't see new tests asserting counters, but compilation is handled.
Blockers: none critical found in diffs.

Critical/code-anchored review items

library/src/solver_if.cpp: Use a portable printf format for uint64_t counters (avoid hardcoding %llu); prefer PRIu64 from <inttypes.h> or cast to unsigned long long consistently across platforms and document the choice.
library/src/solver_if.cpp: The fprintf format uses %llu but casts to (unsigned long long) — consider including <inttypes.h> and using PRIu64 for clarity/portability, or add a small comment explaining the cast.
library/src/solver_if.cpp: The getenv check uses std::string(env) == "1" which allocates a string; consider comparing env[0] == '1' && env[1] == '\0' to avoid allocation in hot tooling code (micro-optimization).
library/src/trans_table/trans_table_l.cpp: The create_or_update instrumentation increments num_adds_ and conditionally num_overwrites_. Please add a short comment that documents the overwrite condition (n == BlocksPerEntry) so future readers understand why that signifies an overwrite.
library/src/ab_search.cpp: The tt counter increments assume per-thread single ownership — ensure apply_ab_tt_lookup cannot be called concurrently for the same ThreadData from multiple threads; if that is guaranteed elsewhere, add a brief comment to justify non-atomic counters.

Possible improvements / suggestions (non-blocking)

library/src/solver_if.cpp: Prefer using the project's logging API (if one exists) rather than fprintf(stderr, ...) so the output is consistent with other diagnostics and can be filtered/redirected more easily. If there's no logging facility, add a one-line justification for using stderr directly.
library/src/solver_if.cpp & thread_data.hpp: Consider encapsulating per-solve stat reset into a small helper method (e.g., ThreadData::ResetTTStats()) to avoid repeating thrp->tt_lookup_count = 0; thrp->tt_hit_count = 0; in multiple locations.
library/src/trans_table/trans_table_l.hpp/.cpp: Consider making the add/overwrite counters 64-bit if the trans-table is used in very long runs (probably unnecessary for per-solve stats, but safe).

Small style/consistency nits (purely optional)

The new includes (<cstdlib>, <cstdio>) are fine; just verify include ordering is consistent with project style.
When printing percentages, consider guarding divide-by-zero defensively (the code already checks lookup_count > 0 before computing hit_rate — good).

Summary / verdict

This PR is well-scoped, low-risk, and useful for performance investigation. The only actionable changes I recommend before merging are the portability/formatting nit around printing uint64_t and a couple of clarifying comments. None of the findings are blocking for merging.