Deduplicate identical deals in CalcAllTablesN#215
Draft
tameware wants to merge 1 commit into
Draft
Conversation
CalcAllTablesN expanded every deal x strain into a board and solved them all, even when a batch contained identical deals. v2.9 deduplicates identical deals (crossref/CopyCalcSingle), solving each distinct deal once and copying the table to the copies. Detect duplicate deals by card content, solve only the unique deals, and map every deal (unique or copy) back to its canonical solve when filling the result tables. Identical deals always yield identical double-dummy tables, so this is exact. On hands/list1000.txt (36% duplicate deals) single-threaded calc drops from ~104s to ~68s of user time, matching v2.9 (~65s); the residual gap versus v2.9 is parallel scaling, addressed separately. All library tests pass and dtest's table validation is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves CalcAllTablesN performance by deduplicating identical DdTableDeal inputs, solving each unique deal once, and copying the solved double-dummy tables to duplicate entries—matching the legacy v2.9 “crossref” behavior and reducing redundant solves.
Changes:
- Added card-content equality check for
DdTableDealand a deduplication pass to map each deal to a canonical unique deal. - Adjusted board construction to solve only unique deals and remap results back to all original deal indices.
Comment on lines
266
to
+274
| if (count * dealsp->no_of_tables > MAXNOOFTABLES * DDS_STRAINS) | ||
| return RETURN_TOO_MANY_TABLES; | ||
|
|
||
| // Deduplicate identical deals: solve each distinct deal once and map every | ||
| // copy back to the canonical solve. Identical deals always produce identical | ||
| // double-dummy tables, so this is exact. Mirrors v2.9's crossref behaviour and | ||
| // avoids re-solving repeated deals in a batch. | ||
| std::vector<int> deal_to_unique(static_cast<unsigned>(dealsp->no_of_tables)); | ||
| std::vector<int> unique_deals; |
Comment on lines
+269
to
+294
| // Deduplicate identical deals: solve each distinct deal once and map every | ||
| // copy back to the canonical solve. Identical deals always produce identical | ||
| // double-dummy tables, so this is exact. Mirrors v2.9's crossref behaviour and | ||
| // avoids re-solving repeated deals in a batch. | ||
| std::vector<int> deal_to_unique(static_cast<unsigned>(dealsp->no_of_tables)); | ||
| std::vector<int> unique_deals; | ||
| for (int m = 0; m < dealsp->no_of_tables; m++) | ||
| { | ||
| int found = -1; | ||
| for (unsigned u = 0; u < unique_deals.size(); u++) | ||
| { | ||
| if (same_deal_cards(dealsp->deals[unique_deals[u]], dealsp->deals[m])) | ||
| { | ||
| found = static_cast<int>(u); | ||
| break; | ||
| } | ||
| } | ||
| if (found < 0) | ||
| { | ||
| deal_to_unique[static_cast<unsigned>(m)] = | ||
| static_cast<int>(unique_deals.size()); | ||
| unique_deals.push_back(m); | ||
| } | ||
| else | ||
| deal_to_unique[static_cast<unsigned>(m)] = found; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Initial benchmarking shows this change may do more harm than good. On the back-burner for now.
zzcgumn
reviewed
Jun 29, 2026
|
|
||
| namespace | ||
| { | ||
| auto same_deal_cards(const DdTableDeal& a, const DdTableDeal& b) -> bool |
Collaborator
There was a problem hiding this comment.
I would define this as operator==(const DdTableDeal& a, const DdTableDeal& b) -> bool even though DdTableDeal is a C struct. This file is after all pure C++.
Would Cursor disagree?
Collaborator
Author
There was a problem hiding this comment.
I'll ask if you like, but we are abandoning this PR as, per our email discussion, we don't expect identical deals in practice.
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.
CalcAllTablesN expanded every deal x strain into a board and solved them all, even when a batch contained identical deals. v2.9 deduplicates identical deals (crossref/CopyCalcSingle), solving each distinct deal once and copying the table to the copies.
Detect duplicate deals by card content, solve only the unique deals, and map every deal (unique or copy) back to its canonical solve when filling the result tables. Identical deals always yield identical double-dummy tables, so this is exact.
On hands/list1000.txt (36% duplicate deals) single-threaded calc drops from ~104s to ~68s of user time, matching v2.9 (~65s); the residual gap versus v2.9 is parallel scaling, addressed separately. All library tests pass and dtest's table validation is unchanged.