[DRAFT] Featuring `MemoryManager` to resolve issues related to native memory leaking

[KillStr3aK]

Hey there, I propose this as a 'demo' currently. It works but not sure if this is the right implementation for this on the managed side, so I'm open for any ideas or extensions.

Currently every instance of Vector/QAngle/Angle creates a new pointer in unmanaged that is stored but never released, therefore it leaks memory which has been known since they were implemented.

Is it a breaking change?

The current implementation that involves IDisposableMemory/DisposableMemory should not be a breaking change as the types are still NativeObject, DisposableMemory is more like a proxy type to correctly release the resources.

How it works currently

When we access a value through the schema system, usually we instantiate the managed class and pass the pure game pointer to it so we can access the elements under the hood. Since we should not release these, I've added a PurePointer property that is only set to true when we explicitly mark the instance as pure.

Instances that are not marked as pure are should be safe to release, and we enforce these to be collected by the GC through the IMemoryManager that runs indefinitely (in a separate thread) by an interval that can be set in the CoreConfig. Once an instance is collected by the garbage collector, the finalizer will call the corresponding native to release it on the unmanaged side.

Since these are collected by the garbage collector itself, of course only the unused instances will be collected. Even if we don't use IMemoryManager (lets say we don't implement that part, or just disable it via CoreConfig) these will be collected, we just don't know when.

Anything in the schemasystem that is Span<T> (#134) OR NetworkedVector<T> (#135) (where T is Vector, QAngle, Quaternion, Vector2D, Vector4D, etc) cannot be used so currently the implementation for those are not that important (I kinda made it tho, but could not test)

Benchmark

I have no performance benchmarks in production with forcing GC yet, also we might change CoreConfig.MemoryManagerInterval to a viable interval, it was only used for tests.

However I tried to mass release instances and I could release 100.000 vector pointers in ~234ms without the server crashing, or disconnecting any players, however there were lags but I'd say it was related to the IO operations and the massive native calls.

Syntax adjustments

Since these types become IDisposable this syntax is now allowed:

using (Vector vector = new Vector(150.0f, 150.0f, 150.0f))
{
    // work with the vector here
}

// `vector` instance is now released automatically without GC

Releasing Vector instances:

// Instances like these are not supposed to be collected, and they won't be as long as its stored/not changed. Otherwise the GC will clean it up
public Vector ThisShouldNotBeGarbageCollected = new Vector();

MemAlloc

Also features the MemAlloc class with exposed natives.

public static class MemAlloc
{
    /// <summary>
    /// Indirect allocation using 'MemAlloc'
    /// </summary>
    /// <param name="size"></param>
    /// <returns></returns>
    public static nint Allocate(int size)
    {
        return NativeAPI.MemAllocAllocate(size);
    }

    /// <summary>
    /// Indirect allocation using 'MemAlloc'
    /// </summary>
    /// <param name="size"></param>
    /// <param name="align"></param>
    /// <returns></returns>
    public static nint AllocateAligned(int size, int align)
    {
        return NativeAPI.MemAllocAllocateAligned(size, align);
    }

    public static nint ReallocateAligned(nint pointer, int size, int align)
    {
        return NativeAPI.MemAllocReallocateAligned(pointer, size, align);
    }

    public static int GetSizeAligned(nint pointer)
    {
        return NativeAPI.MemAllocGetSizeAligned(pointer);
    }

    /// <summary>
    /// Release pointer using 'MemAlloc'
    /// </summary>
    /// <param name="ptr"></param>
    public static void Free(nint ptr)
    {
        NativeAPI.MemAllocFreePointer(ptr);
    }

    /// <summary>
    /// Release pointer using 'MemAlloc'
    /// </summary>
    /// <param name="ptr"></param>
    public static void FreeAligned(nint ptr)
    {
        NativeAPI.MemAllocFreePointerAligned(ptr);
    }
}

I will continue working on this, and once we get this one done we can implement CEntityKeyValues (#615)

From the minimal and limited tests I've done I can say that this whole system could work

[!NOTE]
I would not log the memory manager related stuff as 'INFO' but 'TRACE'

[oylsister]

Is this pretty much done or still have something to work on?

[KillStr3aK]

Is this pretty much done or still have something to work on?

I believe we should look for another way of doing this as the cleanup comes with overhead, but I'd assume that any change that would fix this issue would be a breaking one. I'd prefer a dev branch with breaking changes to give plugin developers more time to adjust their codebase than the current situation

[2vg]

I experimented with this in a private fork. Releasing vectors works well, but with something like BulletTracer, there might be overhead in releasing them after 64 people shoot at once. Since I use Satori for GC, it doesn't bother me much in my environment but I thought of something like a DisposableBag to reduce overhead, but I haven't tried it with a full player yet. With this bag, I think it should be possible to solve the problem by releasing Span<T> every frame as well... (On the Managed side, I just mark them, and release them all or little by little on the GameFrame hook...) Another idea for compatibility, I'm considering an implicit operator to minimize it. It won't be useful if we explicitly specify Span<T>, but if we declare it as a var, we should be able to convert it implicitly. :eyes: Anyway, thanks for the great work!

[KillStr3aK]

I experimented with this in a private fork. Releasing vectors works well, but with something like BulletTracer, there might be overhead in releasing them after 64 people shoot at once. Since I use Satori for GC, it doesn't bother me much in my environment but I thought of something like a DisposableBag to reduce overhead, but I haven't tried it with a full player yet. With this bag, I think it should be possible to solve the problem by releasing Span<T> every frame as well... (On the Managed side, I just mark them, and release them all or little by little on the GameFrame hook...) Another idea for compatibility, I'm considering an implicit operator to minimize it. It won't be useful if we explicitly specify Span<T>, but if we declare it as a var, we should be able to convert it implicitly. 👀 Anyway, thanks for the great work!

pretty sure we would need to get rid of the native call overhead to do this, thanks for the info!