Stop using MEF as part of the public API

[TomFinley]

We have received feedback that we should limit our usage of MEF as part of the public API, according to feedback by .NET team. We currently for the sake of writing "custom" transformers have the user interact with it. This can be seen here.

machinelearning/test/Microsoft.ML.Tests/Scenarios/Api/CookbookSamples/CookbookSamplesDynamicApi.cs

Line 535 in 2d351eb

newContext.CompositionContainer = new CompositionContainer(new TypeCatalog(typeof(CustomMappings)));

As we see, we are hsing on MLContext MEF containers and catalogs, using types exposing MEF Import and Export attributes, etc. etc. The feedback from .NET team is, stop, do something else, because MEF is apparently not "clean" enough.

(What is to be done instead? Possibly registering some "named" delegate to produce these things, or other. But anyway, we should not use MEF directly. Maybe not use attributes at all, and just require explicit registration somehow. I'm not sure.)

@eerhardt and others may have more specific recommendations or context on why we should not use MEF in this way.

[CESARDELATORRE]

Yeah, I remember I recommended NOT to use MEF in one of the calls last year when it was suggested to use MEF in ML.NET... but what's done is done, so let's fix it.

Also, there's the UWP issue relates to MEF:
#1736

If we need DI (Dependency Injection), we could use the light IoC container provided by .NET:

https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection

It is used by default in ASP.NET Core:

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2

Importantly, because this DI support is not integrated in to ASP.NET Core as an intrinsic feature but rather implemented as separate NuGet packages, they can be included in other .NET Core projects, not just ASP.NET Core. Even more importantly, these packages support .NET Framework 4.5.1 in addition to .NET Core.

https://scottdorman.github.io/2016/03/17/integrating-asp.net-core-dependency-injection-in-mvc-4/

So it'd be worth to explore this option (if we need DI), since this is the official DI/IoC framework provided by the .NET team.

Other third party library that supports all .NET frameworks and very much used in the .NET community is AutoFac:
https://autofac.org/

[sharwell]

What exactly is the concern here? MEF is well-supported and lightweight on both .NET Framework and .NET Core, and has exceptional performance for test scenarios.

[CESARDELATORRE]

Related to this:
UWP issue related to MEF:
#1736

And the fact is that MEF is pretty old and not an investment/strategy in the .NET team heading the future.

[sharwell]

UWP issue related to MEF:
#1736

This issue doesn't mention MEF, and I don't see an Emit dependency in System.Composition. Are you sure these are the same issue?

If we need DI (Dependency Injection), we could use the light IoC container provided by .NET:

https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection

It is used by default in ASP.NET Core

I'm having trouble finding documentation regarding the use of this package, in particular an overview of features and a getting started guide for people coming from MEF.

Other third party library that supports all .NET frameworks and very much used in the .NET community is AutoFac:

Given we have built-in solutions that work well, I don't see a particular value in moving in a direction like this. This is not at all a statement against alternatives, merely a suggestion that we not take an unnecessary dependency at this level in the application hierarchy.

[CESARDELATORRE]

@sharwell - That is the "standard" way to go for DI in ASP.NET Core, documented here, see:

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2

About the specific issues in ML.NET with MEF, @eerhardt can comment further.
Let's take this discussion offline, ok? :)

[CESARDELATORRE]

The other option is to really confirm if we really need DI/IoC for our scenario in ML.NET.
Eric, Tom, you and the team can confirm this point.

[sharwell]

The whole thread feels very "seat of the pants" to me. I believe we should step back and evaluate the needs of the project and be clear (and objective) regarding the advantages and disadvantages of available approaches. I have absolutely no objection to this discussion, but on the available information I would not consider this proposal complete or ready for adoption.

It may very well be the case that ML doesn't require any service location infrastructure, in which case this proposal simplifies to just removing the current use of MEF and not replacing it with any particular alternative library.

[CESARDELATORRE]

I'd agree with your last paragraph if that is the case (if we don't require any service location infrastructure).
I'm just saying that IF we need a DI/IoC infrastructure, I recommend to use microsoft.extensions.dependencyinjection which is mainstream in .NET Core, ASP.NET Core and also works on .NET Framework, as mentioned in my links above.

[AArnott]

I'm not too familiar with microsoft.extensions.dependencyinjection, but as a typical app author, I wouldn't want a library to force me to use that package as my IoC container any more than I'd want it to force me to use MEFv1.

I don't know why we'd have MEFv1, MEFv2, and microsoft.extensions.dependencyinjection and trust that the 3rd is going to be smiled upon for longer than the prior two MEF implementations anyway. These things sadly tend to come and go, which is one big reason why I support removing MEF from the public API, and leaving any IoC integration to the library's user.

But to add to @CESARDELATORRE's list of IoC containers, there's also vs-mef which is faster than MEFv1 and (for many cases) for MEFv2, it doesn't rely on Ref.Emit, and supports caching for super-fast startup time. Not that that changes my above recommendation however.

[eerhardt]

FYI - the issue tracking ML.NET doesn’t work on UWP because of MEF is #1595. And the underlying reason is because of https://github.com/dotnet/corefx/issues/33434

[CESARDELATORRE]

@AArnott - Not sure what you mean with "I wouldn't want a library to force me to use that package as my IoC container".

In this case, the DI library would be used internally by ML.NET. It'd be transparent for the user/developer.
It is like if ML.NET is using internally any other .NET library or BCL class from Microsoft.

ML.NET won't force any developer to use that package (microsoft.extensions.dependencyinjection) as your app's IoC container, of course.

[AArnott]

@CESARDELATORRE I'm talking about the "public API" part that I understand as the point of this issue. I wouldn't want any library to require that I participate in an IoC container of their choosing as part of my larger app composition. If the library uses one strictly internally, I don't care (provided it doesn't hurt perf).

[CESARDELATORRE]

@AArnott - Sure, then this case doesn't apply for that concern. The fact that we might use or not a DI library it would be for internal usage in the ML.NET framework, nothing related to the user's code for his larger end-user app composition.

[neven10]

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2

Importantly, because this DI support is not integrated in to ASP.NET Core as an intrinsic feature but rather implemented as separate NuGet packages, they can be included in other .NET Core projects, not just ASP.NET Core. Even more importantly, these packages support .NET Framework 4.5.1 in addition to .NET Core.

In Net Core 3 preview they are dropping some packages.
See : dotnet/aspnetcore#3756

"The API for these packages will still be available, and will be part of Microsoft.AspNetCore.App.
Projects which use these API should replace the <PackageReference> with a <FrameworkReference> to Microsoft.AspNetCore.App."

There is no Microsoft.Extensions.Dependency package for Net Core 3.
See:
https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection?view=aspnetcore-2.2&viewFallbackFrom=aspnetcore-3.0

Will this be an issue?

[eerhardt]

Will this be an issue?

It shouldn't be an issue because I don't think we should use the Microsoft.Extensions.DependencyInjection library to remove ML.NET's dependency on MEF.

But this information gives that decision even more weight.

[CESARDELATORRE]

@neven10 - Not sure if I misunderstood you, but microsoft.extensions.dependencyinjection will continue being in a separate package in .NET Core 3.0 timeframe, so you can reference and use it from ASP.NET Core, any .NET Core app or .NET Framework:

See it confirmed here by the team:

dotnet/aspnetcore#3756 (comment)

That doesn't mean that we'll use it or not in ML.NET since Eric commented that we might not need microsoft.extensions.dependencyinjection for the intended features.

Just saying that microsoft.extensions.dependencyinjection will keep being an independent package, not part of ASP.NET Core 3.0, so it is usable for any other application type.

[neven10]

@CESARDELATORRE
Nevermind. I was looking at documentation for 3.0 .
Didn't even bother to look on Nuget.
It's there Microsoft.Extensions.DependencyInjection -Version 3.0.0-preview.19074.2
Sorry.