Powershell: Change Utils.CombineHashCodes() to use .NET Core's HashCode.Combine() method

Created on 21 Jun 2018  路  4Comments  路  Source: PowerShell/PowerShell

After discussion in https://github.com/PowerShell/PowerShell/pull/7126#discussion_r197228400 and reading various posts (1, 2, 3, 4, 5, 6), it seems like a multiplicative hash function might be better implementation than the current XOR hash used in Utils.CombineHashCodes().

Note that changing the algorithm is, by definition, not a breaking change. To quote the MSDN page on Object.GetHashCode():

A hash code is intended for efficient insertion and lookup in collections that are based on a hash table. A hash code is not a permanent value... Do not serialize hash code values or store them in databases... Do not send hash codes across application domains or processes. In some cases, hash codes may be computed on a per-process or per-application domain basis... Do not test for equality of hash codes to determine whether two objects are equal. (Unequal objects can have identical hash codes.) To test for equality, call the ReferenceEquals or Equals method.

Also, it would be worthwhile implementing an array overload of Utils.CombineHashCodes(), like Utils.CombineHashCodes(params int[] hs) and Utils.CombineHashCodes(int h, params int[] hs).

Issue-Code Cleanup WG-Engine

Most helpful comment

Great, so they added a public api - best to just use that and remove the PowerShell copy.

All 4 comments

That code is copied from .Net here - so if you believe you have a better implementation, consider opening an issue with corefx.

I think the shift avoids the problems of xor transitivity and that shift + xor is faster than multiplication.

And last thing - an overload taking params is discouraged because it allocates memory. For non-library code, such an overload is never needed because we can always add 1 more overload that takes the exact number of parameters. Library code does not have that luxury, so libraries make the trade-off of providing a number of overloads for the most common uses and the inefficient overload for all other cases.

We have several alternative implementations of GetHashCode method in the repo.
I discovered this when I had to implement this method for SymVer type some time ago. I had the idea of opening a case on this issue but I thought that this is a very rare situation. And this question again appeared. I think we need to revise our code and if possible use the standard implementation from CoreFX.

Great, so they added a public api - best to just use that and remove the PowerShell copy.

Was this page helpful?
0 / 5 - 0 ratings