-
Notifications
You must be signed in to change notification settings - Fork 169
perf: improve hashing performance #2001
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR optimizes hashing performance across commonly used data structures by replacing JDK utility methods with custom implementations that avoid known performance issues. The changes target types frequently used as hash keys (Pair, Triple, Quadruple, composite keys) and value ranges.
Key changes:
- Replace
Objects.hash()andObjects.hashCode()with direct hashCode calculations to avoid array allocation and performance bugs - Standardize initial hash seed value from
7to1across all implementations - Leverage
@NullMarkedannotations to remove redundant null checks
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| Pair.java, Triple.java, Quadruple.java | Optimized equals/hashCode for tuple classes with manual null-safe implementations |
| BiCompositeKey.java | Updated composite key hashing to avoid Objects utility methods |
| MegaCompositeKey.java | Changed to use Arrays.deepEquals/deepHashCode for proper array comparison |
| TemporalValueRange.java | Removed null checks, updated equals/hashCode, simplified annotations |
| IntValueRange.java, LongValueRange.java | Updated primitive wrapper range classes with optimized hashing |
| BigIntegerValueRange.java, BigDecimalValueRange.java | Improved hashing for big number ranges |
| BooleanValueRange.java | Simplified contains() logic and removed unnecessary Boolean.valueOf() |
| SetValueRange.java, ListValueRange.java | Updated collection-based ranges with optimized equals/hashCode |
| CompositeCountableValueRange.java, NullAllowingCountableValueRange.java | Improved composite range hashing |
| DoubleValueRange.java | Simplified hashCode calculation (though inconsistent with other changes) |
| EmptyValueRange.java | Changed singleton hashCode from 7 to 0 |
| TestdataObjectEquals.java | Simplified test class hashCode |
| TemporalValueRangeTest.java | Removed null validation tests (covered by @NullMarked) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ld/solver/core/impl/domain/valuerange/buildin/composite/NullAllowingCountableValueRange.java
Outdated
Show resolved
Hide resolved
...java/ai/timefold/solver/core/impl/domain/valuerange/buildin/temporal/TemporalValueRange.java
Outdated
Show resolved
Hide resolved
...java/ai/timefold/solver/core/impl/domain/valuerange/buildin/temporal/TemporalValueRange.java
Outdated
Show resolved
Hide resolved
.../ai/timefold/solver/core/impl/domain/valuerange/buildin/bigdecimal/BigDecimalValueRange.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/BiCompositeKey.java
Outdated
Show resolved
Hide resolved
...java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java
Show resolved
Hide resolved
.../ai/timefold/solver/core/impl/domain/valuerange/buildin/biginteger/BigIntegerValueRange.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/util/Quadruple.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/test/java/ai/timefold/solver/core/testdomain/clone/lookup/TestdataObjectEquals.java
Outdated
Show resolved
Hide resolved
|
...n/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/ListValueRange.java
Outdated
Show resolved
Hide resolved
...in/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/collection/SetValueRange.java
Outdated
Show resolved
Hide resolved
...efold/solver/core/impl/domain/valuerange/buildin/composite/CompositeCountableValueRange.java
Outdated
Show resolved
Hide resolved
...ld/solver/core/impl/domain/valuerange/buildin/composite/NullAllowingCountableValueRange.java
Outdated
Show resolved
Hide resolved
.../main/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primint/IntValueRange.java
Outdated
Show resolved
Hide resolved
...java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primdouble/DoubleValueRange.java
Show resolved
Hide resolved
...ain/java/ai/timefold/solver/core/impl/domain/valuerange/buildin/primlong/LongValueRange.java
Outdated
Show resolved
Hide resolved
...java/ai/timefold/solver/core/impl/domain/valuerange/buildin/temporal/TemporalValueRange.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Frederico Gonçalves <[email protected]>
…nge/buildin/collection/SetValueRange.java Co-authored-by: Frederico Gonçalves <[email protected]>
Co-authored-by: Frederico Gonçalves <[email protected]>
….java Co-authored-by: Frederico Gonçalves <[email protected]>
Co-authored-by: Frederico Gonçalves <[email protected]>
…nge/buildin/composite/NullAllowingCountableValueRange.java Co-authored-by: Frederico Gonçalves <[email protected]>
…nge/buildin/composite/CompositeCountableValueRange.java Co-authored-by: Frederico Gonçalves <[email protected]>
…nge/buildin/collection/ListValueRange.java Co-authored-by: Frederico Gonçalves <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


hashCode()implementation in types which are commonly used in hashing (such as pairs and composite keys) due to the default methods for records doing far too much work.Objects.hashCode(...)andObjects.equals(...), with the assumption that the JDK project optimized those. We avoid actually calling these JDK methods due to https://bugs.openjdk.org/browse/JDK-8015417.