Performance issue of KeyValuePair.

Yoshitaka Kimisaki

Hello.

I found a performance issue in AnimationStateData.cs: https://github.com/EsotericSoftware/spine-runtimes/blob/master/spine-csharp/src/AnimationStateData.cs#L38

Comparison of KeyVakuePair is slow.
In this case, you should use a class like Tuple instead of KeyValuePair.

public struct AnimationStateDataPair : IEquatable<AnimationStateDataPair>
{
    public AnimationStateData From { get; private set; }
    public AnimationStateData To { get; private set; }

public AnimationStateDataPair(AnimationStateData from, AnimationStateData to)
{        
    From = from;
    To = to;
}

public override int GetHashCode()
{
    return From.GetHashCode() + To.GetHashCode();
}

public override bool Equals(object other)
{
    if (other is AnimationStateDataPair )
        return Equals((AnimationStateDataPair)other);
    
    return false;
}

public bool Equals(AnimationStateDataPair other)
{
    return ReferenceEquals(From, other.From) && ReferenceEquals(To, other.To);
}
}

Yoshitaka Kimisaki

Pharan

Thanks! Have you tried this on your project? I think we did something similar to the Skin dictionary but haven't switched it over to a struct yet. Should work fine there when we do.

Yoshitaka Kimisaki

Oh, I've just tested and checked it work on iOS(IL2CPP) and Android now.
Exactly, structs may cause "System.ExecutionEngineException: Attempting to JIT compile method" on iOS(Mono).

Pharan

a struct causes that exception? What about the struct you used in your recommended code above?

I don't think the GetHashCode implementation is correct here.
It will cause it to equate two pairs when one's From animation is the other's To animation.
Also has some misplaced types.

But this is noted we'll be making this change very soon since it's pretty straightforward.

Yoshitaka Kimisaki

There are two conditions to be caused this exception by structs.

Interface+Generics+Struct

public interface IMyInterface {
    void MyMethod<T>(T x);
}
 
public class MyClass: IMyInterface {
    public void MyMethod<T>(T) {
    }
}

IMyInterface instance = new MyClass();
instance.MyMethod(100);

Lambda+Generics+Struct

var act = new Action<T>(_ => { Debug.Log("foo"); });
act(default(T));

Usage of AnimationPair, I think it's not covered in the above two conditions.

Thank you for accepting this!

Pharan

That's interesting. I'll keep an eye out for that in the future, to avoid problems with iOS.

Also, I guess I was wrong about GetHashCode needing to be unique. But I got the implementation from Tuple.cs anyway.

I pushed a commit github a few hours ago. You can replace your code with this now: https://github.com/EsotericSoftware/spine-runtimes/commit/2a244bbbe0163d23562b0984b104a3b4031dbee0

The implementation also includes the prevention of boxing, based on knowledge from Dictionary.cs here. It seems like IEquatable is used when the Dictionary tries to get EqualityComparer<TKey>.Default and that creates a new comparer based on TKey's type. This indirection is bypassed by implementing our own EqualityComparer<TKey>
The same implementation was also already in Skin.cs.

I'm not sure if this implementation will cause the exception on iOS(Mono). The it doesn't have a generic method, but it does have a method identified by the qualified generic interface it's implementing.