Less GC from animations, much better performance

ilmari

Hi @ChrisIceBox, it's me again,whining about per frame allocations, which add unnecessary overhead.

We are using "AnimEngine_Legacy" for a number of our animations, and it seems that the animation related code is generating quite a lot of allocations every frame, and it becomes heavier the more animations there are in all characters of a given scene (not active anims, just existing ones). I got around 2kb extra allocations from that _per frame_, which is eating FPS heavily.

I investigated this with deep profiling, and noticed that actually pretty much all of the animation related allocations came from using clip.name (AnimationClip.name). That actually call Object.get_name(), which always allocates. For comparison anim.name (AnimationState.name) actually does not call Object.get_name(), and doesn't allocate.

Anyway, getting rid of the per frame clip.name allocations could be achieved in many ways by caching the strings. I actually considered several ways :

 - HashTables

 - String builders

 - Gstring (https://github.com/vexe/gstring)

 

I left out String builders/Gstring because they probably wouldnt get integrated into AC. I ended up simply using a Dictionary, because HashTables seem less handy to use, especially with <int, string>, and Dictionaries use Hashtables internally, making them very fast, but more useable.

clip.name is actually used quite a lot in the AC codebase, but I just concentrated on the stuff happening in one location, but on every frame: AnimEngine_Legacy.PlayStandardAnim().

I hacked the code sligtly..

ilmari

                

AnimationState animationState = animation[NonAllocAnimationClipName(clip)] 

 

//if (clip != null && animation[lastClipName] != null)

                if (clip != null && animationState != null)

                {

                    // if (!animation [lastClipName].enabled)

                    if (!animationState.enabled)

I then added a new method in the same class to provide remove the allocation overhead.

ilmari

 Here is the whole rest of the code, with added stuff for profiling

private void PlayStandardAnim (AnimationClip clip, bool doLoop, bool reverse)
{

if (character == null)
{
return;
}

UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim animation");
Animation animation = null;

if (character.spriteChild && character.spriteChild.GetComponent ())
{
animation = character.spriteChild.GetComponent ();
}
if (character.GetComponent ())
{
animation = character.GetComponent ();
}
UnityEngine.Profiling.Profiler.EndSample();

if (animation != null)
{
UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim NonAlloc animationName");
AnimationState animationState = animation[NonAllocAnimationClipName(clip)];
UnityEngine.Profiling.Profiler.EndSample();

UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim USE lastAnimationName");
//if (clip != null && animation[lastClipName] != null)
if (clip != null && animationState != null)
{
// if (!animation [lastClipName].enabled)
if (!animationState.enabled)
{
if (doLoop)
{
UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim RUN A1");
AdvGame.PlayAnimClip (animation, AdvGame.GetAnimLayerInt (AnimLayer.Base), clip, AnimationBlendMode.Blend, WrapMode.Loop, character.animCrossfadeSpeed, null, reverse);
UnityEngine.Profiling.Profiler.EndSample();
}
else
{
UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim RUN A2");
AdvGame.PlayAnimClip (animation, AdvGame.GetAnimLayerInt (AnimLayer.Base), clip, AnimationBlendMode.Blend, WrapMode.Once, character.animCrossfadeSpeed, null, reverse);
UnityEngine.Profiling.Profiler.EndSample();
}
}
}
else
{
if (doLoop)
{
UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim RUN B1");
AdvGame.PlayAnimClip (animation, AdvGame.GetAnimLayerInt (AnimLayer.Base), clip, AnimationBlendMode.Blend, WrapMode.Loop, character.animCrossfadeSpeed, null, reverse);
UnityEngine.Profiling.Profiler.EndSample();
}
else
{
UnityEngine.Profiling.Profiler.BeginSample("AC AnimEngine_Legacy.PlayStandardAnim RUN B2");
AdvGame.PlayAnimClip (animation, AdvGame.GetAnimLayerInt (AnimLayer.Base), clip, AnimationBlendMode.Blend, WrapMode.Once, character.animCrossfadeSpeed, null, reverse);
UnityEngine.Profiling.Profiler.EndSample();

}
}
// Add profiler
UnityEngine.Profiling.Profiler.EndSample();
}
}



// clip.name (AnimationClip.name) is actually calling Object.get_name(), which allocates garbage.
// If this happens in a loop on every frame for many animations, it can lead to horrific
// performance, especially on mobile. I've seen around 2kb allocations per FRAME,
// which is usually over 70% of all GC overhead! Note: AnimationState.name does not
// cause the same problem, it seems to be allocation safe.
public static string lastClipName = "";
public static bool showDebug = false;
public static bool replaceEmptyClipName = false;
public static Dictionary animationStateDict = new Dictionary();

public static string NonAllocAnimationClipName(AnimationClip clip){
if(clip == null){
return "";
}
int clipHash = clip.GetInstanceID();

if (clipHash >= 0) {
// Saving and fetching in the most efficien way. Dictionaries are built on fast HashTables,
// and TryGetValue() is the fastest way to get data out of it.
if (animationStateDict.TryGetValue (clipHash, out lastClipName)) {
if (lastClipName != "") {
return lastClipName;
} else if(replaceEmptyClipName) {
// Even on miss we can update empty entries, this should be a safe way. Probably.
lastClipName = clip.name;
animationStateDict[clipHash] = lastClipName;
return lastClipName;
}else{
return "";
}
} else {
// Only call this allocation causing clip.name when needed, normally once per animation clip.
lastClipName = clip.name;
animationStateDict.Add (clipHash, lastClipName);
if(showDebug){
Debug.Log ("ADDED NEW: clip.name= " + lastClipName + ", clipHash= " + clipHash);
}
return lastClipName;
}
}
return "";
}
}

}

ilmari

Sorry about the crappy formatting, it seems this forum doesnt really allow to copypaste stuff properly (very short limits on text length etc). I could send you my version of those files. I believe getting rid of this alloc would make many AC users happy.

NOTE: There are also unnecessary GetTranslation() allocations every frame, will post about that later.

ChrisIceBox

Thanks, I'll go over this and see what can be done.

For future reference, http://pasteall.org/ is a nice way of posting links to nicely-formatting code.

ChrisIceBox

Yes, good stuff here.  I agree that hashes are the best solution.  I'll look into removing the needless GetComponent calls as well.