12
\$\begingroup\$

I use the following structure in a web service where performance is critical. It is used for making joins between domain data objects, the key is made of two integers. The idea is to fit the two 32 bits integer in a long (64 bits).

Before using that structure, I used Tuple<int, int> which was really slower.

Is there any additional optimization to be made? (e.g.: addtional override, unneeded cast etc...)

public struct CombinedKey : IEquatable<CombinedKey>
{
   private readonly long value;

   public CombinedKey(int item1, int item2)
   {
        unchecked
        {
            value = (long)item1 << 32 | (uint)item2;
        }
    }

    public override int GetHashCode()
    {
        return (int)value ^ (int)(value >> 32);
    }

    public bool Equals(CombinedKey other)
    {
        return this.value == other.value;
    }

    public int Item1
    {
        get
        {
            return (int)(this.value >> 32);
        }
    }

    public int Item2
    {
        get
        {
            return (int)(this.value & 0xffffffff);
        }
    }
}
\$\endgroup\$
7
  • 4
    \$\begingroup\$ Why do you not just store item1 and item2 as two fields? \$\endgroup\$
    – David Arno
    Commented Sep 11, 2015 at 11:33
  • 1
    \$\begingroup\$ What is what you need more from this? The combined key or each individually? If you don't need the combined key often, then don't make the conversion until it's needed, and save them as int, int. The other option is saving both ints and the combined key and return what is needed. The thing is to stop the conversion from int to long, and long to int as much as possible \$\endgroup\$ Commented Sep 11, 2015 at 12:38
  • \$\begingroup\$ @fernando.reyes : the only requirement is to implement IEquatable. There is no need to retrieve the keys individually after (Eg : by doing combinedKey.Item1). There is two getters in the code example because there was a ToString() method that i removed that used them initially. I had in mind to put the two items in a long instead of two separate int fields to perform the equals operation in one instruction on 64 bits systems instead of two tests separated by logical AND. Unfortunately in all cases the GetHashCode need to be done in two steps. \$\endgroup\$
    – tigrou
    Commented Sep 11, 2015 at 14:42
  • \$\begingroup\$ Why don't you just use long? \$\endgroup\$
    – tia
    Commented Sep 11, 2015 at 16:00
  • \$\begingroup\$ @tia : I would then need to take care combining the two int's to one long in all places I need this, it's easier to encapsulate this operation in a single place, in the structure constructor. It's also better for readability : if someone see long in code it can mean lot of things, CombinedKey is obvious. \$\endgroup\$
    – tigrou
    Commented Sep 12, 2015 at 8:47

1 Answer 1

16
\$\begingroup\$

You could use explicit layout of the structure to omit values calculation.
It can act similar to the union in C/C++:

[StructLayout(LayoutKind.Explicit)]
public struct CombinedKey : IEquatable<CombinedKey>
{
    [FieldOffset(0)]
    private readonly long value;
    [FieldOffset(0)]
    public readonly int Item1;
    [FieldOffset(sizeof(int))]
    public readonly int Item2;

    public CombinedKey(int item1, int item2)
    {
        value = 0;  // We need to init all the fields
        Item1 = item1;
        Item2 = item2;
    }

    public bool Equals(CombinedKey other)
    {
        return value == other.value;
    }

    public override int GetHashCode()
    {
        return Item1 ^ Item2;
    }
}

You could also wrap readonly fields into get-only properties.


Additional link: [StructLayout attribute magic][2]
\$\endgroup\$
5
  • \$\begingroup\$ @Dmitry : Thanks for this. I tested performance of your solution against the original one. It is very similar if not identical. However I prefer your solution as it is shorter. \$\endgroup\$
    – tigrou
    Commented Sep 11, 2015 at 13:27
  • \$\begingroup\$ I needed a while to get what's going on here and read the links a few times but now I find it's a pretty cool trick :-) I also think it would be easier to understand if there was sizeof(int) instead of the 4 like in the second link. \$\endgroup\$
    – t3chb0t
    Commented Sep 11, 2015 at 19:47
  • 1
    \$\begingroup\$ Slick. I'll be back to award a bounty for teaching me something new. ++ \$\endgroup\$
    – RubberDuck
    Commented Sep 12, 2015 at 11:49
  • \$\begingroup\$ IEquatable<T> Reminder from MSDN: "If you implement IEquatable<T>, you should also override the base class implementations of Object.Equals(Object) and GetHashCode so that their behavior is consistent with that of the IEquatable<T>.Equals method." \$\endgroup\$
    – radarbob
    Commented Sep 13, 2015 at 22:40
  • 1
    \$\begingroup\$ I'd rather avoid low level hacks like this. For example one issue with this is that the generated long depends on host endianness. \$\endgroup\$ Commented Sep 14, 2015 at 8:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.