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

User Story: Source generators are added to common libraries to improve performance #43545

Closed
Tracked by #5366
samsp-msft opened this issue Oct 16, 2020 · 34 comments
Closed
Tracked by #5366
Assignees
Labels
area-Meta Epic Groups multiple user stories. Can be grouped under a theme. Priority:2 Work that is important, but not critical for the release Team:Runtime untriaged New issue has not been triaged by the area owner

Comments

@samsp-msft
Copy link
Member

samsp-msft commented Oct 16, 2020

Source Generators are a new technology that was added to Roslyn in the .NET 5 timeframe, but we have not yet utilized them by creating generators to assist with runtime and framework features. We will work to define customer scenarios in more detail here with customer development.

Code that uses runtime reflection and reflection.emit causes issues for Trimming and Native AOT. Source generators can be used to generate alternative code at build time that is static so can be used in conjunction with Trimming and AOT. Work on trimming ASP.NET apps has revealed where there are current limitations that can be solved by source generators.

@samsp-msft samsp-msft added area-Meta User Story A single user-facing feature. Can be grouped under an epic. labels Oct 16, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 16, 2020
@YairHalberstadt
Copy link
Contributor

Trimming / Blazor / Native & Mono AOT – source generators can be used to convert code that does reflection.emit at runtime to source generators which will create code at build time rather than runtime. The generated code can then be analyzed by the illinker or native AOT enabling those scenarios.
Dependency Injection – is an important part of ASP.NET startup that could be replaceable with code generation at build time.

I have a project which does DI entirely at compile time via source generators: https://github.com/YairHalberstadt/stronginject

It's ideal for use cases which want to avoid any reflection, so great for trimming, blazor, and AOT. It's not a drop in replacement for Asp.Net DI, but maybe might serve as some inspiration.

There's plenty of other open source projects using source generators for things like serialization, and other reflection usages. I think it would be awesome if Microsoft could help with spreading the word about these projects, and possibly contributing, rather than creating competing alternatives!

Thanks for all the good work!

@samsp-msft samsp-msft changed the title User Story: Source generators continue to assist with runtime and framework features User Story: Improve framework trimming using source generators Oct 20, 2020
@danmoseley
Copy link
Member

Any community feedback on @samsp-msft prioritization above?

@samsp-msft samsp-msft changed the title User Story: Improve framework trimming using source generators User Story: Source generators are added to common libraries to enable apps using them to be trimmed. Oct 22, 2020
@richlander
Copy link
Member

I'd like to see the BCL and ASP.NET options prioritized separately. I don't think its useful to stack rank those together.

@cathysull cathysull added Priority:0 Work that we can't release without Priority:2 Work that is important, but not critical for the release and removed Priority:0 Work that we can't release without labels Oct 30, 2020
@danmoseley
Copy link
Member

@samsp-msft We should break these into separate user stories, probably one per source ge erator -- as they need to be costed, prioritized, owned separately. Can we do that?.

@samsp-msft
Copy link
Member Author

does it need a user story per generator (will become a bit heavy when it rolls up to the larger epic)? I would suggest converting the list in this to checkbox links off to individual task items, each of which can be costed / prioritized?

@danmoseley
Copy link
Member

@samsp-msft sure, seems fine to me to make child items, I think they should be User Stories. Generally these source generators will be user-visible experiences, eg., they have to enable them or do something to make them work.

@danmoseley
Copy link
Member

danmoseley commented Nov 4, 2020

Razor source generator is dotnet/aspnetcore#26902 in case anyone comes here looking for it. (But it's not related, as it's a build time optimization)

@Pilchie
Copy link
Member

Pilchie commented Nov 4, 2020

Just a note that the razor generator isn't a runtime scenario, and so doesn't contribute to trimming.

@danmoseley
Copy link
Member

@tommcdon your team owns the EventSource generator, if any. Can you help understand the priority?

@jaredpar System.Linq.Expressions could be listed here -- or perhaps in Blazor/Xamarin, it uses the interpreter mode? Can you evaluate this one.

On my side -- we need to consider whether there are other places that use Ref.Emit, especially that apply to Blazor/Xamarin. Assumption is that Native AOT scenario would benefit from those also.

@danmoseley
Copy link
Member

The priority of this is probably wrong - at least some will likely be P0.

@Pilchie
Copy link
Member

Pilchie commented Nov 4, 2020

Tagging @SteveSandersonMS here for the first item, can you take a look at what the set of places we might want to tackle within Blazor is?

@Pilchie
Copy link
Member

Pilchie commented Nov 4, 2020

Looking for using System.Reflection.Emit in https://github.com/dotnet/aspnetcore there are only 2:

  1. src/Http/Routing/src/Matching/ILEmitTrieFactory.cs
  2. src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs

@danmoseley
Copy link
Member

These are the uses of ref-emit in dotnet/runtime:

  • Microsoft.CSharp (for creating "VariantArray"s) -- owned by @jaredpar (#)
  • Microsoft.Extensions.DependencyInjection ("ILEmitResolver") -- covered above
  • System.ComponentModel.Composition (MEF) (#)
  • System.Diagnostics.DiagnosticSource (access private System.Net fields)
  • System.Linq.Expressions -- owned by @jaredpar
  • System.Private.DataContractSerialization -- owned by @HongGit
  • System.Private.Xml (Xml serializer) -- owned by @HongGit
  • System.Private.Xml (XSL compilation) (#)
  • System.Text.Json (Json serialization) ---- covered above
  • System.Text.RegularExpressions (compiled regex) ---- covered above

@HongGit @jaredpar FYI that if your team's technologies above are important for our target scenarios for AOT/trimming then we may need to consider creating a source generator that could be used to avoid the Ref.Emit.

(#) = I'm assuming these are not likely to be high priority.

cc @stephentoub

@marek-safar
Copy link
Contributor

Looking at the problem again I think we should redefine this User story to be instead

"Developers can use .NET APIs which depends on code generation when targeting AOT" and move other source generators to different user stories.

@samsp-msft
Copy link
Member Author

Looking at the problem again I think we should redefine this User story to be instead

"Developers can use .NET APIs which depends on code generation when targeting AOT" and move other source generators to different user stories.

This originally started with Trimming, which has a lot of synergy with AOT, but I don't know what your suggestion would do to the list - I suspect it mainly impacts "Marshaling interop – #43060", but i'd like to understand what @marek-safar's list would look like.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Nov 6, 2020

@Pilchie:

Tagging @SteveSandersonMS here for the first item, can you take a look at what the set of places we might want to tackle within Blazor is?

Done. There's no direct Reflection.Emit (from memory, and as far as I can tell via searching sources), but there's a lot of reflection, which I've listed.

@marek-safar
Copy link
Contributor

@samsp-msft yeah, that was my point. The list will look very different if the intent of the user story is for AOT (and which AOT) or if it's for making the output smaller.

If we stay with AOT only it I'd exclude
- Regular Expressions
- ASP.NET
- Marshaling interop

but others could be added, like SLE

@samsp-msft
Copy link
Member Author

samsp-msft commented Nov 9, 2020

@danmoseley
Copy link
Member

@jaredpar, System.Linq.Expressions has come up a few times - @marek-safar mentions it works today for precompiled Xamarin apps so it needs to continue to work when they're rebuilt against .NET 6. Your team owns this library. Have you discussed whether there is work needed on S.L.E for this scenario to continue to work? I am not sure whether they have their own implementation of it in Mono, or it's the same - if the same, one would hope there's little or no work.

@danmoseley
Copy link
Member

@marek-safar looking at the other uses of Ref.Emit above -- is it important that XML serialization and DCS serialization (owned by @HongGit ) work in AOT apps ? What about Microsoft.CSharp (owned by @jaredpar )?

I'm going to assume we don't care about MEF and XSL compilation..

@jaredpar
Copy link
Member

@marek-safar mentions it works today for precompiled Xamarin apps so it needs to continue to work when they're rebuilt against .NET 6.

@marek-safar how does SLE work in this scenario? Are you just using the interpreted version vs. the compiled?

@marek-safar
Copy link
Contributor

is it important that XML serialization and DCS serialization

XML serialization and System.Runtime.Serialization both work today on mobile and are trimmable at BCL level.

What about Microsoft.CSharp

Microsoft.CSharp works today so it would be regression if we break it in .NET6 for mobile or we need to have a viable alternative.

how does SLE work in this scenario

@jaredpar it uses interpreted version of SLE for AOT platforms

@danmoseley
Copy link
Member

XML serialization and System.Runtime.Serialization both work today on mobile and are trimmable at BCL level.

@marek-safar perhaps these are among the libraries that we simply mark "don't trim". Not sure whether by trimmable you mean the linker actually trims them.
cc @HongGit who owns these.

@marek-safar
Copy link
Contributor

perhaps these are among the libraries that we simply mark "don't trim".

I guess XML serialization is somewhere inside System.Private.XML which is like 4MB, not trimming that won't work.

Not sure whether by trimmable you mean the linker actually trims them.

Yes, linker can trim the library code.

what I also meant by "XML serialization and System.Runtime.Serialization both work today on mobile" is that they are AOT compatible.

@jaredpar
Copy link
Member

it uses interpreted version of SLE for AOT platforms

Gotcha. In that case my thought is that we should continue using the interpreted version there unless there is a reason I'm missing to change that.

@danmoseley
Copy link
Member

System.Runtime.Serialization contains both DCS and also BinaryFormatter. I assume you mean DCS.

@eerhardt I assume DCS work is still required after your #42824

@eerhardt
Copy link
Member

@eerhardt I assume DCS work is still required after your #42824

Yes, we either need to mark it as "unsafe for full trimming", make a source generator for it, or add ILLinker feature(s) to support it.

@samsp-msft samsp-msft changed the title User Story: Source generators are added to common libraries to enable apps using them to be trimmed. User Story: Source generators are added to common libraries to improve performance Nov 24, 2020
@samsp-msft samsp-msft added Epic Groups multiple user stories. Can be grouped under a theme. and removed User Story A single user-facing feature. Can be grouped under an epic. labels Nov 24, 2020
@danmoseley
Copy link
Member

@samsp-msft @marek-safar this story no longer has any children. Should it be closed? Or we should link XmlSerializer and DCS - anything else?

Note the name of this user story needs to be improved -- what is "performance"? runtime perf? size? startup? it should be one thing.

@jaredpar thoughts about the use of ref-emit in Microsoft.CSharp - it's AOT unfriendly and apparently Mono has a working version today. cc @marek-safar

@jkotas
Copy link
Member

jkotas commented Nov 29, 2020

use of ref-emit in Microsoft.CSharp

It is used on COM interop specific code path. It is not a problem for Mono because Mono does not support COM interop (and we do not even support Mono in dotnet/runtime repo on Windows currently).

I do not think there is anything to do. The legacy COM interop is AOT unfriendly in many different ways.

@danmoseley
Copy link
Member

@jkotas OK makes sense.

@marek-safar
Copy link
Contributor

this story no longer has any children. Should it be closed?

I'd close it, and if we have XmlSerializer and DCS user story add them to relevant epic they are addressing problems for

@danmoseley
Copy link
Member

I'd close it, and if we have XmlSerializer and DCS user story add them to relevant epic they are addressing problems for

@HongGit OK to close? do you want to create stories for source generators for XmlSerializer and/or DCS? I suggest to do that even if they won't happen in .NET 6, as that will record the decision and discussion can continue there.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2021
@mairaw
Copy link
Contributor

mairaw commented Jan 15, 2021

Is this really completed or was it cut?

@samsp-msft
Copy link
Member Author

The work from this was refactored into other work items, and so was no longer relevant as the planning node.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Meta Epic Groups multiple user stories. Can be grouped under a theme. Priority:2 Work that is important, but not critical for the release Team:Runtime untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests