Skip to content

Conversation

@Nihisil
Copy link
Contributor

@Nihisil Nihisil commented Jan 27, 2026

I started to work on #125 and resurrected the code for hands validation.

So far I checked on 2kk hands and no regressions were found. Before the release 2.0 I will check on more hands and will update readme with this info.

While I was here I did some profiling and few optimizations there and there to speed up lib a bit. They were low hanging fruits that can be done without sacrificing readability much and without big refactorings.

  • Baseline: 7450 hands/sec (324.838s total)
  • Optimized: 9541 hands/sec (223.332s total)
  • Improvement: ~22% performance gain

@Nihisil Nihisil requested a review from Apricot-S January 27, 2026 16:01
@Apricot-S Apricot-S changed the title chore: improve performance across the project refactor!: Improve performance across the project Jan 27, 2026
@Apricot-S
Copy link
Collaborator

Thanks for the great performance improvements.

One note: switching several constants from list to frozenset and renaming them introduces a breaking change for users who import these constants directly.
We track breaking changes explicitly, so I updated the PR title to refactor! to reflect this.

Could you add a short note here describing the intention behind the constant renaming and the expected impact on users?
This will help us document the change clearly in the release notes.

@Nihisil
Copy link
Contributor Author

Nihisil commented Jan 28, 2026

switching several constants from list to frozenset and renaming them introduces a breaking change for users who import these constants directly.

It is a good point, I completely forgot about compatibility when was working on it.

I have reverted changes in constant names, but keep them as frozensets and add a note about breaking changes in PR description.

@Nihisil
Copy link
Contributor Author

Nihisil commented Jan 28, 2026

Also updated hand calculation logic a bit to calculate yaku only when it is necessary, it added +2% performance gain on default hand config (and didn't introduce breaking changes)

@Nihisil Nihisil requested a review from Apricot-S January 28, 2026 15:34
# Conflicts:
#	tests/tests_utils.py
@Nihisil
Copy link
Contributor Author

Nihisil commented Jan 28, 2026

@Apricot-S thanks for thoughtful review. I have addressed your comments and also added breaking changes notes to changelog.md. Once this branch is merged, please add notes to the same file about breaking changes that were introduced in your other PRs

@Apricot-S Apricot-S added this to the v2.0.0 milestone Jan 29, 2026
@Nihisil Nihisil requested a review from Apricot-S January 29, 2026 02:12
Copy link
Collaborator

@Apricot-S Apricot-S left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Apricot-S Apricot-S merged commit 563eeaa into master Jan 29, 2026
10 checks passed
@Apricot-S Apricot-S deleted the benchmark branch January 29, 2026 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants