-
-
Notifications
You must be signed in to change notification settings - Fork 30
Fix: combined hashcode conflict #77
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
Hi @ikws4,
Could you provide a code snippet to reproduce this issue?
int CombineHashCode(int hash1, int hash2) { var hash = 17; hash = hash * 31 + hash1; hash = hash * 31 + hash2; return hash; } CombineHashCode(601853953, -523751028) // 953868668 CombineHashCode(-1082829219, 161819752) // 953868668
In my case, I have two viewmodels with different properties, but CombineHashCode merges them into the same hash code. Later, when accessing MemberInfo, it gets messed up.
ChebanovDmitry
commented
Apr 16, 2025
In my case, I have two viewmodels with different properties, but CombineHashCode merges them into the same hash code. Later, when accessing MemberInfo, it gets messed up.
Can you share the code from these two ViewModels here? Thanks.
Sorry, I'm unable to provide those ViewModels because the HashCode changes each time it's compiled. It's very hard to reproduce, but I can confirm that the issue was resolved with this fix.
I can confirm that the issue was resolved with this fix.
Yes, your solution solves the problem. But I think we can come up with a more performant solution that does not require additional memory allocation.
Can you change the CombineHashCode
method as follows and test it with your ViewModels?
[MethodImpl(MethodImplOptions.AggressiveInlining)] public static int CombineHashCode(int hash1, int hash2) { var hash = (uint) hash1; hash ^= (uint) hash2 + 0x9e3779b9 + (hash << 6) + (hash >> 2); return (int) hash; }
If this solution works, you can modify your PR and I will merge it. Thanks 🙂
I asked Gemini to write a test for CombineHashCode
. Here are the results, it seems the problem still exists🤔.
ChebanovDmitry
commented
Apr 17, 2025
I asked Gemini to write a test for
CombineHashCode
. Here are the results, it seems the problem still exists🤔.
I don't have access to your prompt
Sorry, I've updated the link's permission. Would you please try again?
Thanks!
There is no hash function with a fixed output size (int
in our case) that can completely eliminate collisions. The collision rate in the provided CombineHashCode
method is relatively low (100 collisions out of 1,000,000 attempts, roughly 0.01%). I think this is acceptable in our case since we are not aiming to write a cryptographically strong method.
But in future versions, it might be worth thinking about changing the return type from int
to ulong
, for example.
No description provided.