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

NativeCallableAttribute should be a public API #32462

Closed
AaronRobinsonMSFT opened this issue Feb 18, 2020 · 9 comments · Fixed by #33005
Closed

NativeCallableAttribute should be a public API #32462

AaronRobinsonMSFT opened this issue Feb 18, 2020 · 9 comments · Fixed by #33005
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Interop-coreclr
Milestone

Comments

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Feb 18, 2020

Update This API's name has changed. See #35433.

The NativeCallableAttribute should be made public since it will provide a symmetrical solution with the C# Function pointer proposal.

Proposed API

namespace System.Runtime.InteropServices
{
    /// <summary>
    /// Any method marked with <see cref="System.Runtime.InteropServices.NativeCallableAttribute" /> can be directly called from
    /// native code. The function token can be loaded to a local variable using LDFTN
    /// and passed as a callback to native method.
    /// </summary>
    /// <remarks>
    /// Methods marked with this attribute have the following restrictions:
    ///   * Method must be marked "static".
    ///   * Must not be called from managed code.
    ///   * Must only have <see href="https://docs.microsoft.com/dotnet/framework/interop/blittable-and-non-blittable-types">blittable</see> arguments.
    /// </remarks>
    [AttributeUsage(AttributeTargets.Method)]
    public sealed class NativeCallableAttribute : Attribute
    {
        public NativeCallableAttribute();

        /// <summary>
        /// Optional. If omitted, compiler will choose one for you.
        /// </summary>
        public CallingConvention CallingConvention;

        /// <summary>
        /// Optional. If omitted, then the method is native callable, but no export is emitted during AOT compilation.
        /// </summary>
        public string? EntryPoint;
    }
}

Related:

/cc @jkotas @jaredpar @davidwrighton

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 5.0 milestone Feb 18, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 18, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Feb 18, 2020
@jaredpar
Copy link
Member

C# Function pointer concerns.

I wouldn't consider the C# concerns here when deciding whether or not to make this public. Those concerns are mostly just scenarios where we'd need to take action, or decide explicitly to not take action, if this attribute was made public. The scenarios are fairly well understood, as well as our options for handling them and the cost is small compared to the work we're already putting into function pointers.

Or another way of stating it: if the runtime decides it's worth making this public then the language will be able to adjust. 😄

@AaronRobinsonMSFT
Copy link
Member Author

@jaredpar I believe the plan is for the runtime to expose NativeCallableAttribute for the .NET 5 release. Should I create an issue in another repo?

@jaredpar
Copy link
Member

@333fred we should take an item to make sure we rationalize this in the function pointer design.

@AaronRobinsonMSFT
Copy link
Member Author

/cc @Scottj1s

@AaronRobinsonMSFT
Copy link
Member Author

@lambdageek The CoreCLR is planning on exposing this attribute to provide symmetry with the C# functions pointers language feature. It would be ideal if mono also provided supported. Do you have any thoughts on this?

@lambdageek
Copy link
Member

@AaronRobinsonMSFT we had some discussion about this at mono/mono#17479

In particular there's a couple of points I want to highlight:

  1. It would be great if the EntryPoint attribute from CoreRT was part of the API. This would enable AOT scenarios to dlopen the symbol without having to call into managed first. On Xamarin.Android we would get faster startup.
  2. Mono's MonoPInvokeCallableAttribute that is used by Xamarin.iOS also supports a delegate type attribute which allows the specification of marshalling behavior. Support System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute mono/mono#17479 (comment)
  3. I'm unclear why this restriction: "May not be called from managed code." (I'm interpreting it as "must not be called by managed code") is necessary.

/cc @vargaz @jonpryor @rolfbjarne

@jkotas
Copy link
Member

jkotas commented Feb 28, 2020

It would be great if the EntryPoint attribute from CoreRT was part of the API. This would enable AOT scenarios to dlopen the symbol without having to call into managed first.

I think it is fine idea to add it. Of course, it is going be recognized only when you have native AOT compiler or something equivalent.

Do you believe that you will be able to add support for the EntryPoint attribute in .NET 5 for Mono AOT and use it for Xamarin Android?

On Xamarin.Android we would get faster startup.

What would be your back-of-the-envelope estimate for the improvement? I do not think it would significantly faster to use dlopen vs. just use C# function pointers + NativeCallbableAttribute.

Mono's MonoPInvokeCallableAttribute that is used by Xamarin.iOS also supports a delegate type attribute which allows the specification of marshalling behavior.

We are intentionally limiting this to blittable types only. We would like to avoid introducing more places that depend on marshaling magic done by the runtime. The marshaling is best to be generated at build time, e.g. like it is done for Xamarin Android or iOS bindings today. Source generators should make it more straightforward.

I'm unclear why this restriction: "May not be called from managed code."

Disallowing the NativeCallableMethod from being called by other managed code makes the implementation a lot simpler. There would be a lot of cases to consider otherwise, e.g. the method can be called via reflection, etc.

In practice, we do not expect this to be material limitation. The methods implementing reverse PInvoke callbacks are very rarely called by other managed code today.

@rolfbjarne
Copy link
Member

Do you believe that you will be able to add support for the EntryPoint attribute in .NET 5 for Mono AOT and use it for Xamarin Android?

I've wished for something like this for Xamarin.iOS for years, it would simplify and speed up the native-to-managed transition when Objective-C calls managed code. Exactly how much would depend on a lot of things (number and types of arguments for instance), but I believe it could be substantial (testing would be required to get actual numbers).

I do not think it would significantly faster to use dlopen vs. just use C# function pointers + NativeCallbableAttribute.

For Xamarin.iOS we're generating Objective-C code, and we'd just generate a direct C function call to the EntryPoint. No dlopen needed (which is slow, and prevents some optimizations Apple does at both link time and runtime).

We are intentionally limiting this to blittable types only.

That shouldn't pose a problem for Xamarin.iOS.

I'm unclear why this restriction: "May not be called from managed code."

In practice, we do not expect this to be material limitation.

It seems it's easy enough to work around this limitation by just using a second managed method which does all the work.

@terrajobst terrajobst added blocking Marks issues that we want to fast track in order to unblock other important work api-approved API was approved in API review, it can be implemented and removed api-ready-for-review blocking Marks issues that we want to fast track in order to unblock other important work labels Mar 3, 2020
@terrajobst
Copy link
Member

terrajobst commented Mar 3, 2020

Video

  • Looks good as proposed.
  • We modelled it off of DllImportAttribute, which is why it's named EntryPoint and those are fields.
namespace System.Runtime.InteropServices
{
    [AttributeUsage(AttributeTargets.Method)]
    public sealed class NativeCallableAttribute : Attribute
    {
        public NativeCallableAttribute();

        public CallingConvention CallingConvention;
        public string? EntryPoint;
    }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Interop-coreclr
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants