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

Question: How to use locally built CsWinRT #989

Closed
kant2002 opened this issue Sep 10, 2021 · 21 comments
Closed

Question: How to use locally built CsWinRT #989

kant2002 opened this issue Sep 10, 2021 · 21 comments
Labels
question Further information is requested
Milestone

Comments

@kant2002
Copy link
Contributor

I trying to run WinRT application using NativeAOT, using this sample dotnet/runtimelab#1453.
If I start using ToastNotificationManager.GetTemplateContent then NativeAOT start complaining about usage of S.L.E.Expression.GetDelegateType. Error which i'm seeing comes from here

private static readonly Type GetAt_0_Type = Expression.GetDelegateType(new Type[] { typeof(void*), typeof(uint), Marshaler<T>.AbiType.MakeByRefType(), typeof(int) });

I would like to change corresponding codegen here
w.write("public static readonly Type %_Type = Expression.GetDelegateType(new Type[]{ typeof(void*), %typeof(int) });\n",

But I would like to test changes locally before even trying to submit something here.
Also what's rationale for making delegate type dynamically, and not using generated delegate types.

@kant2002 kant2002 added the question Further information is requested label Sep 10, 2021
@manodasanW
Copy link
Member

@kant2002 you can find instructions on how to do a local build here. Let me know if you run into any issues. Then you can consume the test version of the projections (Windows, Reunion) that we build during the build with the built cswinrt in the unittest project or WinUIDesktopSample to try out your changes.

Note that the code that you point to above in IReadOnlyList is a manual projection of that code gen, so you would probably need to update both.

As for why it is done dynamically, based on what T is for IReadOnlyList, the delegate parameter types can be different like for Marshaler<T>.AbiType.MakeByRefType() and doing it dynamically allowed to handle that. Are you proposing declaring it as a delegate with generics for those types or generating it for each potential T?

@kant2002
Copy link
Contributor Author

Are you proposing declaring it as a delegate with generics for those types or generating it for each potential T?

Yes, I think I looking into something like that. I think I would like to put Marshaler<T>.AbiType.MakeByRefType logic in C++ to be able to generate strict delegate out of T

@kant2002
Copy link
Contributor Author

@manodasanW If I read code correctly, Expression.GetDelegateType used because it is not known how much parameters are delegate would have. So that means it is possible that Func/Action types cannot be used. I do not know if all signatures will fit Func/Action. If that's somehow can be checked, I would like to know how. It that's impossible in general case I may add conditional generation of Func/Action based on number of parameters passed. Does that sounds like a plan?

@manodasanW
Copy link
Member

@kant2002 It is true that there isn't a max limit to the number of parameters in the delegates. But given Expression.GetDelegateType is only used in the generic scenarios, which cannot typically be defined outside of the built-in ones from MIDL, I think we can make an assumption that it will be less than C#'s limit of 16 for Func/Action. We can either put an error check in the case it is greater or handle that scenario via another way if it is not much more effort.

For my own understanding, is your plan to replace those instances with Func/Action which then makes it AOT compatible?

@kant2002
Copy link
Contributor Author

is your plan to replace those instances with Func/Action which then makes it AOT compatible?

Yes. That's the current plan.

@jkoritzinsky
Copy link
Member

The other reason Expression.GetDelegateType is used is to handle the fact that generic delegates can't be used in reflection scenarios.

@kant2002
Copy link
Contributor Author

That's sounds unfortunate for NativeAOT case. Does that mean that instead of using Func/Action I should generate custom delegate types, one per each set of parameters?

@jkoritzinsky
Copy link
Member

Yes, but then you hit a problem with generic types.

cc: @AaronRobinsonMSFT, we might want to reconsider interop support for generic delegates or generic unmanaged function pointers to enable making WinRT code AOT-friendly

@kant2002
Copy link
Contributor Author

What kind of problem with generic types?

Isn't I can create two kind of delegates. One which live in namespace, and others can live in the generic type itself. That's a bit complicated, but seems solve all problems. Am I miss something?

@jkoritzinsky
Copy link
Member

A delegate in a generic type (like the following examples) are still technically generic types, so they'll hit the same problem:

class F<T>
{
     delegate void G(T t);
     delegate void H();
}

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Sep 17, 2021

we might want to reconsider interop support for generic delegates or generic unmanaged function pointers to enable making WinRT code AOT-friendly

I don't think this is needed but rather makes things cleaner in C#. I'm not against this at all but if most of the code that consumes this is source generated, is it really high priority relative to other things? These interop boundaries are always ugly in practice so I'm not sure how much better this makes things. Some good examples of what is currently required vs what it could be would be helpful.

@jkotas
Copy link
Member

jkotas commented Sep 17, 2021

we might want to reconsider interop support for generic delegates or generic unmanaged function pointers

If we had that, how would you create the generic instantiations? Would all generic instantiations be discoverable statically, or would you use MakeGenericType/MakeGenericMethod to create them? If it is the latter, it would still work poorly with AOT.

@jkoritzinsky
Copy link
Member

I don't think this is needed but rather makes things cleaner in C#. I'm not against this at all but if most of the code that consumes this is source generated, is it really high priority relative to other things? These interop boundaries are always ugly in practice so I'm not sure how much better this makes things. Some good examples of what is currently required vs what it could be would be helpful.

If it's not required (aka there's an alternative that's AOT friendly), then I'm fine doing the alternative.

we might want to reconsider interop support for generic delegates or generic unmanaged function pointers

If we had that, how would you create the generic instantiations? Would all generic instantiations be discoverable statically, or would you use MakeGenericType/MakeGenericMethod to create them? If it is the latter, it would still work poorly with AOT.

I agree that doing any generic delegate work would only be worth it if we could discover the types statically.

@jkoritzinsky
Copy link
Member

Aaron and I talked offline and came up with some ideas (that would be new .NET 7 work) that would provide alternative solutions without hitting issues with generic delegates in interop or MakeGenericType/MakeGenericMethod. We'll work on putting up a proposal in dotnet/runtime for some of our ideas.

@AaronRobinsonMSFT
Copy link
Member

This is proving to be far more complicated to address with the cleverness @jkoritzinsky and I came up with. We have something but it is going to be better, but not a perfect solution. Will update when the proposal is more mature.

@charlesroddie
Copy link

The other reason Expression.GetDelegateType is used is to handle the fact that generic delegates can't be used in reflection scenarios.

Is it necessary to support these reflection scenarios? I don't know the technicals here but deploying WinUI apps with AOT compilation appears to be blocked by this.

@kant2002
Copy link
Contributor Author

@jkoritzinsky @AaronRobinsonMSFT can you share what's the plan how to move forward here or in near area. I think progress toward AOT in WinUI apps that's interesting.

@AaronRobinsonMSFT
Copy link
Member

@kant2002 It is indeed interesting. We are starting down some other paths right now - primarily ref field work. Our solution for this unfortunately has been slightly deferred and is still in a planning phase. I hate to say this, but can we get another ping in March if we don't say anything before hand?

@kant2002
Copy link
Contributor Author

@AaronRobinsonMSFT obviously I will ping. Even if I probably understand why you don't want expose half-backed plan, I still would like to ask for some information, maybe about general direction of your ideas. Right now all my activity WinUI direction would be halted because I have to wait for March and maybe even more.

This is risky, since right now this is first problem on the way to NativeAOT + WinUI and I bet there some other issues which will pops up after this one would be cracked. And we are talking about only NativeAOT side there. And for me at least 2 additional areas which will generate issues on my road - CsWinRT + WinUI.

So that request coming just from my attempt to speedup process of finding other unknown risky areas on the road to WinUI. I was under impression that my initial plan to have explicit Func/Action types (only where that's possible) is no-go at all. If I can implement that as stop-gap solution, then probably this is not a problem for me at all. I have to ask @manodasanW (because he will maintain) and probably @jkotas (because he probably can have some other cold shower on my enthusiasm)

If answer still "no information for now", then what can I do to make you and Jeremy return to drawing board sooner? Maybe help with testing ref fields in NativeAOT context, or maybe just testing that functionality. I assume that anything which allow ref fields work finish faster will helps.

@AaronRobinsonMSFT
Copy link
Member

The current thought will be to leverage TypedReference and have developers supply callbacks. However, there are a myriad of issues with the below API. The most obvious being TypedReference can't be used as an argument or in a collection or in Generics.

A theoretical usage of the concept:

delegate void UniversalCalleeDelegate(TypedReference retValue, Span<TypedReference> args);

void RegisterCalleeFor(Type ret, IEnumerable<Type> args, UniversalCalleeDelegate callee);

The reason we need to come back to it is the ref fields work is going to give us a long-term building block to enable optimized scenarios that will be as low-overhead as possible, like the above. This also relates to the Reflection work that is being considered. We are at a point where we are trying to create tools/APIs/features that will make Native AOT as good as possible and unfortunately that is going to take some time to get there.

As far as the ref fields work and what can be done, that will be a wait and see. We need to start down the path of adding support and then updating the ECMA spec so we can design and understand a test matrix. Since this also involves language changes, the initial tests will be written in IL, not C#. Feel free to follow dotnet/runtime#63768.

@jkotas
Copy link
Member

jkotas commented Jan 15, 2022

The current thought will be to leverage TypedReference and have developers supply callbacks

The idea behind this is to build a better interpreter than System.Linq.Expressions. It may make things work better than System.Linq.Expressions, but it is not going to match .NET Native. It may get close enough in some cases, depends. If you just want to make a demo work, you can also just use the existing System.Linq.Expressions-based implementation and augment it with rd.xml, etc.

If you are looking for a solution that is on par performance-wise with .NET Native, all this code has to be generated at build time and no reflection and interpreters should be used at runtime. It is what .NET Native has done.

  • Analyzes the application and identify all generic WinRT types that it needs. This analyzer can be either extension of the existing cswinrt tool or a new tool.
  • Generate the marshaling code for all generic WinRT types.
  • Teach the cswinrt runtime to use the pre-generated code for the generic WinRT types.

@manodasanW manodasanW added this to the Release 2.1 milestone Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants