Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

The Witness: Another small access rule optimisation #4256

Merged
merged 1 commit into from
Dec 4, 2024
Merged

The Witness: Another small access rule optimisation #4256

merged 1 commit into from
Dec 4, 2024

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Nov 27, 2024 •
edited
Loading

This is split off from #3961 in an effort to split some of my giant PRs into smaller PRs.

  • Use state.has_all instead of state.has_all_counts when all counts are 1
  • Sort any calls to has_all or has_all_counts by which condition is least likely to be true

The performance gain here is minor (1-2%), but it becomes bigger in the aforementioned PR because of the nature of the changes that PR makes.

@NewSoupVi NewSoupVi added is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Nov 27, 2024
@NewSoupVi NewSoupVi changed the title The Witness: Another small optimisation The Witness: Another small access rule optimisation Nov 27, 2024
Exempt-Medic
Exempt-Medic approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

LGTM, the code makes sense, and it generates just fine. I'll just trust Violet on the optimization aspects of this

@NewSoupVi NewSoupVi merged commit f43fa61 into ArchipelagoMW:main Dec 4, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Exempt-Medic Exempt-Medic Exempt-Medic approved these changes

@Berserker66 Berserker66 Berserker66 approved these changes

Assignees
No one assigned
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NewSoupVi @Berserker66 @Exempt-Medic