Improve hash implementations for paths in Matter.framework. #41368
Conversation
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
This gives better set-insertion behavior.
There was a problem hiding this comment.
Pull Request Overview
This PR improves hash function implementations for various path objects in the Matter framework to provide better set-insertion behavior. The changes replace simple bit-shifting hash calculations with a centralized, more sophisticated hash function.
- Introduces a new
HashPathutility function that provides optimized hash calculations for different NSUInteger sizes - Replaces existing hash implementations across multiple path classes to use the centralized function
- Updates bit allocation strategy to reduce hash collisions
|
PR #41368: Size comparison from a6bb081 to 0a2c56c Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the hash implementations for various path objects, which should lead to better performance in hash-based collections. The introduction of a centralized HashPath function is a good approach. I've found a couple of issues: one is a bug in the new HashPath function that could lead to hash collisions, and the other is an inconsistency in -[MTRClusterPath hash] which doesn't use the new helper function. My review includes suggestions to fix these issues to make the hashing more robust and consistent.
jtung-apple
approved these changes
Oct 9, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41368 +/- ##
==========================================
- Coverage 50.95% 50.95% -0.01%
==========================================
Files 1378 1378
Lines 100614 100643 +29
Branches 13025 13028 +3
==========================================
+ Hits 51272 51283 +11
- Misses 49342 49360 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
bzbarsky-apple
commented
Oct 9, 2025
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
bzbarsky-apple
commented
Oct 9, 2025
|
PR #41368: Size comparison from a6bb081 to aa6356d Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This gives better set-insertion behavior.
Testing
Manual measurements of the number of isEqual calls that happen during set operations.