Fix small performance regression versus v2.9#214
Open
tameware wants to merge 3 commits into
Open
Conversation
The heuristic extraction refactor changed weight_alloc_trump_void1's first branch from `lead_suit == trump` to `suit == trump`. Since that is exhaustive with the following `else if (suit != trump)`, the three ruffing branches (using the `24 - rank + ...` formula) became dead code, and trump ruffs were scored with side-suit discard weights instead. This mis-ordered ruffs, costing alpha-beta cutoffs. The effect is small for solve but compounds heavily in calc's warm-TT iterative deepening: calc explored ~34% more nodes than v2.9. Restoring the original `lead_suit == trump` pitch branch makes the ruffing branches reachable again and cuts calc time ~25% (gap to v2.9: 1.37x -> 1.02x). Ordering-only change; double-dummy results are unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Per Copilot. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The heuristic/quick-tricks refactor introduced static_cast<unsigned char>
wrappers on values that v2.9 used as signed, changing search behavior:
- make_3 / make_3_ctx: winner[]/second_best[] .hand and .rank were cast
to unsigned char, turning the -1 "no card" sentinel into 255. This broke
winner[trump].hand == -1 style checks in QuickTricks, losing cutoffs.
- weight_alloc_trump_void2 / _void3: rel_rank[aggr[suit]][...] indexed
through static_cast<unsigned char>(aggr[suit]), truncating the 13-bit
aggregate holding to 8 bits and reading the wrong rel_rank row.
- QuickTricksPartnerHand{Trump,NT}: bit_map_rank index cast the signed
rank through unsigned char.
With these reverted to v2.9's signed handling, the per-move-generation
ordering trace now matches v2.9 exactly (0 divergences on list1), closing
the residual calc gap to parity. Ordering/pruning-only change; double-dummy
results are unchanged and all library tests pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to regain a small (~2%) performance regression vs v2.9 by removing redundant/narrowing casts and simplifying a hot-path heuristic branch, reducing unnecessary conversions and work in frequently executed code paths.
Changes:
- Removes redundant
static_cast<unsigned char>(...)/ double-cast patterns when indexing rank/relative-rank tables. - Simplifies part of
weight_alloc_trump_void1()for the “void in trump when trump is led” discard case. - Removes unnecessary casts when copying winner/second-best ranks and hands from
RelRanksTypeintoPos.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| library/src/quick_tricks.cpp | Simplifies rank-to-index conversion when setting win_ranks from AbsRankType::rank. |
| library/src/heuristic_sorting/heuristic_sorting.cpp | Removes redundant casts in rel_rank indexing and simplifies a discard-weighting branch in a hot heuristic function. |
| library/src/ab_search.cpp | Removes unnecessary casts when updating cached winner/second-best info from thrp->rel[...]. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Regains roughly 2% of performance.
Compare is the speedup-opus branch. Branch is this branch.