Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate DI from Microsoft.VisualStudio.Composition to Microsoft.Extensions.DependencyInjection #3308

Merged

Conversation

tom-englert
Copy link
Contributor

This PR only migrates the DI engine

Migrating the Export attributes to Microsoft.Extensions.DependencyInjection.Abstractions will be done in a separate PR, to not overload this PR.

@tom-englert tom-englert force-pushed the dev/DependencyInjection branch 2 times, most recently from d969989 to fd8dc53 Compare October 19, 2024 16:51
@siegfriedpammer
Copy link
Member

Can you provide some explanation why the DI engine needs to be changed?

@siegfriedpammer siegfriedpammer self-assigned this Oct 19, 2024
@tom-englert
Copy link
Contributor Author

Mainly to get rid of technical debt.
Microsoft has abandoned Mef1 and Mef2, and replaced it with the new, lightweight and faster DependencyInjection framework that is now used in every modern DotNet application.
Same reason why we use DotNet8, even though NetFramework is still working fine...

@christophwille
Copy link
Member

christophwille commented Oct 20, 2024

Didn't we very much intentionally move from MEF to VS MEF?

Edit: yes, we moved for performance reasons. https://github.com/microsoft/vs-mef

@tom-englert
Copy link
Contributor Author

DependencyInjection framework should be even more slim and faster

@tom-englert
Copy link
Contributor Author

tom-englert commented Oct 20, 2024

https://github.com/danielpalme/IocPerformance
image

So the performance improvement of VSMef is only when compared to MEF1!

@christophwille
Copy link
Member

I concur that the original reason may no longer apply (and we are a small extensibility provider anyways).

Btw. is there a way to reduce the "package sprawl"? I mean some of the Toms.* are super-small and we now (w transitive dependencies) include quite a few of those.

@tom-englert
Copy link
Contributor Author

I had no idea how, beside duplicating all of the source code.
However getting rid of three heavy ones in exchange for five small ones isn't a too bad deal, isn't it?

@tom-englert
Copy link
Contributor Author

image

It's even one less after final migration (left) compared to 8.2 (right)!

…the lightweight System.Composition.AttributedModel
@siegfriedpammer siegfriedpammer merged commit c3261a3 into icsharpcode:master Oct 24, 2024
5 checks passed
@siegfriedpammer
Copy link
Member

Thank you very much!

@christophwille
Copy link
Member

@tom-englert @siegfriedpammer we plan to release Preview 3 on ~4th November evening. Any big ticket items missing on your end?

@tom-englert
Copy link
Contributor Author

Just cleaning up the use of singletons, and replace them by DI services. This is just architectural cleanup, nothing that is required for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants