Find the slow down

Huh, I did not realize you can take address of a variable that's not definitely assigned.

Svick,

I am surprised by this as well.

The killer is that this code is correct, it does the right thing Are you sure? I'm pretty sure there's a bug in GetHashCode. When Value is an int, you use &l instead of &i, but l is 0 since the value is not a long. Also, we don't know what Standart.Hash.xxHash.xxHash32.ComputeHash does

Thomas,

The code will produce the right result, in all cases. There is a performance issue, for sure, but any unit test you want will work. The bug is there, yes, but it won't show as incorrect behavior.

Is the value of l (L) zero (because of the typo bug) so the hashcode you are returning is always the same for all entries. You are basically turning the dictionary into one long linked list.

// Ryan

How can article from future be readable today? :D

There is bug in the code (as Thomas and Ryan mentioned). It is getting address of lowercase 'L' where it should take address of 'i'.

Maybe it's copy/paste issue. But from my tests changing 'l' to 'i' fixes the issue.

The bug causes the hashcode to be the same for all those NumericValue objects used as Dictionary key. But Equals is returning False when comparing those objects and Dictionary have a problem when adding items with such keys.

Shouldn't NumericValue implement IEquitable<NumericValue>? I know the recommendation for types that serve as Dictionary keys. There's also a recommendation that GetHashCode does not allocate. I'm sure you can calculate the hash without the Span allocations...

Itay,

There is no requirement for implementation of any interfaces. Any object can be put in a dictionary. Note that there is no memory allocations in the GetHashCode() method, we are using Span, which are value types.

Beniamin,

I actually moved them around in the scheduling queue, looks like I did that only after it was actually published. And yes, that is the issue, and nearly impossible to spot.

There's no requirement, but there's a recommendation to implement it if you want better performance.

IItay,

Can you find any reason why that would be the case?

I'm not an expert in that area, but from what I understand, implementing IEquatable<T> results in using the more efficient GenericEqualityComparer, instead of the ObjectEqualityComparer (which if I understand correctly, performs boxing/unboxing).

Itay,

That is really interesting, I just checked the code, and I believe that you are correct. Good to know.

We could probably argue over whether the statement "this code is correct" is actually true:

public override int GetHashCode() => 0; 
public override int GetHashCode() => Name.GetHashCode();

Are these two implementations of GetHashCode "correct" ? The second version is essentially your bug (well, a fixed value is used instead of no value at all, and only if the Value happens to be an int).

GetHashCode isn't required to produce unique results, so these versions aren't "incorrect" in that sense. But I'd argue that you would probably call neither of my 2 versions of GetHashCode "correct" if you were to see them in a code review, wouldn't you?

Enzi,

There is no argument here that this is a bug. The problem is that it will still "work" with 0 as the value.

NumericValue should implement IEquatable<NumericValue>.

There's a lot of boxing/unboxing due to the value being a value type stored as object. long is good enough.

The dictionary can be initialized with initial capacity, to avoid dynamic growth.

```csharp unsafe class Program { public class NumericValue { public string Name; public object Value;

    public override bool Equals(object obj)
    {
        return obj is NumericValue value &&
                Name == value.Name &&
                Equals(Value, value.Value);
    }

    public unsafe override int GetHashCode()
    {
        Span<byte> span;
        if (Value is long l)
        {
            span = new Span<byte>(&l, sizeof(long));
        }
        else if (Value is int i)
        {
            span = new Span<byte>(&l, sizeof(int));
        }
        else
        {
            throw new NotSupportedException("Unknown value");
        }

        return (int)Standart.Hash.xxHash.xxHash32.ComputeHash(span, span.Length, (uint)Name.GetHashCode());
    }
}

static void Main(string[] args)
{
    const int count = 100_000;
    var dic = new Dictionary<NumericValue, int>(count);
    var sp = Stopwatch.StartNew();
    for (int i = 0; i < count; i++)
    {
        dic.Add(new NumericValue
        {
            Name = "test",
            Value = i
        }, i);
    }
    Console.WriteLine(sp.ElapsedMilliseconds);
}

} ~~~ About 600~750 times faster on my machine.

The issue, I think, is the undefined variable l in the else if. It should be i.