Disposing/Releasing GPU memory

[zsogitbe]

The memory was not disposed when Dispose() was called because ReleaseHandle(); was not called by the base class.

[zsogitbe]

I have no idea why this fails with the tests (nothing is changed there). It is OK on my computer.

[martindevans]

I've restarted the tests, sometimes they're flakey (although usually only the osx ones).

[martindevans]

From some research I don't think calling ReleaseHandle directly is necessary.

The docs show an example of a file handle, which does not explicitly implement Dispose and does not explicity call ReleaseHandle:

internal class MySafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
    {
        // Create a SafeHandle, informing the base class
        // that this SafeHandle instance "owns" the handle,
        // and therefore SafeHandle should call
        // our ReleaseHandle method when the SafeHandle
        // is no longer in use.
        private MySafeFileHandle()
            : base(true)
        {
        }
        [ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)]
        override protected bool ReleaseHandle()
        {
            return NativeMethods.CloseHandle(handle);
        }
    }

Pulling out an important comment:

...SafeHandle should call our ReleaseHandle method when the SafeHandle is no longer in use.

Additionally I just tried putting a breakpoint on SafeLlamaModelHandle.ReleaseHandle and debugging the AdvancedModelProperties test. This breakpoint is hit after the test has run.

[zsogitbe]

I needed this in my code because GPU memory was not released when I was calling Dispose(). After checking the code it was obvious that the memory was not released because there was no Dispose on the child class (and ReleaseHandle was not called).

I guess that someone counted on the fact in the tests the memory was not released after calling Dispose(), but this is the wrong way of working.

[martindevans]

I've just added some more tests (this PR: #551) asserting that memory is freed correctly.

Try putting breakpoints into SafeLLamaModelHandle.ReleaseHandle and debugging those tests. You'll see it gets called as soon as Dispose is called without needing to call it directly. As I understand it that's the entire point of a SafeHandle!

Note the second tests introduced there shows one possible case you may not expect. if you do this:

1. Load model weights
2. Create context using those weights
3. Dispose model weights
4. Nothing happens, weights are still loaded
5. Dispose context
6. Now the weights will be disposed (and the context)

That is, the context keeps model weights in memory even after disposal is called! If you're failing to free a context that will cause an apparent leak.

[zsogitbe]

I have tested your suggestion, but the GPU memory is not freed. The GPU memory is only freed if llama_free_model(handle); is called. And llama_free_model(handle); is only called if ReleaseHandle() is called. But in your safehandle implementation the Dispose() was forgotten and ReleaseHandle() is never called (only if the app exits, but that is too late for the second inference run...).

Maybe your explanation is OK if you use CPU, but it is definitely not OK if you use GPU.

[zsogitbe]

I have found a problem with my Dispose solution also on my computer. The Relleasehadle gets called also automatically after several minutes, but then it was already released by my code, so it crashes.

I do not have a good solution yet for a loading/inference/unloading/loading/inference run. If I do not Dispose it manually, then I cannot load it again because the GPU memory is full. I have tried GC.Collect() also. Any ideas?

[zsogitbe]

I have found the problem. There was a KernelMemory connected to it and it kept it in memory... need to dispose the KernelMemory also. I will remove this pull requested after you confirmed receiving my message.

[martindevans]

Sounds like you figured it out 👍

[zsogitbe]

Yes, sorry for the waste of time.

[martindevans]

Not a problem! In fact thank you for coming and contributing fixes to the project. Please don't let this stop you making PRs in the future :)

[zsogitbe]

I was not able to dispose KernelMemory connected to the model (I had to remove the code). Maybe if you have some time, then please add a memory release test where KernelMemory is also added to the model and try to dispose the model.