Search code examples
c#unity-game-engine

How to reduce memory allocation for data types whose lifetime is undefined


I have a data type whose lifetime is undefined. Therefore it is a class. However, it is created very often, which causes it to constantly allocate memory. It also has quite a bit of weight, which makes it problematic.

What can be done to reduce the number of allocations required by this data type, optimize its allocations and deallocations, and reduce GC pressure? (Making a structure would not work, as it would require constant copying of data, with the impossibility of always being able to release data from the internal array via try-finally).

[Serializable]
public class DamageData
{
    [Serializable]
    public class DamageDataInternal
    {
        public float[] values;
        public float totalDamage;
        public int cachedHashCode;
        public bool isHashCodeDirty;
    }

    private DamageDataInternal _internalData;
    
    public static readonly DamageType[] DamageTypes;

    public int Hash => GetHashCode();
    public bool IsInit => _internalData != null;

    static DamageData()
    {
        DamageTypes = (DamageType[])Enum.GetValues(typeof(DamageType));
    }

    ~DamageData()
    {
        DamageDataInternalPool.Shared.Release(_internalData);
        _internalData = null;
    }

    public DamageData()
    {
        _internalData = DamageDataInternalPool.Shared.Get();
        _internalData.totalDamage = 0f;
        _internalData.isHashCodeDirty = true;
        _internalData.cachedHashCode = 0;
    }
}
using UnityEngine.Pool;
public class DamageDataInternalPool
{
    private readonly ObjectPool<DamageData.DamageDataInternal> _pool;

    public static DamageDataInternalPool Shared { get; } = new();

    public DamageDataInternalPool()
    {
        _pool = new ObjectPool<DamageData.DamageDataInternal>(
            createFunc: () => new DamageData.DamageDataInternal(),
            actionOnGet: obj =>
            {
                obj.totalDamage = default;
                obj.cachedHashCode = default;
                obj.isHashCodeDirty = default;
                obj.values = ArrayPool<float>.Shared.Rent(DamageData.DamageTypes.Length);
                for (var i = 0; i < DamageData.DamageTypes.Length; i++)
                {
                    obj.values[i] = 0f;
                }
            },
            actionOnRelease: obj =>
            {
                ArrayPool<float>.Shared.Return(obj.values);  
            },
            actionOnDestroy: null,
            defaultCapacity: 10,
            maxSize: 10000
        );
    }

    public DamageData.DamageDataInternal Get()
    {
        return _pool.Get();
    }

    public void Release(DamageData.DamageDataInternal obj)
    {
        _pool.Release(obj);
    }
}

I tried to encapsulate its internal data in a separate class, instances of which I get via object-pooling, so that the main class should weigh only SIZE_OF_LINK + SOME_META_DATA_SIZE. However, this doesn't seem to have changed the situation at all and there is still a lot of allocation.


Solution

  • You may be focusing too much on the precise size of each GC allocation, but it might be a better idea to minimize the number of GC allocations and overall complexity of your code. Since you are working in C# 8, don't actually need to serialize your DamageData type, and are open to writing unsafe code, I am going to suggest an alternate, simpler design: you could replace your DamageDataInternal class with an unsafe fixed int values[DamageTypeLength]; field in some private struct field. With this design DamageData will be somewhat larger than your ideal solution, but it will still be quite small while eliminating the need to allocate and dispose of any additional memory, whether pooled or not.

    Let's say your DamageType looks something like this. First add some compile-time constant indicating its length:

    public enum DamageType { Default = 0, Rupture = 1, Kinetic = 2, Thermal = 3, Cryogenic = 4, Explosive = 5, Chemical = 6, Light = 7, Radiation = 8, Electric = 9, };
    
    public static class DamageTypeConstants
    {
        static DamageTypeConstants()
        {
            DamageTypes = (DamageType[])Enum.GetValues(typeof(DamageType));
            Debug.Assert(DamageTypes.All(d => (int)d >= 0 && (int)d < DamageTypeConstants.DamageTypeLength));
            Debug.Assert(DamageTypes.Length == DamageTypeConstants.DamageTypeLength);
        }
    
        // KEEP THIS IN SYNC WITH THE MAX VALUE IN THE DamageType enum!!!
        public const int DamageTypeLength = 1 + (int)DamageType.Electric; 
        public static readonly DamageType[] DamageTypes; // I moved this into the "Constants" class
    }
    

    Then reimplement your DamageData and DamageDataInternal as follows, making DamageDataInternal a private struct field containing a fixed size buffer:

    public class DamageData
    {
        private DamageDataStruct damageDataStruct = new();
    
        public int this[DamageType damageType]
        {
            get => damageDataStruct[damageType];
            set => damageDataStruct[damageType] = value;
        }
        
        public int TotalDamage => damageDataStruct.TotalDamage;
    }
    
    public unsafe struct DamageDataStruct
    {
        int totalDamage;
        unsafe fixed int values[DamageTypeConstants.DamageTypeLength];
    
        public int TotalDamage => totalDamage;
    
        public int this[DamageType damageType]
        {
            get
            {
                var index = (int)damageType;
                if (index < 0 || index >= DamageTypeConstants.DamageTypeLength)
                    throw new IndexOutOfRangeException(nameof(damageType));
                return values[index];
            }
            set
            {
                var index = (int)damageType;
                if (index < 0 || index >= DamageTypeConstants.DamageTypeLength)
                    throw new IndexOutOfRangeException(nameof(damageType));
                var oldValue = values[index];
                totalDamage += (value - oldValue);
                values[index] = value;
            }
        }
    }
    

    I doubt (but do not know for certain) that fixed size buffer fields can be serialized by Unity. You mentioned in comments that I have another type that is the initializer for DamageData. If so, it would need to look something like the following DTO:

    [Serializable]
    public class DamageDataDTO
    {
        public int [] values;
        
        public DamageDataDTO(DamageData data)
        {
            values = new int[DamageTypeConstants.DamageTypeLength];
            for (int i = 0; i < values.Length; i++)
                values[i] = data[(DamageType)i];
        }
        
        public DamageData ToDamageData()
        {
            var data = new DamageData();
            if (values == null)
                return data;
            for (int i = 0, count = Math.Min(values.Length, DamageTypeConstants.DamageTypeLength); i < count; i++)
                data[(DamageType)i] = values[i];
            return data;
        }
    }
    

    To construct a DamageData with some specific damage you could use an initializer like so:

    var damageType = DamageType.Explosive;
    var value = 10;
    var damageData = new DamageData
    {
        [damageType] = value,
    };
    

    Notes:

    • Your existing DamageData class keeps most of its data in a pooled DamageDataInternal type which uses a rented float [] array. This approach does not decrease the number of GC allocations (you still allocate a DamageData every time), it only decreases the size of each GC allocation, at the cost of pool allocations.

    • You wrote, Making a structure would not work, as it would require constant copying of data. As long as the damageDataInternal field is private and you never return it via some property or method, this will not be a problem in practice as you will never actually copy it.

    • The size of the DamageDataInternal struct will be DamageTypeLength integers, plus an extra integer for the cached total value. So 11 integers total = 44 bytes (more likely 48 bytes with padding).

    • As the DamageDataInternal struct is embedded in the DamageData class, the size of this class will be the size of DamageDataInternal plus whatever else you store there, such as a cached hash code.

    • Your existing DamageData class uses a finalizer to release the pooled DamageDataInternal and rented float[] array. Classes with finalizers can negatively impact GC performance as explained in Do destructors affect performance?, and is not deterministic as is pointed out in comments by Guru Stron.

      The GC performance hit can be avoided by using the dispose pattern and calling GC.SuppressFinalize(this); from Dispose() -- however you stated that the lifetime of DamageData is undefined, so this does not seem workable, as you will never know when to call Dispose(). Replacing the pooled and rented values with smallish fixed size buffer that is never copied seems a reasonable alternative that eliminates the finalization performance penalty.

      If you had a much larger number of damage types, my design would be less attractive.

    • In your question, you don't show how DamageData is used. It may be possible to replace it with DamageDataStruct and avoid excess copying by using ref return properties and indexers to access it.

    • Fixed-size buffers may only be instance fields of structs, however those structs can be instance fields of classes as well as allocated directly on the stack.

    • In C# 12 you could eliminate the unsafe code by using an inline array as shown in this fiddle:

      public struct DamageDataStruct
      {
          int totalDamage;
          [System.Runtime.CompilerServices.InlineArray(DamageTypeConstants.DamageTypeLength)]
          struct Buffer { private int _element0; }
          Buffer values;
      
          public int TotalDamage => totalDamage;
      
          public int this[DamageType damageType]
          {
              get => values[(int)damageType];
              set
              {
                  var index = (int)damageType;
                  var oldValue = values[index];
                  totalDamage += (value - oldValue);
                  values[index] = value;
              }
          }
      }