Roslyn: Struct unboxing - very bad performance of pattern matching

Created on 24 Apr 2017  路  3Comments  路  Source: dotnet/roslyn

Version Used: 15.1 (26403.7)

Suppose we have following struct:
```C#
struct MyStruct
{
public readonly int Value;

public bool Equals1(object obj)
{
    if (obj is MyStruct)
    {
        return Equals((MyStruct)obj);
    }
    return false;
}

public bool Equals2(object obj)
{
    try
    {
        return Equals((MyStruct)obj);
    }
    catch
    {
        return false;
    }
}

public bool Equals3(object obj)
{
    var sn = obj as MyStruct?;
    return sn.HasValue ? Equals(sn.Value) : false;
}

public bool Equals4(object obj)
{
    return obj is MyStruct s ? Equals(s) : false;
}

public bool Equals(MyStruct other)
{
    return Value == other.Value;
}

}


`Equals1` goes against recommendation for not using "is" and than "cast" but is the fastest one.
`Equals2` looks stupid but is the second fastest (assuming `obj` is always `MyStruct`).
`Equals3` looks more clever (i was using it before C# 7) but it is MUCH MUCH slower than `Equals1` and even `Equals2` (found out just now).
`Equals4` is the the slowest one. It is translated to similar MSIL as `Equals3` but there is one more local variable.

I believe the syntax of `Equals4` should be the recommended one (or any better idea?).
Not sure why it's so slow, but probably because of working with `Nullable<MyStruct>`.
Shouldn't compiler emit more efficient MSIL for this cases?

Here is my measurements (Win7, Intel Xeon E5-1650 v3):
Release/32bit:
Equals1: 367.0922ms
Equals2: 1084.6545ms
Equals3: 10767.0583ms
Equals4: 11156.0162ms
Equals: 93.8451ms

Release/64bit:
Equals1: 279.8061ms
Equals2: 1052.9261ms
Equals3: 8290.5574ms
Equals4: 8774.8108ms
Equals: 95.9861ms

Measuring code looks like this:
```C#
var watch = new Stopwatch();
var a = new MyStruct();
var b = (object)new MyStruct();

a.Equals1(b);
a.Equals2(b);
a.Equals3(b);
a.Equals4(b);

const int Count = 300000000;

watch.Restart();
for (int i = 0; i < Count; i++)
{
    a.Equals1(b);
}
Console.WriteLine($"Equals1: {watch.Elapsed.TotalMilliseconds}ms");

watch.Restart();
for (int i = 0; i < Count; i++)
{
    a.Equals2(b);
}
Console.WriteLine($"Equals2: {watch.Elapsed.TotalMilliseconds}ms");

watch.Restart();
for (int i = 0; i < Count; i++)
{
    a.Equals3(b);
}
Console.WriteLine($"Equals3: {watch.Elapsed.TotalMilliseconds}ms");

watch.Restart();
for (int i = 0; i < Count; i++)
{
    a.Equals4(b);
}
Console.WriteLine($"Equals4: {watch.Elapsed.TotalMilliseconds}ms");

watch.Restart();
for (int i = 0; i < Count; i++)
{
    a.Equals(a);
}
Console.WriteLine($"Equals: {watch.Elapsed.TotalMilliseconds}ms");
Area-Compilers Bug New Language Feature - Pattern Matching

Most helpful comment

We of course are using generic version of Equals. But when you implement IEquatable<T> you should override Equals(object) and GetHashCode(). Today I saw in some our code Equals2 style implementation and was angry that someone is using it. But when I was comparing performance of Equals3/Equals4 (which I believed must be better) and Equals2 style I was really surprised.

Ok, I accept that recommendation for "not using 'is' and than 'cast'" is not valid for value types and it is probably the right way of doing this.

So back to the main question - shouldn't more efficient MSIL be generated for pattern matching with value types?
For code:
```C#
private static void M(object obj)
{
if (obj is MyStruct value)
{
value.ToString();
}
}

following is generated:
```C#
var myStruct = obj as MyStruct?;
var value = myStruct.GetValueOrDefault();
if (myStruct.HasValue)
{
    value.ToString();
}

which is inefficient and following should be ideally generated:
C# if (obj is MyStruct) { ((MyStruct)obj).ToString(); }

All 3 comments

Probably want to add
a.Equals(a);
To your warmup

Equals1 goes against recommendation for not using "is" and than "cast" but is the fastest one.

From what I've seen, this recommendation is for reference types (where the as operator is applicable), not value types. Equals1 is what I would expect to see for value types, aside from reordering the clauses:

public bool Equals1(object obj)
{
    if (!(obj is MyStruct))
    {
        return false;
    }

    return Equals((MyStruct)obj);
}

With that said, if you are using any of these equality methods, you're probably not using value types efficiently. It only operates on boxed value types, so if it's showing up often then you could be using a value type where a reference type makes more sense. The primary one you should be using is directly calling the method that implements IEquatable<MyStruct>.

We of course are using generic version of Equals. But when you implement IEquatable<T> you should override Equals(object) and GetHashCode(). Today I saw in some our code Equals2 style implementation and was angry that someone is using it. But when I was comparing performance of Equals3/Equals4 (which I believed must be better) and Equals2 style I was really surprised.

Ok, I accept that recommendation for "not using 'is' and than 'cast'" is not valid for value types and it is probably the right way of doing this.

So back to the main question - shouldn't more efficient MSIL be generated for pattern matching with value types?
For code:
```C#
private static void M(object obj)
{
if (obj is MyStruct value)
{
value.ToString();
}
}

following is generated:
```C#
var myStruct = obj as MyStruct?;
var value = myStruct.GetValueOrDefault();
if (myStruct.HasValue)
{
    value.ToString();
}

which is inefficient and following should be ideally generated:
C# if (obj is MyStruct) { ((MyStruct)obj).ToString(); }

Was this page helpful?
0 / 5 - 0 ratings