Slow model loading performance after version 3.0.1

[ogo-adp]

System Information

• OS & Version: Windows 10, Windows 2016
• ML.NET Version: ML.NET > 3.01 (including 5.0 preview)
• .NET Version: .NET 8.0

Describe the bug
We use ML.NET on a large application with approx 20 models using only ML.NET FastTree regression.
For all ML.NET versions after 3.0.1 the time to load a model from a MLContext has increased dramatically

To Reproduce
Create a regression model using FastTree
Load the model from a MLContext with version 3.0.1. Note the loading time
Load the same model with version 4.0.2. The loading time should be much higher
We observed between 7 and 50 times increased in loading times

Expected behavior
The loading time should not be different

Screenshots, Code, Sample Projects
var mlContext = new MLContext(seed: 0);
var loadedModel = mlContext.Model.Load("path_of_model.zip", out _);
-> loading time 111ms with 3.0.1, 1100ms with 4.0.2

Additional contextA
Loading a model produced with version 4.0.2 using ML.NET 3.0.1 is fast
Loading a model produced with version 4.0.2 using ML.NET 4.0.2 is slow
Our model zip file range from 1000ko to 3000ko

[ogo-adp]

I've found the cause of the problem. Starting with version 4.0.0 ML.NET create a temporary directory in %appdata%\local\temp named "ml_dotnet" to load the model file. It does never delete this directory, and they pile up ...

On one of my servers there were near 200 000 "ml_dotnet" directories hence the problem. Once deleted the problme disappear (but will reappear after some time).

Version 3.0.1 did not create any temporary directory.

Culprit commit seems to be this

Especially this awkward code :
int dirNumber = 0;
string mlNetTempDir = null!;
while (Directory.Exists(mlNetTempDir = Path.Combine(Path.GetFullPath(tempPath), $"ml_dotnet{dirNumber++}"))) ;
var path = Path.Combine(mlNetTempDir, Path.GetRandomFileName());

In my opinion this is a huge bug with serious consequences on production servers using ML.NET. Would it be possible to release a 4.0.3 version that include a fix ?

Best regards,
Olivier

[ssanders82]

I am seeing this as well. Incredible. I've got 39,710 folders called ml_dotnetxxxxx in my local appdata

[ericstj]

So those are created by b4a4b10#diff-336b6a717eb7a0527b3b166736f1fdb4129a6fc101b2be0f3e5cd6278adda263

Previously it created a single directory, now it creates many, and the algorithm for finding a number is linear with the number of directories:

machinelearning/src/Microsoft.ML.Core/Data/Repository.cs

Line 128 in 439a052

while (Directory.Exists(mlNetTempDir = Path.Combine(Path.GetFullPath(tempPath), $"ml_dotnet{dirNumber++}"))) ;

That should be fixed. That's what caused this regression - perf will degrade over time if the directories are not cleaned up.

Still it's not OK to leave them behind - that may be an existing issue in your codebase. It does look like the Repository which creates this has a finalizer that will clean up the directory:

machinelearning/src/Microsoft.ML.Core/Data/Repository.cs

Line 168 in b28b6d4

Directory.Delete(DirTemp, true);

I don't see anywhere we leak Repository in ML.NET. Can you check if you have uses of ModelSaveContext that are missing a using, or rooted by a static? Alternatively -- if you happen to have a workflow that terminates the process it could also cause this.

So let's let this issue track fixing that algorithm for temp directory creation to not degrade linearly with number of directories, but I think it would also be good to see how this is being leaked in your application (or ML.NET - in case I'm missing it). Worse-case if you can't root out the leak, you can give your process a unique MLContext.TempFilePath and clean it up after.

To fix the temp directory creation we need a different approach than #7173. Presumably temp is either unique per user or has a permissive ACL if it's shared. Rather than create with an increment (current fix) or use a shared ml_dotnet folder with a random unique subfolder (previous behavior - has user process creating shared directory) we should be using a single subfolder that is randomly generated, and retry on collision. This way there is no user created shared folder (original issue) and the algorithm for generating new folders is more efficient.

[ogo-adp]

Hi @ericstj and thanks for your detailed response.

However I do not see any way to properly Dispose() resources that will remove those directories.

This simple "predict only" code will leak a directory even if calling Dispose() everywhere it is possible :

var mlContext = new MLContext(seed: 0);
var loadedModel = mlContext.Model.Load("model.zip", out _);
using (var predictionEngine = mlContext.Model.CreatePredictionEngine<X, Y>(loadedModel))
{
predictionEngine.Predict(some X instance);
}

Even calling GC.Collect() and GC.WaitForPendingFinalizers() will not remove anything after that.

I did not see version 3.0.1 create any temporary file.

[ericstj]

I made the fix that will address the regression in #7520.

This simple "predict only" code will leak a directory even if calling Dispose() everywhere it is possible

The temporary directory should be deleted when exiting this using block --

machinelearning/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs

Lines 156 to 194 in 7b60281

	using (var rep = RepositoryReader.Open(stream, _env))
	{
	var entry = rep.OpenEntryOrNull(SchemaEntryName);
	if (entry != null)
	{
	var loader = new BinaryLoader(_env, new BinaryLoader.Arguments(), entry.Stream);
	inputSchema = loader.Schema;
	ModelLoadContext.LoadModel<ITransformer, SignatureLoadModel>(_env, out var transformerChain, rep,
	CompositeDataLoader<object, ITransformer>.TransformerDirectory);
	return transformerChain;
	}
	ModelLoadContext.LoadModelOrNull<IDataLoader<IMultiStreamSource>, SignatureLoadModel>(_env, out var dataLoader, rep, null);
	if (dataLoader == null)
	{
	// Try to see if the model was saved without a loader or a schema.
	if (ModelLoadContext.LoadModelOrNull<ITransformer, SignatureLoadModel>(_env, out var transformerChain, rep,
	CompositeDataLoader<object, ITransformer>.TransformerDirectory))
	{
	inputSchema = null;
	return transformerChain;
	}
	// Try to load from legacy model format.
	try
	{
	var loader = ModelFileUtils.LoadLoader(_env, rep, new MultiFileSource(null), false);
	inputSchema = loader.Schema;
	return TransformerChain.LoadFromLegacy(_env, stream);
	}
	catch (Exception ex)
	{
	throw _env.Except(ex, "Could not load legacy format model");
	}
	}
	var transformer = DecomposeLoader(ref dataLoader);
	inputSchema = dataLoader.GetOutputSchema();
	return transformer;
	}

Previously even a successful run to leave a directory behind, because it created both an ml_dotnet<integer> directory and a random subdirectory, but then only deleted the subdirectory. So I agree - even on success this was leaving directories behind.

To workaround this bug, you can use:

var mlContext = new MLContext(seed: 0)
{
 // Choose a unique file path for temp
 TempFilePath = Path.Combine(Path.GetTempPath(), "myTempFilePath")
};

try
{
 var loadedModel = mlContext.Model.Load("model.zip", out _);
 // other work using the model
}
finally
{
 try
 {
 Directory.Delete(mlContext.TempFilePath, true);
 }
 catch
 {
 Console.WriteLine("Failed to delete temporary directory");
 }
}

I will port the fix for this back to 4.0 as well.