Fixed leak in object pool

[harishsk]

I noticed this issue when running OneHotHashEncodingOnnxConversionTest for several iterations. Even after the Onnx related memory leaks were fixed, memory usage kept increasing at each iteration. When I took memory snapshots at the beginning of each iteration I noticed that about 1400 Single[] objects were being leaked (amounting to about 180MB).
The allocation of these objects can be traced to the object pool implemented using ConcurrentBag. It appears that the ConcurrentBag objects stay on in memory associated with the threads. (I haven't figured how these should be disposed off correctly yet). But removing the objects in the pool gets rid of those objects from the managed heap by the start of the second iteration and memory usage remains far more stable across iterations.

This also fixes memory leaks in my local x86 builds.

Since this is part of core ML.NET code that has been untouched for a long time, it is possible I may have overlooked something. Please review this.

[frank-dong-ms-zz]

[michaelgsharp]

Take a look at my comment and let me know what you think.

[michaelgsharp]

I believe the original code was a copy paste error. If you look at the code for Set, it has the same if check and the same _pool.Return... _pool.Return actually adds it to the Bag (bad method name? Not intuitive). So I think your fix maxes sense since get is actually removing it from the pool unlike return.

Right now since you are just doing a get, the resources are freed when the GC is run. Would it be better to do something like this?

while (_pool.Count > 0)
{
    var item = _pool.Get();
    (item as IDisposable)?.Dispose();
}

This way if it is disposable it will free it up right away. Not sure if that would impact performance much though.

What are your thoughts?

[harishsk]

We don't have many disposable objects. The disposable objects we consume are mostly from Tensorflow and OnnxRuntime. These aren't part of the object pool. If we add that line, I can't think of a test we can add that would exercise that code path.

---

In reply to: 534439391 [](ancestors = 534439391)