Tiered compilation should generate optimized overrides for ValueType.GetHashCode and ValueType.Equals

[YairHalberstadt]

It's well known that if a valuetype is used as a dictionary or hashset key, or in any other situation where .Equals or .GetHashCode is often used, you should override the default ValueType.Equals and .GetHashCode as the default implementation is reflection based and very slow.

For example here is a benchmark comparing a struct using the Equals and GetHashCode roslyn generates to one without any override:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Collections.Generic;

public static class Program
{
    public static void Main()
    {
        BenchmarkRunner.Run<Benchmarks>();
    }
}

public class Benchmarks
{
    [Benchmark]
    public void ValueTypeEquals()
    {
        var dictionary = new Dictionary<S1, int>();
        for (int i = 0; i < 10000; i++)
            dictionary[new S1 { s = i.ToString(), i = i }] = i;
    }

    [Benchmark]
    public void GeneratedEquals()
    {
        var dictionary = new Dictionary<S2, int>();
        for (int i = 0; i < 10000; i++)
            dictionary[new S2 { s = i.ToString(), i = i }] = i;
    }
}

struct S1
{
    public string s;
    public int i;
}

struct S2 : IEquatable<S2>
{
    public string s;
    public int i;

    public override bool Equals(object obj)
    {
        return obj is S2 s && Equals(s);
    }

    public bool Equals(S2 other)
    {
        return s == other.s &&
               i == other.i;
    }

    public override int GetHashCode()
    {
        return HashCode.Combine(s, i);
    }
}

Method	Mean	Error	StdDev
ValueTypeEquals	7.146 ms	0.1570 ms	0.4629 ms
GeneratedEquals	3.902 ms	0.1078 ms	0.3177 ms

However the override is functionally identical to the default one, and so it seems pointless to generate this for every struct used as a dictionary key. It's also too easy to forget to do so for a struct which needs it, or alternatively do it for too many structs and lead to code bloat.

Is it possible for the Jit to generate the same override at runtime? Whilst it would not be practical to do so for every struct, tiered compilation might be able tell you which structs have Equals and GetHashCode used in hot paths.

[EgorBo]

It's well known that if a valuetype is used as a dictionary or hashset key, you should override the default ValueType.Equals and .GetHashCode

it should always be overridden if used. Your S1 and S2 implementations are not the same, e.g.

int hc1 = new S1 { s = "Hello", i = 10 }.GetHashCode();
int hc2 = new S1 { s = "Hello", i = 12 }.GetHashCode();

hc1 and hc2 are the same value for S1 and different for S2. The only case you can rely on default GetHashCode for value types is when your struct consists of primitives without padding in between.

[YairHalberstadt]

What is the reason not to use all the fields in GetHashCode?

[EgorBo]

What is the reason not to use all the fields in GetHashCode?

runtime/src/coreclr/src/vm/comutilnative.cpp

Line 2032 in 4224c4c

// change the implementation here to use all fields instead of just the 1st one.

[YairHalberstadt]

So it would theoretically be possible to make both changes in one go, and use all the fields for GetHashCode, and then optimize that on the fly when it seems necessary.

[EgorBo]

So it would theoretically be possible to make both changes in one go, and use all the fields for GetHashCode, and then optimize that on the fly when it seems necessary.

Maybe? I personally agree with #9802 (comment)

[AndyAyersMS]

The jit can't generate overrides like this; that has to be done when the method table for the type is created. Changing this to area-vm.

[GSPP]

Related: #4414