Skip to content

Commit

Permalink
Randomize ValueTuple hash codes (dotnet/corefx#14141)
Browse files Browse the repository at this point in the history 
Merging on behalf of @jamesqo 

Commit migrated from dotnet/corefx@15d0033
  • Loading branch information
@jamesqo @jcouv
jamesqo authored and jcouv committed Dec 10, 2016
1 parent dff7161 commit 9ba4377
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 43 deletions.
8 changes: 5 additions & 3 deletions src/libraries/Common/src/System/Numerics/Hashing/HashHelpers.cs
View file
Expand Up @@ -6,12 +6,14 @@ namespace System.Numerics.Hashing
{
internal static class HashHelpers
{
public static readonly int RandomSeed = Guid.NewGuid().GetHashCode();

public static int Combine(int h1, int h2)
{
// The jit optimizes this to use the ROL instruction on x86
// RyuJIT optimizes this to use the ROL instruction
// Related GitHub pull request: dotnet/coreclr#1830
uint shift5 = ((uint)h1 << 5) | ((uint)h1 >> 27);
return ((int)shift5 + h1) ^ h2;
uint rol5 = ((uint)h1 << 5) | ((uint)h1 >> 27);
return ((int)rol5 + h1) ^ h2;
}
}
}
Expand Down
17 changes: 7 additions & 10 deletions src/libraries/System.ValueTuple/src/System/ValueTuple/ValueTuple.cs
View file
Expand Up @@ -246,40 +246,37 @@ string ITupleInternal.ToStringEnd()

internal static int CombineHashCodes(int h1, int h2)
{
// Forward to helper class in Common for this
// We keep the actual hashing logic there, so
// other classes can use it for hashing
return HashHelpers.Combine(h1, h2);
return HashHelpers.Combine(HashHelpers.Combine(HashHelpers.RandomSeed, h1), h2);
}

internal static int CombineHashCodes(int h1, int h2, int h3)
{
return CombineHashCodes(CombineHashCodes(h1, h2), h3);
return HashHelpers.Combine(CombineHashCodes(h1, h2), h3);
}

internal static int CombineHashCodes(int h1, int h2, int h3, int h4)
{
return CombineHashCodes(CombineHashCodes(h1, h2, h3), h4);
return HashHelpers.Combine(CombineHashCodes(h1, h2, h3), h4);
}

internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5)
{
return CombineHashCodes(CombineHashCodes(h1, h2, h3, h4), h5);
return HashHelpers.Combine(CombineHashCodes(h1, h2, h3, h4), h5);
}

internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5, int h6)
{
return CombineHashCodes(CombineHashCodes(h1, h2, h3, h4, h5), h6);
return HashHelpers.Combine(CombineHashCodes(h1, h2, h3, h4, h5), h6);
}

internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5, int h6, int h7)
{
return CombineHashCodes(CombineHashCodes(h1, h2, h3, h4, h5, h6), h7);
return HashHelpers.Combine(CombineHashCodes(h1, h2, h3, h4, h5, h6), h7);
}

internal static int CombineHashCodes(int h1, int h2, int h3, int h4, int h5, int h6, int h7, int h8)
{
return CombineHashCodes(CombineHashCodes(h1, h2, h3, h4, h5, h6, h7), h8);
return HashHelpers.Combine(CombineHashCodes(h1, h2, h3, h4, h5, h6, h7), h8);
}
}

Expand Down
30 changes: 0 additions & 30 deletions src/libraries/System.ValueTuple/tests/ValueTupleTests.cs
View file
Expand Up @@ -1030,28 +1030,6 @@ public static void EightTuples()
Assert.Equal(1, sc.CompareTo(CreateLong(1, 1, 1, 1, 1, 1, 3, ValueTuple.Create(1)), TestComparer.Instance));
Assert.Equal(1, sc.CompareTo(CreateLong(1, 1, 1, 1, 1, 1, 1, ValueTuple.Create(3)), TestComparer.Instance));

Assert.Equal(2138941962, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create()).GetHashCode());
Assert.Equal(2138941954, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8)).GetHashCode());
Assert.Equal(-1746596640, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9)).GetHashCode());
Assert.Equal(121964360, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10)).GetHashCode());
Assert.Equal(4363008, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11)).GetHashCode());
Assert.Equal(9413384, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12)).GetHashCode());
Assert.Equal(305131744, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12, 13)).GetHashCode());
Assert.Equal(1479338186, CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12, 13, 14)).GetHashCode());
Assert.Equal(1573514559, CreateLong(1, 2, 3, 4, 5, 6, 7, CreateLong(8, 9, 10, 11, 12, 13, 14, ValueTuple.Create())).GetHashCode());
Assert.Equal(1573514711, CreateLong(1, 2, 3, 4, 5, 6, 7, CreateLong(8, 9, 10, 11, 12, 13, 14, ValueTuple.Create(15))).GetHashCode());

Assert.Equal(2138941962, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create())).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(2138941954, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(-1746596640, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(121964360, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(4363008, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(9413384, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(305131744, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12, 13))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(1479338186, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, ValueTuple.Create(8, 9, 10, 11, 12, 13, 14))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(1573514559, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, CreateLong(8, 9, 10, 11, 12, 13, 14, ValueTuple.Create()))).GetHashCode(TestEqualityComparer.Instance));
Assert.Equal(1573514711, ((IStructuralEquatable)CreateLong(1, 2, 3, 4, 5, 6, 7, CreateLong(8, 9, 10, 11, 12, 13, 14, ValueTuple.Create(15)))).GetHashCode(TestEqualityComparer.Instance));

Assert.False(se.Equals(t, DummyTestEqualityComparer.Instance));

// Notice that 0-tuple prints as empty position
Expand Down Expand Up @@ -1128,19 +1106,11 @@ public static void LongTuplesWithNull()
[Fact]
public static void EightTuplesWithBadRest()
{
// Change as necessary if in the future
// the hash algorithm is modified again.
const int ExpectedHash = 1291467969;

var d = default(ValueTuple<int, int, int, int, int, int, int, int>);
d.Item1 = 1;
d.Rest = 42;
Assert.Equal(ExpectedHash, d.GetHashCode());
Assert.Equal(ExpectedHash, ((IStructuralEquatable)d).GetHashCode());
Assert.Equal("(1, 0, 0, 0, 0, 0, 0, 42)", d.ToString());

Assert.Equal(ExpectedHash, CreateLong(1, 2, 3, 4, 5, 6, 7, d).GetHashCode());

// GetHashCode only tries to hash the first 7 elements when rest is not ITupleInternal
Assert.Equal(ValueTuple.Create(1, 0, 0, 0, 0, 0, 0).GetHashCode(), d.GetHashCode());
Assert.Equal(((IStructuralEquatable)ValueTuple.Create(1, 0, 0, 0, 0, 0, 0)).GetHashCode(TestEqualityComparer.Instance), ((IStructuralEquatable)d).GetHashCode(TestEqualityComparer.Instance));
Expand Down

0 comments on commit 9ba4377

Please sign in to comment.