dotnet / dotnet/machinelearning

PredictionEngine does not Dispose all resources properly

Open
#6,579 1 comment 0 reactions 1 assignee Claimed by @vejbomar View on GitHub
area-Core
Dominant language
C#
Stars
9.4k
Forks
2k
Avg merge
2d 20h
Merged PRs (30d)
11

Description

**System Information:**
- OS & Version: Windows 11
- ML.NET Version: ML.Net 2.0.1
- .NET Version: .NET 7.0

**Describe the bug**
`PredictionEngine.Dispose()` does not dispose of all resources, leaving hundreds of MB of memory behind.
Specifically, I suspect the `_outputRow` member in `PredictionEngineBase` is the culprit, as it is not disposed anywhere. Manually disposing of that using reflection hacks causes the expected memory to be free'd up.

**To Reproduce**
Unfortunately I can't give a code-based repro as I'm using a proprietary ONNX model. However I suspect it's reproducable with other models too.
Steps to reproduce the behavior:
1. Create a pipeline & predictionEngine - I'm using an object-detection ONNX model, being fed a 1280x720 image.
2. Call `predictionEngine.Predict()`
3. To ensure a clean slate, call `GC.Collect()` and `GC.WaitForPendingFinalizers()`.
4. Log memory used (560MB in my case)
5. Call `predictionEngine.Dispose()`, and ensure no references to the PredictionEngine exist any more
6. Call `GC.Collect()` & log memory (458MB in my case). This still seems very high. Furthermore:
8. Call `GC.WaitForPendingFinalizers();` & log memory (72MB in my case). This is now back to where you'd expect it, and suggests that `PredictionEngine` has left some disposables behind.

**Expected behavior**
I'd expect the memory usage after calling `predictionEngine.Dispose()` would drop significantly. I wouldn't expect anything to be left on the Finalizer queue.

**Additional context**
When I poked around in a memory profiler, it seemed like there were some OnnxRuntime `OrtValue` items left on the finalizer queue, and they _seem_ to be ultimately owned by the `PredictionEngineBase._outputRow` field.
So using a reflection hack, I disposed `_outputRow` myself and it appeared that all memory was being released:
```
var outputRow = typeof(PredictionEngineBase)
.GetField("_outputRow", BindingFlags.NonPublic | BindingFlags.Instance)
.GetValue(predictionEngine);

((IDisposable) outputRow).Dispose();
predictionEngine.Dispose();
```
Looking at https://github.com/dotnet/machinelearning/blob/main/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs#L129 it seems like `disposer` should get rid of both inputRow and outputRow.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.